From fa4c0b6af3f3ce9535d4ac23979e5bf2eeb0e1d2 Mon Sep 17 00:00:00 2001 From: Ludovic Henry Date: Sun, 12 Feb 2017 11:10:42 -0500 Subject: [PATCH] [finalizer] Suspend finalizer thread on after shutdown timeout (#4363) Fixes bug https://bugzilla.xamarin.com/show_bug.cgi?id=52429 --- mono/metadata/gc.c | 83 ++++++++++++++++++----------------- mono/metadata/threads-types.h | 1 + mono/metadata/threads.c | 22 ++++++++++ 3 files changed, 65 insertions(+), 41 deletions(-) diff --git a/mono/metadata/gc.c b/mono/metadata/gc.c index 7594090b1d4..bf53c0f126b 100644 --- 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 b562d2c8505..e6a1011d201 100644 --- 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 99206e69f7d..968bd091061 100644 --- 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 -- 2.25.1