Fix Monitor.Wait unregistration in CoreCLR upon a thread exiting

src/coreclr/vm/threaddebugblockinginfo.cpp

@@ -14,13 +14,6 @@ ThreadDebugBlockingInfo::ThreadDebugBlockingInfo()
     m_firstBlockingItem = NULL;
 }
 
-//Destructor
-ThreadDebugBlockingInfo::~ThreadDebugBlockingInfo()
-{
-    WRAPPER_NO_CONTRACT;
-    _ASSERTE(m_firstBlockingItem == NULL);
-}
-
 // Adds a new blocking item at the front of the list
 // The caller is responsible for allocating the memory this points to and keeping it alive until
 // after PopBlockingItem is called

src/coreclr/vm/threaddebugblockinginfo.h

@@ -51,7 +51,6 @@ class ThreadDebugBlockingInfo
 
 public:
     ThreadDebugBlockingInfo();
-    ~ThreadDebugBlockingInfo();
 
 #ifndef DACCESS_COMPILE
     // Adds a new blocking item at the front of the list

src/coreclr/vm/threads.cpp

@@ -930,6 +930,7 @@ HRESULT Thread::DetachThread(BOOL fDLLThreadDetach)
         m_ThreadHandleForClose = hThread;
     }
 
+    UnregisterWaitEventLinks();
     CooperativeCleanup();
 
     // We need to make sure that TLS are touched last here.
@@ -2398,16 +2399,6 @@ Thread::~Thread()
 
     _ASSERTE(IsDead() || IsUnstarted() || IsAtProcessExit());
 
-    if (m_WaitEventLink.m_Next != NULL && !IsAtProcessExit())
-    {
-        WaitEventLink *walk = &m_WaitEventLink;
-        while (walk->m_Next) {
-            ThreadQueue::RemoveThread(this, (SyncBlock*)((DWORD_PTR)walk->m_Next->m_WaitSB & ~1));
-            StoreEventToEventStore (walk->m_Next->m_EventWait);
-        }
-        m_WaitEventLink.m_Next = NULL;
-    }
-
     if (m_StateNC & TSNC_ExistInThreadStore) {
         BOOL ret;
         ret = ThreadStore::RemoveThread(this);
@@ -2689,6 +2680,24 @@ void Thread::CleanupCOMState()
 }
 #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT
 
+void Thread::UnregisterWaitEventLinks()
+{
+    _ASSERTE(this == GetThreadNULLOk());
+
+    if (m_WaitEventLink.m_Next != NULL && !IsAtProcessExit())
+    {
+        WaitEventLink *walk = &m_WaitEventLink;
+        while (walk->m_Next)
+        {
+            ThreadQueue::RemoveThread(this, (SyncBlock*)((DWORD_PTR)walk->m_Next->m_WaitSB & ~1));
+            StoreEventToEventStore(walk->m_Next->m_EventWait);
+            walk = walk->m_Next;
+        }
+
+        m_WaitEventLink.m_Next = NULL;
+    }
+}
+
 // Thread cleanup that must be run in cooperative mode before the thread is destroyed.
 void Thread::CooperativeCleanup()
 {
@@ -2777,6 +2786,7 @@ void Thread::OnThreadTerminate(BOOL holdingLock)
 
     if (this == GetThreadNULLOk())
     {
+        UnregisterWaitEventLinks();
         CooperativeCleanup();
     }
 

src/coreclr/vm/threads.h

@@ -1180,8 +1180,11 @@ class Thread
     void            BaseWinRTUninitialize();
 #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT
 
+private:
+    void        UnregisterWaitEventLinks();
     void        CooperativeCleanup();
 
+public:
     void        OnThreadTerminate(BOOL holdingLock);
 
     static void CleanupDetachedThreads();

src/libraries/System.Threading/tests/MonitorTests.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -511,5 +513,61 @@ public static void InterruptWaitTest()
                 waitForThread();
             }
         }
+
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
+        [InlineData(false)]
+        [InlineData(true)]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public static void WaitCleanupTest(bool collectThread)
+        {
+            var lockObj = new object();
+            RunThreadAndAbandonWait(lockObj);
+            Thread.Sleep(50); // give the thread some time to actually exit
+
+            if (collectThread)
+            {
+                // Run a GC and wait for finalizers to have the managed thread object for the above thread be collected
+                GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true);
+                GC.WaitForPendingFinalizers();
+                GC.WaitForPendingFinalizers();
+            }
+
+            lock (lockObj)
+            {
+                Monitor.PulseAll(lockObj);
+            }
+
+            [MethodImpl(MethodImplOptions.NoInlining)]
+            static void RunThreadAndAbandonWait(object lockObj)
+            {
+                var t = new Thread(() =>
+                {
+                    var sc = new MonitorWaitCleanupTest_SynchronizationContext();
+                    SynchronizationContext.SetSynchronizationContext(sc);
+                    lock (lockObj)
+                    {
+                        // The thread exits during the wait override
+                        Monitor.Wait(lockObj);
+                    }
+                });
+                t.IsBackground = true;
+                t.Start();
+                t.Join();
+            }
+        }
+
+        private sealed class MonitorWaitCleanupTest_SynchronizationContext : SynchronizationContext
+        {
+            public MonitorWaitCleanupTest_SynchronizationContext() => SetWaitNotificationRequired();
+
+            public override int Wait(IntPtr[] waitHandles, bool waitAll, int millisecondsTimeout)
+            {
+                ExitThread(0);
+                return 0;
+            }
+        }
+
+        [DllImport("kernel32.dll", SetLastError = true)]
+        private static extern void ExitThread(uint exitCode);
     }
 }