From cd1fb047330603d08d2882b0ac812d194cbea0c6 Mon Sep 17 00:00:00 2001 From: Ludovic Henry Date: Fri, 17 Feb 2017 14:47:24 -0500 Subject: [PATCH] [threadpool] Let the runtime abort and wait for threads on shutdown (#4348) * [threadpool] Let the runtime abort and wait for threads on shutdown We would previously have the threadpool abort and wait for its threads during runtime shutdown, but it would tend to be buggy or leaky. The runtime already takes care of aborting and waiting all the other threads, so we simply also let it take care of the threadpool threads. * [threadpool] Wake up all parked threads on shutdown This fixes a hang at shutdown, where one or more threadpool threads might not be unparked and only exit after the timeout, delaying runtime shutdown. --- mono/metadata/gc-internals.h | 2 - mono/metadata/gc.c | 43 ------ mono/metadata/runtime.c | 3 - mono/metadata/threadpool-io.c | 29 ++-- mono/metadata/threadpool-worker-default.c | 156 ++++++++++------------ mono/metadata/threadpool.c | 122 ++++++----------- mono/metadata/threads.c | 15 +-- mono/mini/mini-runtime.c | 3 + mono/utils/mono-lazy-init.h | 7 +- 9 files changed, 145 insertions(+), 235 deletions(-) diff --git a/mono/metadata/gc-internals.h b/mono/metadata/gc-internals.h index 38aa44de6ca..f71f0bc9f4a 100644 --- a/mono/metadata/gc-internals.h +++ b/mono/metadata/gc-internals.h @@ -168,8 +168,6 @@ void mono_gc_wbarrier_set_root (gpointer ptr, MonoObject *value); mono_gc_wbarrier_set_root (&((s)->fieldname), (MonoObject*)value); \ } while (0) -void mono_gc_finalize_threadpool_threads (void); - /* fast allocation support */ typedef enum { diff --git a/mono/metadata/gc.c b/mono/metadata/gc.c index bf53c0f126b..7402096d704 100644 --- a/mono/metadata/gc.c +++ b/mono/metadata/gc.c @@ -68,7 +68,6 @@ static MonoCoopMutex finalizer_mutex; static MonoCoopMutex reference_queue_mutex; static GSList *domains_to_finalize; -static MonoMList *threads_to_finalize; static gboolean finalizer_thread_exited; /* Uses finalizer_mutex */ @@ -157,18 +156,6 @@ coop_cond_timedwait_alertable (MonoCoopCond *cond, MonoCoopMutex *mutex, guint32 return res; } -static gboolean -add_thread_to_finalize (MonoInternalThread *thread, MonoError *error) -{ - mono_error_init (error); - mono_finalizer_lock (); - if (!threads_to_finalize) - MONO_GC_REGISTER_ROOT_SINGLE (threads_to_finalize, MONO_ROOT_SOURCE_FINALIZER_QUEUE, "finalizable threads list"); - threads_to_finalize = mono_mlist_append_checked (threads_to_finalize, (MonoObject*)thread, error); - mono_finalizer_unlock (); - return is_ok (error); -} - /* * actually, we might want to queue the finalize requests in a separate thread, * but we need to be careful about the execution domain of the thread... @@ -241,15 +228,6 @@ mono_gc_run_finalize (void *obj, void *data) if (mono_gc_is_finalizer_internal_thread (t)) /* Avoid finalizing ourselves */ return; - - if (t->threadpool_thread && finalizing_root_domain) { - /* Don't finalize threadpool threads when - shutting down - they're finalized when the - threadpool shuts down. */ - if (!add_thread_to_finalize (t, &error)) - goto unhandled_error; - return; - } } if (o->vtable->klass->image == mono_defaults.corlib && !strcmp (o->vtable->klass->name, "DynamicMethod") && finalizing_root_domain) { @@ -344,22 +322,6 @@ unhandled_error: mono_domain_set_internal (caller_domain); } -void -mono_gc_finalize_threadpool_threads (void) -{ - while (threads_to_finalize) { - MonoInternalThread *thread = (MonoInternalThread*) mono_mlist_get_data (threads_to_finalize); - - /* Force finalization of the thread. */ - thread->threadpool_thread = FALSE; - mono_object_register_finalizer ((MonoObject*)thread); - - mono_gc_run_finalize (thread, NULL); - - threads_to_finalize = mono_mlist_next (threads_to_finalize); - } -} - gpointer mono_gc_out_of_memory (size_t size) { @@ -555,11 +517,6 @@ mono_domain_finalize (MonoDomain *domain, guint32 timeout) goto done; } - if (domain == mono_get_root_domain ()) { - mono_threadpool_cleanup (); - mono_gc_finalize_threadpool_threads (); - } - done: if (InterlockedDecrement (&req->ref) == 0) { mono_coop_sem_destroy (&req->done); diff --git a/mono/metadata/runtime.c b/mono/metadata/runtime.c index 4e6e08c2b3e..df6643672e9 100644 --- a/mono/metadata/runtime.c +++ b/mono/metadata/runtime.c @@ -109,9 +109,6 @@ mono_runtime_try_shutdown (void) mono_runtime_set_shutting_down (); - /* This will kill the tp threads which cannot be suspended */ - mono_threadpool_cleanup (); - /*TODO move the follow to here: mono_thread_suspend_all_other_threads (); OR mono_thread_wait_all_other_threads diff --git a/mono/metadata/threadpool-io.c b/mono/metadata/threadpool-io.c index ace9a00bf6a..618d995dceb 100644 --- a/mono/metadata/threadpool-io.c +++ b/mono/metadata/threadpool-io.c @@ -306,6 +306,12 @@ wait_callback (gint fd, gint events, gpointer user_data) } } +static void +selector_thread_interrupt (gpointer unused) +{ + selector_thread_wakeup (); +} + static gsize WINAPI selector_thread (gpointer data) { @@ -321,9 +327,13 @@ selector_thread (gpointer data) states = mono_g_hash_table_new_type (g_direct_hash, g_direct_equal, MONO_HASH_VALUE_GC, MONO_ROOT_SOURCE_THREAD_POOL, "i/o thread pool states table"); - for (;;) { + while (!mono_runtime_is_shutting_down ()) { gint i, j; gint res; + gboolean interrupted = FALSE; + + if (mono_thread_interruption_checkpoint ()) + continue; mono_coop_mutex_lock (&threadpool_io->updates_lock); @@ -422,10 +432,15 @@ selector_thread (gpointer data) mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_THREADPOOL, "io threadpool: wai"); - res = threadpool_io->backend.event_wait (wait_callback, states); + mono_thread_info_install_interrupt (selector_thread_interrupt, NULL, &interrupted); + if (interrupted) + continue; - if (res == -1 || mono_runtime_is_shutting_down ()) + res = threadpool_io->backend.event_wait (wait_callback, states); + if (res == -1) break; + + mono_thread_info_uninstall_interrupt (&interrupted); } mono_g_hash_table_destroy (states); @@ -548,13 +563,7 @@ initialize (void) static void cleanup (void) { - /* we make the assumption along the code that we are - * cleaning up only if the runtime is shutting down */ - g_assert (mono_runtime_is_shutting_down ()); - - selector_thread_wakeup (); - while (io_selector_running) - mono_thread_info_usleep (1000); + // FIXME destroy everything } void diff --git a/mono/metadata/threadpool-worker-default.c b/mono/metadata/threadpool-worker-default.c index a0cdd9a2170..5fc716a74e7 100644 --- a/mono/metadata/threadpool-worker-default.c +++ b/mono/metadata/threadpool-worker-default.c @@ -124,18 +124,14 @@ typedef union { gint64 as_gint64; } ThreadPoolWorkerCounter; -typedef MonoInternalThread ThreadPoolWorkerThread; - typedef struct { MonoRefCount ref; ThreadPoolWorkerCounter counters; - GPtrArray *threads; // ThreadPoolWorkerThread* [] - MonoCoopMutex threads_lock; /* protect access to working_threads and parked_threads */ + MonoCoopMutex parked_threads_lock; gint32 parked_threads_count; MonoCoopCond parked_threads_cond; - MonoCoopCond threads_exit_cond; ThreadPoolWorkItem *work_items; // ThreadPoolWorkItem [] gint32 work_items_count; @@ -221,8 +217,7 @@ rand_next (gpointer *handle, guint32 min, guint32 max) static void destroy (gpointer data) { -#if 0 - mono_coop_mutex_destroy (&worker.threads_lock); + mono_coop_mutex_destroy (&worker.parked_threads_lock); mono_coop_cond_destroy (&worker.parked_threads_cond); mono_coop_mutex_destroy (&worker.work_items_lock); @@ -232,7 +227,6 @@ destroy (gpointer data) mono_coop_mutex_destroy (&worker.heuristic_lock); g_free (worker.cpu_usage_state); -#endif } void @@ -245,11 +239,9 @@ mono_threadpool_worker_init (void) mono_refcount_init (&worker, destroy); - worker.threads = g_ptr_array_new (); - mono_coop_mutex_init (&worker.threads_lock); + mono_coop_mutex_init (&worker.parked_threads_lock); worker.parked_threads_count = 0; mono_coop_cond_init (&worker.parked_threads_cond); - mono_coop_cond_init (&worker.threads_exit_cond); /* worker.work_items_size is inited to 0 */ mono_coop_mutex_init (&worker.work_items_lock); @@ -316,43 +308,6 @@ mono_threadpool_worker_init (void) void mono_threadpool_worker_cleanup (void) { - MonoInternalThread *current; - - /* we make the assumption along the code that we are - * cleaning up only if the runtime is shutting down */ - g_assert (mono_runtime_is_shutting_down ()); - - current = mono_thread_internal_current (); - - while (worker.monitor_status != MONITOR_STATUS_NOT_RUNNING) - mono_thread_info_sleep (1, NULL); - - mono_coop_mutex_lock (&worker.threads_lock); - - /* unpark all worker.parked_threads */ - mono_coop_cond_broadcast (&worker.parked_threads_cond); - -#if 0 - for (;;) { - ThreadPoolWorkerCounter counter; - - counter = COUNTER_READ (); - if (counter._.starting + counter._.working + counter._.parked == 0) - break; - - if (counter._.starting + counter._.working + counter._.parked == 1) { - if (worker.threads->len == 1 && g_ptr_array_index (worker.threads, 0) == current) { - /* We are waiting on ourselves */ - break; - } - } - - mono_coop_cond_wait (&worker.threads_exit_cond, &worker.threads_lock); - } -#endif - - mono_coop_mutex_unlock (&worker.threads_lock); - mono_refcount_dec (&worker); } @@ -441,17 +396,33 @@ static void worker_request (void); void mono_threadpool_worker_enqueue (MonoThreadPoolWorkerCallback callback, gpointer data) { + if (!mono_refcount_tryinc (&worker)) + return; + work_item_push (callback, data); worker_request (); + + mono_refcount_dec (&worker); } static void worker_wait_interrupt (gpointer unused) { - mono_coop_mutex_lock (&worker.threads_lock); - mono_coop_cond_signal (&worker.parked_threads_cond); - mono_coop_mutex_unlock (&worker.threads_lock); + /* If the runtime is not shutting down, we are not using this mechanism to wake up a unparked thread, and if the + * runtime is shutting down, then we need to wake up ALL the threads. + * It might be a bit wasteful, but I witnessed shutdown hang where the main thread would abort and then wait for all + * background threads to exit (see mono_thread_manage). This would go wrong because not all threadpool threads would + * be unparked. It would end up getting unstucked because of the timeout, but that would delay shutdown by 5-60s. */ + if (!mono_runtime_is_shutting_down ()) + return; + + if (!mono_refcount_tryinc (&worker)) + return; + + mono_coop_mutex_lock (&worker.parked_threads_lock); + mono_coop_cond_broadcast (&worker.parked_threads_cond); + mono_coop_mutex_unlock (&worker.parked_threads_lock); mono_refcount_dec (&worker); } @@ -461,15 +432,15 @@ static gboolean worker_park (void) { gboolean timeout = FALSE; + gboolean interrupted = FALSE; - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] worker parking", mono_native_thread_id_get ()); + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_THREADPOOL, "[%p] worker parking", mono_native_thread_id_get ()); - mono_coop_mutex_lock (&worker.threads_lock); + mono_coop_mutex_lock (&worker.parked_threads_lock); if (!mono_runtime_is_shutting_down ()) { static gpointer rand_handle = NULL; MonoInternalThread *thread; - gboolean interrupted = FALSE; ThreadPoolWorkerCounter counter; if (!rand_handle) @@ -486,19 +457,14 @@ worker_park (void) worker.parked_threads_count += 1; - mono_refcount_inc (&worker); mono_thread_info_install_interrupt (worker_wait_interrupt, NULL, &interrupted); - if (interrupted) { - mono_refcount_dec (&worker); + if (interrupted) goto done; - } - if (mono_coop_cond_timedwait (&worker.parked_threads_cond, &worker.threads_lock, rand_next (&rand_handle, 5 * 1000, 60 * 1000)) != 0) + if (mono_coop_cond_timedwait (&worker.parked_threads_cond, &worker.parked_threads_lock, rand_next (&rand_handle, 5 * 1000, 60 * 1000)) != 0) timeout = TRUE; mono_thread_info_uninstall_interrupt (&interrupted); - if (!interrupted) - mono_refcount_dec (&worker); done: worker.parked_threads_count -= 1; @@ -509,9 +475,10 @@ done: }); } - mono_coop_mutex_unlock (&worker.threads_lock); + mono_coop_mutex_unlock (&worker.parked_threads_lock); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] worker unparking, timeout? %s", mono_native_thread_id_get (), timeout ? "yes" : "no"); + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_THREADPOOL, "[%p] worker unparking, timeout? %s interrupted? %s", + mono_native_thread_id_get (), timeout ? "yes" : "no", interrupted ? "yes" : "no"); return timeout; } @@ -523,12 +490,12 @@ worker_try_unpark (void) mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] try unpark worker", mono_native_thread_id_get ()); - mono_coop_mutex_lock (&worker.threads_lock); + mono_coop_mutex_lock (&worker.parked_threads_lock); if (worker.parked_threads_count > 0) { mono_coop_cond_signal (&worker.parked_threads_cond); res = TRUE; } - mono_coop_mutex_unlock (&worker.threads_lock); + mono_coop_mutex_unlock (&worker.parked_threads_lock); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] try unpark worker, success? %s", mono_native_thread_id_get (), res ? "yes" : "no"); @@ -543,6 +510,9 @@ worker_thread (gpointer unused) mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_THREADPOOL, "[%p] worker starting", mono_native_thread_id_get ()); + if (!mono_refcount_tryinc (&worker)) + return 0; + COUNTER_ATOMIC (counter, { counter._.starting --; counter._.working ++; @@ -551,10 +521,6 @@ worker_thread (gpointer unused) thread = mono_thread_internal_current (); g_assert (thread); - mono_coop_mutex_lock (&worker.threads_lock); - g_ptr_array_add (worker.threads, thread); - mono_coop_mutex_unlock (&worker.threads_lock); - while (!mono_runtime_is_shutting_down ()) { ThreadPoolWorkItem work_item; @@ -577,18 +543,10 @@ worker_thread (gpointer unused) work_item.callback (work_item.data); } - mono_coop_mutex_lock (&worker.threads_lock); - COUNTER_ATOMIC (counter, { counter._.working --; }); - g_ptr_array_remove (worker.threads, thread); - - mono_coop_cond_signal (&worker.threads_exit_cond); - - mono_coop_mutex_unlock (&worker.threads_lock); - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_THREADPOOL, "[%p] worker finishing", mono_native_thread_id_get ()); mono_refcount_dec (&worker); @@ -641,7 +599,6 @@ worker_try_create (void) counter._.starting ++; }); - mono_refcount_inc (&worker); thread = mono_thread_create_internal (mono_get_root_domain (), worker_thread, NULL, TRUE, 0, &error); if (!thread) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] try create worker, failed: could not create thread due to %s", mono_native_thread_id_get (), mono_error_get_message (&error)); @@ -653,8 +610,6 @@ worker_try_create (void) mono_coop_mutex_unlock (&worker.worker_creation_lock); - mono_refcount_dec (&worker); - return FALSE; } @@ -752,6 +707,9 @@ monitor_thread (gpointer unused) MonoInternalThread *internal; guint i; + if (!mono_refcount_tryinc (&worker)) + return 0; + internal = mono_thread_internal_current (); g_assert (internal); @@ -838,6 +796,7 @@ monitor_thread (gpointer unused) mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] monitor thread, finished", mono_native_thread_id_get ()); + mono_refcount_dec (&worker); return 0; } @@ -864,6 +823,7 @@ monitor_ensure_running (void) // printf ("monitor_thread: creating failed\n"); worker.monitor_status = MONITOR_STATUS_NOT_RUNNING; mono_error_cleanup (&error); + mono_refcount_dec (&worker); } return; } @@ -880,7 +840,7 @@ hill_climbing_change_thread_count (gint16 new_thread_count, ThreadPoolHeuristicS hc = &worker.heuristic_hill_climbing; - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_THREADPOOL, "[%p] hill climbing, change max number of threads %d", mono_native_thread_id_get (), new_thread_count); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] hill climbing, change max number of threads %d", mono_native_thread_id_get (), new_thread_count); hc->last_thread_count = new_thread_count; hc->current_sample_interval = rand_next (&hc->random_interval_generator, hc->sample_interval_low, hc->sample_interval_high); @@ -1204,7 +1164,15 @@ mono_threadpool_worker_notify_completed (void) gint32 mono_threadpool_worker_get_min (void) { - return worker.limit_worker_min; + gint32 ret; + + if (!mono_refcount_tryinc (&worker)) + return 0; + + ret = worker.limit_worker_min; + + mono_refcount_dec (&worker); + return ret; } gboolean @@ -1213,14 +1181,27 @@ mono_threadpool_worker_set_min (gint32 value) if (value <= 0 || value > worker.limit_worker_max) return FALSE; + if (!mono_refcount_tryinc (&worker)) + return FALSE; + worker.limit_worker_min = value; + + mono_refcount_dec (&worker); return TRUE; } gint32 mono_threadpool_worker_get_max (void) { - return worker.limit_worker_max; + gint32 ret; + + if (!mono_refcount_tryinc (&worker)) + return 0; + + ret = worker.limit_worker_max; + + mono_refcount_dec (&worker); + return ret; } gboolean @@ -1232,17 +1213,24 @@ mono_threadpool_worker_set_max (gint32 value) if (value < worker.limit_worker_min || value < cpu_count) return FALSE; - if (value < worker.limit_worker_min || value < cpu_count) + if (!mono_refcount_tryinc (&worker)) return FALSE; worker.limit_worker_max = value; + + mono_refcount_dec (&worker); return TRUE; } void mono_threadpool_worker_set_suspended (gboolean suspended) { + if (!mono_refcount_tryinc (&worker)) + return; + worker.suspended = suspended; if (!suspended) worker_request (); + + mono_refcount_dec (&worker); } diff --git a/mono/metadata/threadpool.c b/mono/metadata/threadpool.c index 4b0aa6ec18d..f3ff6b6ac7d 100644 --- a/mono/metadata/threadpool.c +++ b/mono/metadata/threadpool.c @@ -71,10 +71,6 @@ typedef struct { GPtrArray *domains; // ThreadPoolDomain* [] MonoCoopMutex domains_lock; - GPtrArray *threads; // MonoInternalThread* [] - MonoCoopMutex threads_lock; - MonoCoopCond threads_exit_cond; - ThreadPoolCounter counters; gint32 limit_io_min; @@ -121,14 +117,8 @@ domains_unlock (void) static void destroy (gpointer unused) { -#if 0 g_ptr_array_free (threadpool.domains, TRUE); mono_coop_mutex_destroy (&threadpool.domains_lock); - - g_ptr_array_free (threadpool.threads, TRUE); - mono_coop_mutex_destroy (&threadpool.threads_lock); - mono_coop_cond_destroy (&threadpool.threads_exit_cond); -#endif } static void @@ -141,10 +131,6 @@ initialize (void) threadpool.domains = g_ptr_array_new (); mono_coop_mutex_init (&threadpool.domains_lock); - threadpool.threads = g_ptr_array_new (); - mono_coop_mutex_init (&threadpool.threads_lock); - mono_coop_cond_init (&threadpool.threads_exit_cond); - threadpool.limit_io_min = mono_cpu_count (); threadpool.limit_io_max = CLAMP (threadpool.limit_io_min * 100, MIN (threadpool.limit_io_min, 200), MAX (threadpool.limit_io_min, 200)); @@ -154,47 +140,6 @@ initialize (void) static void cleanup (void) { - guint i; - MonoInternalThread *current; - - /* we make the assumption along the code that we are - * cleaning up only if the runtime is shutting down */ - g_assert (mono_runtime_is_shutting_down ()); - - current = mono_thread_internal_current (); - - mono_coop_mutex_lock (&threadpool.threads_lock); - - /* stop all threadpool.threads */ - for (i = 0; i < threadpool.threads->len; ++i) { - MonoInternalThread *thread = (MonoInternalThread*) g_ptr_array_index (threadpool.threads, i); - if (thread != current) - mono_thread_internal_abort (thread); - } - - mono_coop_mutex_unlock (&threadpool.threads_lock); - -#if 0 - /* give a chance to the other threads to exit */ - mono_thread_info_yield (); - - mono_coop_mutex_lock (&threadpool.threads_lock); - - for (;;) { - if (threadpool.threads->len == 0) - break; - - if (threadpool.threads->len == 1 && g_ptr_array_index (threadpool.threads, 0) == current) { - /* We are waiting on ourselves */ - break; - } - - mono_coop_cond_wait (&threadpool.threads_exit_cond, &threadpool.threads_lock); - } - - mono_coop_mutex_unlock (&threadpool.threads_lock); -#endif - mono_threadpool_worker_cleanup (); mono_refcount_dec (&threadpool); @@ -338,6 +283,9 @@ worker_callback (gpointer unused) ThreadPoolCounter counter; MonoInternalThread *thread; + if (!mono_refcount_tryinc (&threadpool)) + return; + thread = mono_thread_internal_current (); COUNTER_ATOMIC (counter, { @@ -357,10 +305,6 @@ worker_callback (gpointer unused) return; } - mono_coop_mutex_lock (&threadpool.threads_lock); - g_ptr_array_add (threadpool.threads, thread); - mono_coop_mutex_unlock (&threadpool.threads_lock); - /* * This is needed so there is always an lmf frame in the runtime invoke call below, * so ThreadAbortExceptions are caught even if the thread is in native code. @@ -447,14 +391,6 @@ worker_callback (gpointer unused) domains_unlock (); - mono_coop_mutex_lock (&threadpool.threads_lock); - - g_ptr_array_remove_fast (threadpool.threads, thread); - - mono_coop_cond_signal (&threadpool.threads_exit_cond); - - mono_coop_mutex_unlock (&threadpool.threads_lock); - COUNTER_ATOMIC (counter, { counter._.working --; }); @@ -484,8 +420,6 @@ mono_threadpool_begin_invoke (MonoDomain *domain, MonoObject *target, MonoMethod if (!async_call_klass) async_call_klass = mono_class_load_from_name (mono_defaults.corlib, "System", "MonoAsyncCall"); - mono_lazy_initialize (&status, initialize); - mono_error_init (error); message = mono_method_call_message_new (method, params, mono_get_delegate_invoke (method->klass), (params != NULL) ? (&async_callback) : NULL, (params != NULL) ? (&state) : NULL, error); @@ -671,12 +605,18 @@ ves_icall_System_Threading_ThreadPool_GetAvailableThreadsNative (gint32 *worker_ if (!worker_threads || !completion_port_threads) return; - mono_lazy_initialize (&status, initialize); + if (!mono_lazy_initialize (&status, initialize) || !mono_refcount_tryinc (&threadpool)) { + *worker_threads = 0; + *completion_port_threads = 0; + return; + } counter = COUNTER_READ (); *worker_threads = MAX (0, mono_threadpool_worker_get_max () - counter._.working); *completion_port_threads = threadpool.limit_io_max; + + mono_refcount_dec (&threadpool); } void @@ -685,10 +625,16 @@ ves_icall_System_Threading_ThreadPool_GetMinThreadsNative (gint32 *worker_thread if (!worker_threads || !completion_port_threads) return; - mono_lazy_initialize (&status, initialize); + if (!mono_lazy_initialize (&status, initialize) || !mono_refcount_tryinc (&threadpool)) { + *worker_threads = 0; + *completion_port_threads = 0; + return; + } *worker_threads = mono_threadpool_worker_get_min (); *completion_port_threads = threadpool.limit_io_min; + + mono_refcount_dec (&threadpool); } void @@ -697,25 +643,35 @@ ves_icall_System_Threading_ThreadPool_GetMaxThreadsNative (gint32 *worker_thread if (!worker_threads || !completion_port_threads) return; - mono_lazy_initialize (&status, initialize); + if (!mono_lazy_initialize (&status, initialize) || !mono_refcount_tryinc (&threadpool)) { + *worker_threads = 0; + *completion_port_threads = 0; + return; + } *worker_threads = mono_threadpool_worker_get_max (); *completion_port_threads = threadpool.limit_io_max; + + mono_refcount_dec (&threadpool); } MonoBoolean ves_icall_System_Threading_ThreadPool_SetMinThreadsNative (gint32 worker_threads, gint32 completion_port_threads) { - mono_lazy_initialize (&status, initialize); - if (completion_port_threads <= 0 || completion_port_threads > threadpool.limit_io_max) return FALSE; - if (!mono_threadpool_worker_set_min (worker_threads)) + if (!mono_lazy_initialize (&status, initialize) || !mono_refcount_tryinc (&threadpool)) + return FALSE; + + if (!mono_threadpool_worker_set_min (worker_threads)) { + mono_refcount_dec (&threadpool); return FALSE; + } threadpool.limit_io_min = completion_port_threads; + mono_refcount_dec (&threadpool); return TRUE; } @@ -724,16 +680,20 @@ ves_icall_System_Threading_ThreadPool_SetMaxThreadsNative (gint32 worker_threads { gint cpu_count = mono_cpu_count (); - mono_lazy_initialize (&status, initialize); - if (completion_port_threads < threadpool.limit_io_min || completion_port_threads < cpu_count) return FALSE; - if (!mono_threadpool_worker_set_max (worker_threads)) + if (!mono_lazy_initialize (&status, initialize) || !mono_refcount_tryinc (&threadpool)) return FALSE; + if (!mono_threadpool_worker_set_max (worker_threads)) { + mono_refcount_dec (&threadpool); + return FALSE; + } + threadpool.limit_io_max = completion_port_threads; + mono_refcount_dec (&threadpool); return TRUE; } @@ -783,7 +743,7 @@ ves_icall_System_Threading_ThreadPool_RequestWorkerThread (void) if (mono_domain_is_unloading (domain)) return FALSE; - if (!mono_refcount_tryinc (&threadpool)) { + if (!mono_lazy_initialize (&status, initialize) || !mono_refcount_tryinc (&threadpool)) { /* threadpool has been destroyed, we are shutting down */ return FALSE; } @@ -820,9 +780,7 @@ ves_icall_System_Threading_ThreadPool_RequestWorkerThread (void) mono_threadpool_worker_enqueue (worker_callback, NULL); - /* we do not decrement the threadpool refcount, - * as it's going to be done in the worker_callback */ - + mono_refcount_dec (&threadpool); return TRUE; } diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index 968bd091061..fd2fe7315d0 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -3208,22 +3208,21 @@ remove_and_abort_threads (gpointer key, gpointer value, gpointer user) if (wait->num >= MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS) return FALSE; - /* The finalizer thread is not a background thread */ - if (!mono_native_thread_id_equals (thread_get_tid (thread), self) - && (thread->state & ThreadState_Background) != 0 - && (thread->flags & MONO_THREAD_FLAG_DONT_MANAGE) == 0 - ) { + if (mono_native_thread_id_equals (thread_get_tid (thread), self)) + return FALSE; + if (mono_gc_is_finalizer_internal_thread (thread)) + return FALSE; + + if ((thread->state & ThreadState_Background) && !(thread->flags & MONO_THREAD_FLAG_DONT_MANAGE)) { wait->handles[wait->num] = mono_threads_open_thread_handle (thread->handle); wait->threads[wait->num] = thread; wait->num++; THREAD_DEBUG (g_print ("%s: Aborting id: %"G_GSIZE_FORMAT"\n", __func__, (gsize)thread->tid)); mono_thread_internal_abort (thread); - return TRUE; } - return !mono_native_thread_id_equals (thread_get_tid (thread), self) - && !mono_gc_is_finalizer_internal_thread (thread); + return TRUE; } /** diff --git a/mono/mini/mini-runtime.c b/mono/mini/mini-runtime.c index ece32339359..a567bf0507f 100644 --- a/mono/mini/mini-runtime.c +++ b/mono/mini/mini-runtime.c @@ -66,6 +66,7 @@ #include #include #include +#include #include "mini.h" #include "seq-points.h" @@ -4110,6 +4111,8 @@ mini_cleanup (MonoDomain *domain) mono_runtime_cleanup (domain); #endif + mono_threadpool_cleanup (); + mono_profiler_shutdown (); free_jit_tls_data ((MonoJitTlsData *)mono_tls_get_jit_tls ()); diff --git a/mono/utils/mono-lazy-init.h b/mono/utils/mono-lazy-init.h index 7deca1275f5..046ef46d6b1 100644 --- a/mono/utils/mono-lazy-init.h +++ b/mono/utils/mono-lazy-init.h @@ -52,7 +52,7 @@ enum { MONO_LAZY_INIT_STATUS_CLEANED, }; -static inline void +static inline gboolean mono_lazy_initialize (mono_lazy_init_t *lazy_init, void (*initialize) (void)) { gint32 status; @@ -62,7 +62,7 @@ mono_lazy_initialize (mono_lazy_init_t *lazy_init, void (*initialize) (void)) status = *lazy_init; if (status >= MONO_LAZY_INIT_STATUS_INITIALIZED) - return; + return status == MONO_LAZY_INIT_STATUS_INITIALIZED; if (status == MONO_LAZY_INIT_STATUS_INITIALIZING || InterlockedCompareExchange (lazy_init, MONO_LAZY_INIT_STATUS_INITIALIZING, MONO_LAZY_INIT_STATUS_NOT_INITIALIZED) != MONO_LAZY_INIT_STATUS_NOT_INITIALIZED @@ -70,12 +70,13 @@ mono_lazy_initialize (mono_lazy_init_t *lazy_init, void (*initialize) (void)) while (*lazy_init == MONO_LAZY_INIT_STATUS_INITIALIZING) mono_thread_info_yield (); g_assert (InterlockedRead (lazy_init) >= MONO_LAZY_INIT_STATUS_INITIALIZED); - return; + return status == MONO_LAZY_INIT_STATUS_INITIALIZED; } initialize (); mono_atomic_store_release (lazy_init, MONO_LAZY_INIT_STATUS_INITIALIZED); + return TRUE; } static inline void -- 2.25.1