[finalizer] Suspend finalizer thread on after shutdown timeout (#4363)

Fixes bug https://bugzilla.xamarin.com/show_bug.cgi?id=52429

diff --git a/mono/metadata/gc.c b/mono/metadata/gc.c
index 7594090b1d48da5766a663f2af739e796259fec5..bf53c0f126b18873869bc018cc674cbb2fa057b6 100644 (file)
--- a/mono/metadata/gc.c
+++ b/mono/metadata/gc.c
@@ -1017,58 +1017,59 @@ mono_gc_cleanup (void)
                finished = TRUE;
                if (mono_thread_internal_current () != gc_thread) {
                        int ret;
-                       gint64 start_ticks = mono_msec_ticks ();
-                       gint64 end_ticks = start_ticks + 40000;
+                       gint64 start;
+                       const gint64 timeout = 40 * 1000;
 
                        mono_gc_finalize_notify ();
+
+                       start = mono_msec_ticks ();
+
                        /* Finishing the finalizer thread, so wait a little bit... */
                        /* MS seems to wait for about 2 seconds per finalizer thread */
                        /* and 40 seconds for all finalizers to finish */
-                       while (!finalizer_thread_exited) {
-                               gint64 current_ticks = mono_msec_ticks ();
-                               guint32 timeout;
+                       for (;;) {
+                               gint64 elapsed;
 
-                               if (current_ticks >= end_ticks)
-                                       break;
-                               else
-                                       timeout = end_ticks - current_ticks;
-                               mono_finalizer_lock ();
-                               if (!finalizer_thread_exited)
-                                       mono_coop_cond_timedwait (&exited_cond, &finalizer_mutex, timeout);
-                               mono_finalizer_unlock ();
-                       }
+                               if (finalizer_thread_exited) {
+                                       /* Wait for the thread to actually exit. We don't want the wait
+                                        * to be alertable, because we assert on the result to be SUCCESS_0 */
+                                       ret = guarded_wait (gc_thread->handle, MONO_INFINITE_WAIT, FALSE);
+                                       g_assert (ret == MONO_THREAD_INFO_WAIT_RET_SUCCESS_0);
 
-                       if (!finalizer_thread_exited) {
-                               /* Set a flag which the finalizer thread can check */
-                               suspend_finalizers = TRUE;
-                               mono_gc_suspend_finalizers ();
-
-                               /* Try to abort the thread, in the hope that it is running managed code */
-                               mono_thread_internal_abort (gc_thread);
-
-                               /* Wait for it to stop */
-                               ret = guarded_wait (gc_thread->handle, 100, TRUE);
-
-                               if (ret == MONO_THREAD_INFO_WAIT_RET_TIMEOUT) {
-                                       /*
-                                        * The finalizer thread refused to exit. Make it stop.
-                                        */
-                                       mono_thread_internal_stop (gc_thread);
-                                       ret = guarded_wait (gc_thread->handle, 100, TRUE);
-                                       g_assert (ret != MONO_THREAD_INFO_WAIT_RET_TIMEOUT);
-                                       /* The thread can't set this flag */
-                                       finalizer_thread_exited = TRUE;
+                                       mono_thread_join (GUINT_TO_POINTER (gc_thread->tid));
+                                       break;
                                }
-                       }
 
+                               elapsed = mono_msec_ticks () - start;
+                               if (elapsed >= timeout) {
+                                       /* timeout */
+
+                                       /* Set a flag which the finalizer thread can check */
+                                       suspend_finalizers = TRUE;
+                                       mono_gc_suspend_finalizers ();
 
-                       /* Wait for the thread to actually exit. We don't want the wait
-                        * to be alertable, because we assert on the result to be SUCCESS_0 */
-                       ret = guarded_wait (gc_thread->handle, MONO_INFINITE_WAIT, FALSE);
-                       g_assert (ret == MONO_THREAD_INFO_WAIT_RET_SUCCESS_0);
+                                       /* Try to abort the thread, in the hope that it is running managed code */
+                                       mono_thread_internal_abort (gc_thread);
 
-                       mono_thread_join (GUINT_TO_POINTER (gc_thread->tid));
-                       g_assert (finalizer_thread_exited);
+                                       /* Wait for it to stop */
+                                       ret = guarded_wait (gc_thread->handle, 100, FALSE);
+                                       if (ret == MONO_THREAD_INFO_WAIT_RET_TIMEOUT) {
+                                               /* The finalizer thread refused to exit, suspend it forever. */
+                                               mono_thread_internal_suspend_for_shutdown (gc_thread);
+                                               break;
+                                       }
+
+                                       g_assert (ret == MONO_THREAD_INFO_WAIT_RET_SUCCESS_0);
+
+                                       mono_thread_join (GUINT_TO_POINTER (gc_thread->tid));
+                                       break;
+                               }
+
+                               mono_finalizer_lock ();
+                               if (!finalizer_thread_exited)
+                                       mono_coop_cond_timedwait (&exited_cond, &finalizer_mutex, timeout - elapsed);
+                               mono_finalizer_unlock ();
+                       }
                }
                gc_thread = NULL;
                mono_gc_base_cleanup ();

diff --git a/mono/metadata/threads-types.h b/mono/metadata/threads-types.h
index b562d2c850502b9edc67cd390d1978b5f32c59a7..e6a1011d201d5c9c17216573840aca7760b24b17 100644 (file)
--- a/mono/metadata/threads-types.h
+++ b/mono/metadata/threads-types.h
@@ -178,6 +178,7 @@ void mono_thread_internal_check_for_interruption_critical (MonoInternalThread *t
 
 void mono_thread_internal_stop (MonoInternalThread *thread);
 void mono_thread_internal_abort (MonoInternalThread *thread);
+void mono_thread_internal_suspend_for_shutdown (MonoInternalThread *thread);
 
 gboolean mono_thread_internal_has_appdomain_ref (MonoInternalThread *thread, MonoDomain *domain);
 

diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c
index 99206e69f7db75b4a951fb63aec816450f9ad9ed..968bd091061ba5df7eb0828566271b652e0154d4 100644 (file)
--- a/mono/metadata/threads.c
+++ b/mono/metadata/threads.c
@@ -5006,6 +5006,28 @@ self_suspend_internal (void)
        MONO_EXIT_GC_SAFE;
 }
 
+static void
+suspend_for_shutdown_async_call (gpointer unused)
+{
+       for (;;)
+               mono_thread_info_yield ();
+}
+
+static SuspendThreadResult
+suspend_for_shutdown_critical (MonoThreadInfo *info, gpointer unused)
+{
+       mono_thread_info_setup_async_call (info, suspend_for_shutdown_async_call, NULL);
+       return MonoResumeThread;
+}
+
+void
+mono_thread_internal_suspend_for_shutdown (MonoInternalThread *thread)
+{
+       g_assert (thread != mono_thread_internal_current ());
+
+       mono_thread_info_safe_suspend_and_run (thread_get_tid (thread), FALSE, suspend_for_shutdown_critical, NULL);
+}
+
 /*
  * mono_thread_is_foreign:
  * @thread: the thread to query