From dc5e9e4d080f223d877c5b507a1ec4eabd64d4f3 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Mon, 21 Jan 2013 04:52:01 +0100 Subject: [PATCH] Add a hack to allow us to wait for the finalizer thread to exit using pthread_join (). This avoids some shutdown crashes, since WaitForSingleEx () would only wait for the thread's exit event to be signalled, the thread would continue to run. Hopefully fixes #4389. --- mono/io-layer/processes.h | 4 ++++ mono/io-layer/wthreads.c | 8 +++++--- mono/metadata/gc.c | 11 ++++++++++- mono/metadata/threadpool.c | 14 +++++++------- mono/metadata/threads-types.h | 2 +- mono/metadata/threads.c | 20 +++++++++++++++++--- 6 files changed, 44 insertions(+), 15 deletions(-) diff --git a/mono/io-layer/processes.h b/mono/io-layer/processes.h index 486d0005c3c..b0a9d805908 100644 --- a/mono/io-layer/processes.h +++ b/mono/io-layer/processes.h @@ -146,6 +146,10 @@ struct _WapiShellExecuteInfo #define CREATE_DEFAULT_ERROR_MODE 0x04000000 #define CREATE_NO_WINDOW 0x08000000 +#ifndef HOST_WIN32 +#define CREATE_NO_DETACH 0x10000000 +#endif + #ifdef NEW_STUFF #define CREATE_PRESERVE_CODE_AUTHZ_LEVEL find out the value for this one... #endif diff --git a/mono/io-layer/wthreads.c b/mono/io-layer/wthreads.c index 0a422356378..b5d6d6c5511 100644 --- a/mono/io-layer/wthreads.c +++ b/mono/io-layer/wthreads.c @@ -247,9 +247,11 @@ static void *thread_start_routine (gpointer args) { struct _WapiHandle_thread *thread = (struct _WapiHandle_thread *)args; int thr_ret; - - thr_ret = mono_gc_pthread_detach (pthread_self ()); - g_assert (thr_ret == 0); + + if (!(thread->create_flags & CREATE_NO_DETACH)) { + thr_ret = mono_gc_pthread_detach (pthread_self ()); + g_assert (thr_ret == 0); + } thr_ret = pthread_setspecific (thread_hash_key, (void *)thread->handle); diff --git a/mono/metadata/gc.c b/mono/metadata/gc.c index bdf58170ae3..796ff3698cb 100644 --- a/mono/metadata/gc.c +++ b/mono/metadata/gc.c @@ -1118,7 +1118,7 @@ static void mono_gc_init_finalizer_thread (void) { - gc_thread = mono_thread_create_internal (mono_domain_get (), finalizer_thread, NULL, FALSE, 0); + gc_thread = mono_thread_create_internal (mono_domain_get (), finalizer_thread, NULL, FALSE, TRUE, 0); ves_icall_System_Threading_Thread_SetName_internal (gc_thread, mono_string_new (mono_domain_get (), "Finalizer")); } @@ -1207,6 +1207,15 @@ mono_gc_cleanup (void) /* Wait for the thread to actually exit */ ret = WaitForSingleObjectEx (gc_thread->handle, INFINITE, TRUE); g_assert (ret == WAIT_OBJECT_0); + +#ifndef HOST_WIN32 + /* + * The above wait only waits for the exited event to be signalled, the thread might still be running. To fix this race, we + * create the finalizer thread without calling pthread_detach () on it, so we can wait for it manually. + */ + ret = pthread_join ((gpointer)(gsize)gc_thread->tid, NULL); + g_assert (ret == 0); +#endif } } gc_thread = NULL; diff --git a/mono/metadata/threadpool.c b/mono/metadata/threadpool.c index 85ae229e37a..e8a2f1a4574 100644 --- a/mono/metadata/threadpool.c +++ b/mono/metadata/threadpool.c @@ -515,7 +515,7 @@ socket_io_init (SocketIOData *data) data->event_system = POLL_BACKEND; init_event_system (data); - mono_thread_create_internal (mono_get_root_domain (), data->wait, data, TRUE, SMALL_STACK); + mono_thread_create_internal (mono_get_root_domain (), data->wait, data, TRUE, FALSE, SMALL_STACK); LeaveCriticalSection (&data->io_lock); data->inited = 2; threadpool_start_thread (&async_io_tp); @@ -667,7 +667,7 @@ threadpool_start_idle_threads (ThreadPool *tp) #ifndef DISABLE_PERFCOUNTERS mono_perfcounter_update_value (tp->pc_nthreads, TRUE, 1); #endif - mono_thread_create_internal (mono_get_root_domain (), tp->async_invoke, tp, TRUE, stack_size); + mono_thread_create_internal (mono_get_root_domain (), tp->async_invoke, tp, TRUE, FALSE, stack_size); SleepEx (100, TRUE); } while (1); } @@ -1009,7 +1009,7 @@ threadpool_start_thread (ThreadPool *tp) #ifndef DISABLE_PERFCOUNTERS mono_perfcounter_update_value (tp->pc_nthreads, TRUE, 1); #endif - mono_thread_create_internal (mono_get_root_domain (), tp->async_invoke, tp, TRUE, stack_size); + mono_thread_create_internal (mono_get_root_domain (), tp->async_invoke, tp, TRUE, FALSE, stack_size); return TRUE; } } @@ -1048,12 +1048,12 @@ threadpool_append_jobs (ThreadPool *tp, MonoObject **jobs, gint njobs) if (tp->pool_status == 0 && InterlockedCompareExchange (&tp->pool_status, 1, 0) == 0) { if (!tp->is_io) { - mono_thread_create_internal (mono_get_root_domain (), monitor_thread, NULL, TRUE, SMALL_STACK); + mono_thread_create_internal (mono_get_root_domain (), monitor_thread, NULL, TRUE, FALSE, SMALL_STACK); threadpool_start_thread (tp); } /* Create on demand up to min_threads to avoid startup penalty for apps that don't use * the threadpool that much - * mono_thread_create_internal (mono_get_root_domain (), threadpool_start_idle_threads, tp, TRUE, SMALL_STACK); + * mono_thread_create_internal (mono_get_root_domain (), threadpool_start_idle_threads, tp, TRUE, FALSE, SMALL_STACK); */ } @@ -1594,9 +1594,9 @@ ves_icall_System_Threading_ThreadPool_SetMinThreads (gint workerThreads, gint co InterlockedExchange (&async_tp.min_threads, workerThreads); InterlockedExchange (&async_io_tp.min_threads, completionPortThreads); if (workerThreads > async_tp.nthreads) - mono_thread_create_internal (mono_get_root_domain (), threadpool_start_idle_threads, &async_tp, TRUE, SMALL_STACK); + mono_thread_create_internal (mono_get_root_domain (), threadpool_start_idle_threads, &async_tp, TRUE, FALSE, SMALL_STACK); if (completionPortThreads > async_io_tp.nthreads) - mono_thread_create_internal (mono_get_root_domain (), threadpool_start_idle_threads, &async_io_tp, TRUE, SMALL_STACK); + mono_thread_create_internal (mono_get_root_domain (), threadpool_start_idle_threads, &async_io_tp, TRUE, FALSE, SMALL_STACK); return TRUE; } diff --git a/mono/metadata/threads-types.h b/mono/metadata/threads-types.h index 9027c119591..7e9da9a0165 100644 --- a/mono/metadata/threads-types.h +++ b/mono/metadata/threads-types.h @@ -58,7 +58,7 @@ gpointer mono_create_thread (WapiSecurityAttributes *security, guint32 stacksize, WapiThreadStart start, gpointer param, guint32 create, gsize *tid) MONO_INTERNAL; -MonoInternalThread* mono_thread_create_internal (MonoDomain *domain, gpointer func, gpointer arg, gboolean threadpool_thread, guint32 stack_size) MONO_INTERNAL; +MonoInternalThread* mono_thread_create_internal (MonoDomain *domain, gpointer func, gpointer arg, gboolean threadpool_thread, gboolean no_detach, guint32 stack_size) MONO_INTERNAL; void mono_threads_install_cleanup (MonoThreadCleanupFunc func) MONO_INTERNAL; diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index 5b9ca001b15..3f407ec3cb8 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -695,13 +695,22 @@ register_thread_start_argument (MonoThread *thread, struct StartInfo *start_info mono_g_hash_table_insert (thread_start_args, thread, start_info->start_arg); } -MonoInternalThread* mono_thread_create_internal (MonoDomain *domain, gpointer func, gpointer arg, gboolean threadpool_thread, guint32 stack_size) +/* + * mono_thread_create_internal: + * + * If NO_DETACH is TRUE, then the thread is not detached using pthread_detach (). This is needed to fix the race condition where waiting for a thred to exit only waits for its exit event to be + * signalled, which can cause shutdown crashes if the thread shutdown code accesses data already freed by the runtime shutdown. + * Currently, this is only used for the finalizer thread. + */ +MonoInternalThread* +mono_thread_create_internal (MonoDomain *domain, gpointer func, gpointer arg, gboolean threadpool_thread, gboolean no_detach, guint32 stack_size) { MonoThread *thread; MonoInternalThread *internal; HANDLE thread_handle; struct StartInfo *start_info; gsize tid; + guint32 create_flags; thread = create_thread_object (domain); internal = create_internal_thread_object (); @@ -733,8 +742,13 @@ MonoInternalThread* mono_thread_create_internal (MonoDomain *domain, gpointer fu /* Create suspended, so we can do some housekeeping before the thread * starts */ + create_flags = CREATE_SUSPENDED; +#ifndef HOST_WIN32 + if (no_detach) + create_flags |= CREATE_NO_DETACH; +#endif thread_handle = mono_create_thread (NULL, stack_size, (LPTHREAD_START_ROUTINE)start_wrapper, start_info, - CREATE_SUSPENDED, &tid); + create_flags, &tid); THREAD_DEBUG (g_message ("%s: Started thread ID %"G_GSIZE_FORMAT" (handle %p)", __func__, tid, thread_handle)); if (thread_handle == NULL) { /* The thread couldn't be created, so throw an exception */ @@ -773,7 +787,7 @@ MonoInternalThread* mono_thread_create_internal (MonoDomain *domain, gpointer fu void mono_thread_create (MonoDomain *domain, gpointer func, gpointer arg) { - mono_thread_create_internal (domain, func, arg, FALSE, 0); + mono_thread_create_internal (domain, func, arg, FALSE, FALSE, 0); } /* -- 2.25.1