Merge pull request #1718 from madewokherd/sgenthreadcleanup

[sgen] Clean up thread pool on shutdown.

Keeping threads around can cause processes to keep running when they shouldn't, particularly on Windows when the entry point returns.

(The reason this is important for me is that Wine provides _CorExeMain, which is an entry point that does return, unlike normal compiled C programs which appear to call exit() after main() returns. A Windows process doesn't exit until ALL threads have exited, so a thread that never exits means a hung process.)

It doesn't seem like we have other GC-specific cleanup code, so I don't know if this is a good approach. Feedback on that question would be appreciated.

diff --git a/mono/metadata/sgen-mono.c b/mono/metadata/sgen-mono.c
index 42f860df05d7bc979130e9672a251d0f4535c5b9..efa0e892eccf3bd312a72915c482e115c8fad163 100644 (file)
--- a/mono/metadata/sgen-mono.c
+++ b/mono/metadata/sgen-mono.c
@@ -37,6 +37,7 @@
 #include "metadata/handle.h"
 #include "utils/mono-memory-model.h"
 #include "utils/mono-logger-internals.h"
+#include "sgen/sgen-thread-pool.h"
 
 #ifdef HEAVY_STATISTICS
 static guint64 stat_wbarrier_set_arrayref = 0;
@@ -2978,6 +2979,7 @@ mono_gc_base_init (void)
 void
 mono_gc_base_cleanup (void)
 {
+       sgen_thread_pool_shutdown ();
 }
 
 gboolean

diff --git a/mono/sgen/sgen-thread-pool.c b/mono/sgen/sgen-thread-pool.c
index e9706943f847803dc38b88b633b6464aefe85c75..3371d2bfab736b19cb852957758f234d7dc0617d 100644 (file)
--- a/mono/sgen/sgen-thread-pool.c
+++ b/mono/sgen/sgen-thread-pool.c
@@ -41,6 +41,9 @@ static SgenThreadPoolThreadInitFunc thread_init_func;
 static SgenThreadPoolIdleJobFunc idle_job_func;
 static SgenThreadPoolContinueIdleJobFunc continue_idle_job_func;
 
+static volatile gboolean threadpool_shutdown;
+static volatile gboolean thread_finished;
+
 enum {
        STATE_WAITING,
        STATE_IN_PROGRESS,
@@ -109,7 +112,7 @@ thread_func (void *thread_data)
                gboolean do_idle = continue_idle_job ();
                SgenThreadPoolJob *job = get_job_and_set_in_progress ();
 
-               if (!job && !do_idle) {
+               if (!job && !do_idle && !threadpool_shutdown) {
                        /*
                         * pthread_cond_wait() can return successfully despite the condition
                         * not being signalled, so we have to run this in a loop until we
@@ -134,8 +137,7 @@ thread_func (void *thread_data)
                         * have to broadcast.
                         */
                        mono_os_cond_signal (&done_cond);
-               } else {
-                       SGEN_ASSERT (0, do_idle, "Why did we unlock if we still have to wait for idle?");
+               } else if (do_idle) {
                        SGEN_ASSERT (0, idle_job_func, "Why do we have idle work when there's no idle job function?");
                        do {
                                idle_job_func (thread_data);
@@ -146,6 +148,13 @@ thread_func (void *thread_data)
 
                        if (!do_idle)
                                mono_os_cond_signal (&done_cond);
+               } else {
+                       SGEN_ASSERT (0, threadpool_shutdown, "Why did we unlock if no jobs and not shutting down?");
+                       mono_os_mutex_lock (&lock);
+                       thread_finished = TRUE;
+                       mono_os_cond_signal (&done_cond);
+                       mono_os_mutex_unlock (&lock);
+                       return 0;
                }
        }
 
@@ -168,6 +177,24 @@ sgen_thread_pool_init (int num_threads, SgenThreadPoolThreadInitFunc init_func,
        mono_native_thread_create (&thread, thread_func, thread_datas ? thread_datas [0] : NULL);
 }
 
+void
+sgen_thread_pool_shutdown (void)
+{
+       if (!thread)
+               return;
+
+       mono_os_mutex_lock (&lock);
+       threadpool_shutdown = TRUE;
+       mono_os_cond_signal (&work_cond);
+       while (!thread_finished)
+               mono_os_cond_wait (&done_cond, &lock);
+       mono_os_mutex_unlock (&lock);
+
+       mono_os_mutex_destroy (&lock);
+       mono_os_cond_destroy (&work_cond);
+       mono_os_cond_destroy (&done_cond);
+}
+
 SgenThreadPoolJob*
 sgen_thread_pool_job_alloc (const char *name, SgenThreadPoolJobFunc func, size_t size)
 {

diff --git a/mono/sgen/sgen-thread-pool.h b/mono/sgen/sgen-thread-pool.h
index 4dcb3a9a14b76195c06f3ccaf396cdf67dd1e9df..6e227521a3f6a0932f8fc2ff398abc726078808c 100644 (file)
--- a/mono/sgen/sgen-thread-pool.h
+++ b/mono/sgen/sgen-thread-pool.h
@@ -37,6 +37,8 @@ typedef gboolean (*SgenThreadPoolContinueIdleJobFunc) (void);
 
 void sgen_thread_pool_init (int num_threads, SgenThreadPoolThreadInitFunc init_func, SgenThreadPoolIdleJobFunc idle_func, SgenThreadPoolContinueIdleJobFunc continue_idle_func, void **thread_datas);
 
+void sgen_thread_pool_shutdown (void);
+
 SgenThreadPoolJob* sgen_thread_pool_job_alloc (const char *name, SgenThreadPoolJobFunc func, size_t size);
 /* This only needs to be called on jobs that are not enqueued. */
 void sgen_thread_pool_job_free (SgenThreadPoolJob *job);