[threads] Fix leak of gchandle to MonoInternalThread (#6258)

Fixes https://github.com/mono/mono/issues/6169

3 files changed, 60 insertions, 4 deletions

diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c
index 43c8a61d9ab..ee50106c440 100644
--- a/mono/metadata/threads.c
+++ b/mono/metadata/threads.c
@@ -742,11 +742,16 @@ mono_thread_attach_internal (MonoThread *thread, gboolean force_attach, gboolean
 static void
 mono_thread_detach_internal (MonoInternalThread *thread)
 {
+	MonoThreadInfo *info;
 	gboolean removed;
+	guint32 gchandle;
 
 	g_assert (thread != NULL);
 	SET_CURRENT_OBJECT (thread);
 
+	info = (MonoThreadInfo*) thread->thread_info;
+	g_assert (info);
+
 	THREAD_DEBUG (g_message ("%s: mono_thread_detach for %p (%"G_GSIZE_FORMAT")", __func__, thread, (gsize)thread->tid));
 
 #ifndef HOST_WIN32
@@ -768,7 +773,7 @@ mono_thread_detach_internal (MonoInternalThread *thread)
 	* runtime but not identified as runtime threads needs to make sure thread detach calls won't
 	* race with runtime shutdown.
 	*/
-	mono_threads_add_joinable_runtime_thread (thread->thread_info);
+	mono_threads_add_joinable_runtime_thread (info);
 
 	/*
 	 * thread->synch_cs can be NULL if this was called after
@@ -844,7 +849,7 @@ mono_thread_detach_internal (MonoInternalThread *thread)
 	mono_release_type_locks (thread);
 
 	/* Can happen when we attach the profiler helper thread in order to heapshot. */
-	if (!mono_thread_info_lookup (MONO_UINT_TO_NATIVE_THREAD_ID (thread->tid))->tools_thread)
+	if (!info->tools_thread)
 		MONO_PROFILER_RAISE (thread_stopped, (thread->tid));
 
 	mono_hazard_pointer_clear (mono_hazard_pointer_get (), 1);
@@ -883,7 +888,12 @@ done:
 	SET_CURRENT_OBJECT (NULL);
 	mono_domain_unset ();
 
-	mono_thread_info_unset_internal_thread_gchandle ((MonoThreadInfo*) thread->thread_info);
+	if (!mono_thread_info_try_get_internal_thread_gchandle (info, &gchandle))
+		g_error ("%s: failed to get gchandle, info = %p", __func__, info);
+
+	mono_gchandle_free (gchandle);
+
+	mono_thread_info_unset_internal_thread_gchandle (info);
 
 	/* Don't need to close the handle to this thread, even though we took a
 	 * reference in mono_thread_attach (), because the GC will do it
diff --git a/mono/tests/Makefile.am b/mono/tests/Makefile.am
index 90eff773267..e753ed1e4d4 100755
--- a/mono/tests/Makefile.am
+++ b/mono/tests/Makefile.am
@@ -521,7 +521,8 @@ TESTS_CS_SRC=		\
 	bug-58782-plain-throw.cs \
 	bug-58782-capture-and-throw.cs \
 	recursive-struct-arrays.cs \
-	bug-59281.cs
+	bug-59281.cs \
+	threads-leak.cs
 
 if AMD64
 TESTS_CS_SRC += async-exc-compilation.cs finally_guard.cs finally_block_ending_in_dead_bb.cs
diff --git a/mono/tests/threads-leak.cs b/mono/tests/threads-leak.cs
new file mode 100644
index 00000000000..f91b72acb31
--- /dev/null
+++ b/mono/tests/threads-leak.cs
@@ -0,0 +1,45 @@
+using System;
+using System.Threading;
+
+class Driver
+{
+	static ManualResetEvent mre = new ManualResetEvent (false);
+
+	class MyClassInThread
+	{
+		public int I { get; set; }
+
+		~MyClassInThread()
+		{
+			try {
+				Console.WriteLine($"Finalizer {I}");
+			} catch (NotSupportedException) {
+			}
+
+			mre.Set ();
+		}
+	}
+
+	public static void Main(string[] args)
+	{
+		for (int i = 0; i < 50; ++i) {
+			SpawnThread(i);
+			GC.Collect();
+			GC.Collect();
+			GC.Collect();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+			Console.WriteLine($"Loop      {i}");
+		}
+
+		if (!mre.WaitOne(0))
+			Environment.Exit (1);
+	}
+
+	static void SpawnThread(int i)
+	{
+		var th = new Thread(_ => {}) { IsBackground = true, };
+		th.Start(new MyClassInThread { I = i });
+		th.Join();
+	}
+}