Fix race when signaling waitable objects in managed implementation (#…

…55200) * TrySignalToSatisfyWait may invalidate multiple elements of the WaitedListNode linked list, if the same object occurs multiple times in a single WaitForMultipleObjects call. * Fixes #52614
dotnet · Jul 7, 2021 · 9ce467f · 9ce467f
1 parent a1bb733
commit 9ce467f
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 5 deletions.
diff --git a/...ibraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs b/...ibraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs
@@ -641,6 +641,21 @@ public WaitedListNode? Next
                     }
                 }
 
+                // Like Next, but skip nodes registered on the same thread.
+                public WaitedListNode? NextThread
+                {
+                    get
+                    {
+                        s_lock.VerifyIsLocked();
+                        WaitedListNode? ret = _next;
+                        while (ret != null && ReferenceEquals(ret._waitInfo, _waitInfo))
+                        {
+                            ret = ret._next;
+                        }
+                        return ret;
+                    }
+                }
+
                 public void RegisterWait(WaitableObject waitableObject)
                 {
                     s_lock.VerifyIsLocked();

diff --git a/...ibraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.WaitableObject.Unix.cs b/...ibraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.WaitableObject.Unix.cs
@@ -611,7 +611,7 @@ private void SignalManualResetEvent()
                     waiterNode = nextWaiterNode)
                 {
                     // Signaling a waiter will unregister the waiter node, so keep the next node before trying
-                    nextWaiterNode = waiterNode.Next;
+                    nextWaiterNode = waiterNode.NextThread;
 
                     waiterNode.WaitInfo.TrySignalToSatisfyWait(waiterNode, isAbandonedMutex: false);
                 }
@@ -635,7 +635,7 @@ private void SignalAutoResetEvent()
                 {
                     // Signaling a waiter will unregister the waiter node, but it may only abort the wait without satisfying the
                     // wait, in which case we would try to signal another waiter. So, keep the next node before trying.
-                    nextWaiterNode = waiterNode.Next;
+                    nextWaiterNode = waiterNode.NextThread;
 
                     if (waiterNode.WaitInfo.TrySignalToSatisfyWait(waiterNode, isAbandonedMutex: false))
                     {
@@ -689,7 +689,7 @@ public int SignalSemaphore(int count)
                     waiterNode = nextWaiterNode)
                 {
                     // Signaling the waiter will unregister the waiter node, so keep the next node before trying
-                    nextWaiterNode = waiterNode.Next;
+                    nextWaiterNode = waiterNode.NextThread;
 
                     if (waiterNode.WaitInfo.TrySignalToSatisfyWait(waiterNode, isAbandonedMutex: false) && --count == 0)
                     {
@@ -753,7 +753,7 @@ private void SignalMutex(bool isAbandoned)
                 {
                     // Signaling a waiter will unregister the waiter node, but it may only abort the wait without satisfying the
                     // wait, in which case we would try to signal another waiter. So, keep the next node before trying.
-                    nextWaiterNode = waiterNode.Next;
+                    nextWaiterNode = waiterNode.NextThread;
 
                     ThreadWaitInfo waitInfo = waiterNode.WaitInfo;
                     if (waitInfo.TrySignalToSatisfyWait(waiterNode, isAbandoned))

diff --git a/src/libraries/System.Threading.Tasks/tests/MethodCoverage.cs b/src/libraries/System.Threading.Tasks/tests/MethodCoverage.cs
@@ -322,7 +322,6 @@ public static void TaskConfigurableAwaiter()
         /// FromAsync testing: Not supported in .NET Native
         /// </summary>
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/52614", TestPlatforms.iOS | TestPlatforms.tvOS)]
         public static void FromAsync()
         {
             Task emptyTask = new Task(() => { });