From: monojenkins <jo.shields+jenkins@xamarin.com>
Date: Tue, 29 Mar 2016 20:01:05 +0000 (+0100)
Subject: Merge pull request #1718 from madewokherd/sgenthreadcleanup
X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=ac229973136cd80c6ff5c7fee05a63b24e1bcad3;hp=136dd01035b2e954d3da75011c11dd6b6ab429cd;p=mono.git

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 42f860df05d..efa0e892ecc 100644
--- 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 e9706943f84..3371d2bfab7 100644
--- 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 4dcb3a9a14b..6e227521a3f 100644
--- 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);