[threadpool] Fix hang on threadpool cleanup (#4330)

[mono.git] / mono / metadata / threadpool.c

diff --git a/mono/metadata/threadpool.c b/mono/metadata/threadpool.c
index 84165e226b116705c394a60c72e826574bb32c2b..e58a78d47e0e37e9669762067b68316268d9caf5 100644 (file)
--- a/mono/metadata/threadpool.c
+++ b/mono/metadata/threadpool.c
@@ -131,7 +131,13 @@ destroy (gpointer unused)
        mono_coop_mutex_destroy (&threadpool->threads_lock);
        mono_coop_cond_destroy (&threadpool->threads_exit_cond);
 
-       g_free (threadpool);
+       /* We cannot free the threadpool, because there is a race
+        * on shutdown where a managed thread may request a new
+        * threadpool thread, but we already destroyed the
+        * threadpool. So to avoid a use-after-free, we simply do
+        * not free the threadpool, as we won't be able to access
+        * the threadpool anyway because the ref count will be 0 */
+       // g_free (threadpool);
 }
 
 static void
@@ -181,6 +187,7 @@ cleanup (void)
 
        mono_coop_mutex_unlock (&threadpool->threads_lock);
 
+#if 0
        /* give a chance to the other threads to exit */
        mono_thread_info_yield ();
 
@@ -199,6 +206,7 @@ cleanup (void)
        }
 
        mono_coop_mutex_unlock (&threadpool->threads_lock);
+#endif
 
        mono_threadpool_worker_cleanup (threadpool->worker);
 
@@ -560,7 +568,7 @@ mono_threadpool_end_invoke (MonoAsyncResult *ares, MonoArray **out_args, MonoObj
                        g_assert(wait_event);
                        MonoWaitHandle *wait_handle = mono_wait_handle_new (mono_object_domain (ares), wait_event, error);
                        if (!is_ok (error)) {
-                               CloseHandle (wait_event);
+                               mono_w32event_close (wait_event);
                                return NULL;
                        }
                        MONO_OBJECT_SETREF (ares, handle, (MonoObject*) wait_handle);
@@ -796,11 +804,17 @@ ves_icall_System_Threading_ThreadPool_RequestWorkerThread (void)
        if (mono_domain_is_unloading (domain))
                return FALSE;
 
+       if (!mono_refcount_tryinc (threadpool)) {
+               /* threadpool has been destroyed, we are shutting down */
+               return FALSE;
+       }
+
        domains_lock ();
 
        /* synchronize with mono_threadpool_remove_domain_jobs */
        if (mono_domain_is_unloading (domain)) {
                domains_unlock ();
+               mono_refcount_dec (threadpool);
                return FALSE;
        }
 
@@ -813,14 +827,14 @@ ves_icall_System_Threading_ThreadPool_RequestWorkerThread (void)
        domains_unlock ();
 
        COUNTER_ATOMIC (threadpool, counter, {
-               if (counter._.starting == 16)
+               if (counter._.starting == 16) {
+                       mono_refcount_dec (threadpool);
                        return TRUE;
+               }
 
                counter._.starting ++;
        });
 
-       mono_refcount_inc (threadpool);
-
        mono_threadpool_worker_enqueue (threadpool->worker, worker_callback, NULL);
 
        return TRUE;