Source: Hoshpak
Upstream: https://github.com/mono/mono/issues/7167
Reason: reverts replacing condition variables with a semaphore in the threading code
 which caused a severe regression that introduced a 5-60 second hang while compiling code

From ecd1658935128e700962ff440dd460c31403bec7 Mon Sep 17 00:00:00 2001
From: Helmut Pozimski <helmut@pozimski.eu>
Date: Tue, 9 Oct 2018 12:46:03 +0200
Subject: [PATCH] Revert "[threadpool] Replace parked_threads condition
 variable by a semaphore (#6222)"

This reverts commit 5f5c5e97a08f7086d7c18af37352c4a03dc4c0d1.
---
 mono/metadata/threadpool-worker-default.c | 99 ++++++++++++-----------
 1 file changed, 52 insertions(+), 47 deletions(-)

diff --git a/mono/metadata/threadpool-worker-default.c b/mono/metadata/threadpool-worker-default.c
index 33870a6de15..c5e8529a67f 100644
--- mono/metadata/threadpool-worker-default.c
+++ mono/metadata/threadpool-worker-default.c
@@ -131,8 +131,9 @@ typedef struct {
 
 	ThreadPoolWorkerCounter counters;
 
-	MonoCoopSem parked_threads_sem;
+	MonoCoopMutex parked_threads_lock;
 	gint32 parked_threads_count;
+	MonoCoopCond parked_threads_cond;
 
 	volatile gint32 work_items_count;
 
@@ -215,7 +216,8 @@ rand_next (gpointer *handle, guint32 min, guint32 max)
 static void
 destroy (gpointer data)
 {
-	mono_coop_sem_destroy (&worker.parked_threads_sem);
+	mono_coop_mutex_destroy (&worker.parked_threads_lock);
+	mono_coop_cond_destroy (&worker.parked_threads_cond);
 
 	mono_coop_mutex_destroy (&worker.worker_creation_lock);
 
@@ -236,8 +238,9 @@ mono_threadpool_worker_init (MonoThreadPoolWorkerCallback callback)
 
 	worker.callback = callback;
 
-	mono_coop_sem_init (&worker.parked_threads_sem, 0);
+	mono_coop_mutex_init (&worker.parked_threads_lock);
 	worker.parked_threads_count = 0;
+	mono_coop_cond_init (&worker.parked_threads_cond);
 
 	worker.worker_creation_current_second = -1;
 	mono_coop_mutex_init (&worker.worker_creation_lock);
@@ -356,60 +359,69 @@ mono_threadpool_worker_request (void)
 	mono_refcount_dec (&worker);
 }
 
+static void
+worker_wait_interrupt (gpointer unused)
+{
+	/* 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);
+}
+
 /* return TRUE if timeout, FALSE otherwise (worker unpark or interrupt) */
 static gboolean
 worker_park (void)
 {
 	gboolean timeout = FALSE;
 	gboolean interrupted = FALSE;
-	gint32 old, new;
 
 	mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] worker parking",
 		GUINT_TO_POINTER (MONO_NATIVE_THREAD_ID_TO_UINT (mono_native_thread_id_get ())));
 
+	mono_coop_mutex_lock (&worker.parked_threads_lock);
+
 	if (!mono_runtime_is_shutting_down ()) {
 		static gpointer rand_handle = NULL;
+		MonoInternalThread *thread;
 		ThreadPoolWorkerCounter counter;
 
-		if (!rand_handle) {
+		if (!rand_handle)
 			rand_handle = rand_create ();
-			g_assert (rand_handle);
-		}
+		g_assert (rand_handle);
+
+		thread = mono_thread_internal_current ();
+		g_assert (thread);
 
 		COUNTER_ATOMIC (counter, {
 			counter._.working --;
 			counter._.parked ++;
 		});
 
-		do {
-			old = mono_atomic_load_i32 (&worker.parked_threads_count);
-			g_assert (old >= G_MININT32);
+		worker.parked_threads_count += 1;
 
-			new = old + 1;
-		} while (mono_atomic_cas_i32 (&worker.parked_threads_count, new, old) != old);
+		mono_thread_info_install_interrupt (worker_wait_interrupt, NULL, &interrupted);
+		if (interrupted)
+			goto done;
 
-		switch (mono_coop_sem_timedwait (&worker.parked_threads_sem, rand_next (&rand_handle, 5 * 1000, 60 * 1000), MONO_SEM_FLAGS_ALERTABLE)) {
-		case MONO_SEM_TIMEDWAIT_RET_SUCCESS:
-			break;
-		case MONO_SEM_TIMEDWAIT_RET_ALERTED:
-			interrupted = TRUE;
-			break;
-		case MONO_SEM_TIMEDWAIT_RET_TIMEDOUT:
+		if (mono_coop_cond_timedwait (&worker.parked_threads_cond, &worker.parked_threads_lock, rand_next (&rand_handle, 5 * 1000, 60 * 1000)) != 0)
 			timeout = TRUE;
-			break;
-		default:
-			g_assert_not_reached ();
-		}
 
-		if (interrupted || timeout) {
-			/* If the semaphore was posted, then worker.parked_threads_count was decremented in worker_try_unpark */
-			do {
-				old = mono_atomic_load_i32 (&worker.parked_threads_count);
-				g_assert (old > G_MININT32);
+		mono_thread_info_uninstall_interrupt (&interrupted);
 
-				new = old - 1;
-			} while (mono_atomic_cas_i32 (&worker.parked_threads_count, new, old) != old);
-		}
+done:
+		worker.parked_threads_count -= 1;
 
 		COUNTER_ATOMIC (counter, {
 			counter._.working ++;
@@ -417,6 +429,8 @@ worker_park (void)
 		});
 	}
 
+	mono_coop_mutex_unlock (&worker.parked_threads_lock);
+
 	mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] worker unparking, timeout? %s interrupted? %s",
 		GUINT_TO_POINTER (MONO_NATIVE_THREAD_ID_TO_UINT (mono_native_thread_id_get ())), timeout ? "yes" : "no", interrupted ? "yes" : "no");
 
@@ -426,26 +440,17 @@ worker_park (void)
 static gboolean
 worker_try_unpark (void)
 {
-	gboolean res = TRUE;
-	gint32 old, new;
+	gboolean res = FALSE;
 
 	mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] try unpark worker",
 		GUINT_TO_POINTER (MONO_NATIVE_THREAD_ID_TO_UINT (mono_native_thread_id_get ())));
 
-	do {
-		old = mono_atomic_load_i32 (&worker.parked_threads_count);
-		g_assert (old > G_MININT32);
-
-		if (old <= 0) {
-			res = FALSE;
-			break;
-		}
-
-		new = old - 1;
-	} while (mono_atomic_cas_i32 (&worker.parked_threads_count, new, old) != old);
-
-	if (res)
-		mono_coop_sem_post (&worker.parked_threads_sem);
+	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.parked_threads_lock);
 
 	mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_THREADPOOL, "[%p] try unpark worker, success? %s",
 		GUINT_TO_POINTER (MONO_NATIVE_THREAD_ID_TO_UINT (mono_native_thread_id_get ())), res ? "yes" : "no");
-- 
2.19.1

