Attempt to fix race condition reported in visualstudio.xunit/issues#396

xunit · Dec 18, 2023 · 0eb76d2 · 0eb76d2
1 parent 348c56d
commit 0eb76d2
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 3 deletions.
diff --git a/src/xunit.runner.utility/Sinks/ExecutionSink.cs b/src/xunit.runner.utility/Sinks/ExecutionSink.cs
@@ -32,6 +32,7 @@ public class ExecutionSink : LongLivedMarshalByRefObject, IExecutionSink
         DateTime lastTestActivity;
         volatile int skipCount;
         ManualResetEvent stopEvent;
+        bool stopRequested;
 
 #if !NET35
         readonly XElement errorsElement;
@@ -226,8 +227,7 @@ void HandleTestAssemblyFinished(MessageHandlerArgs<ITestAssemblyFinished> args)
             }
 #endif
 
-            Finished.Set();
-            stopEvent?.Set();
+            stopRequested = true;
         }
 
         void HandleTestAssemblyStarting(MessageHandlerArgs<ITestAssemblyStarting> args)
@@ -441,10 +441,20 @@ public bool OnMessageWithTypes(IMessageSinkMessage message, HashSet<string> mess
 #endif
 
             // Dispatch to the reporter handler
-            return
+            result =
                 innerSink.OnMessageWithTypes(message, messageTypes)
                 && result
                 && (options.CancelThunk == null || !options.CancelThunk());
+
+            // Don't request stop until after the inner handler has had a chance to process the message
+            // per https://github.com/xunit/visualstudio.xunit/issues/396
+            if (stopRequested)
+            {
+                Finished.Set();
+                stopEvent?.Set();
+            }
+
+            return result;
         }
 
         void SendLongRunningMessage()

diff --git a/test/test.xunit.runner.utility/Sinks/ExecutionSinkTests.cs b/test/test.xunit.runner.utility/Sinks/ExecutionSinkTests.cs
@@ -339,6 +339,34 @@ public bool OnMessage(IMessageSinkMessage message)
         }
     }
 
+    public class Messages
+    {
+        public class TestAssemblyFinished
+        {
+            [Fact]
+            public void EnsureInnerHandlerIsCalledBeforeFinishedIsSet()
+            {
+                var innerSink = Substitute.For<IMessageSinkWithTypes>();
+                var sink = new ExecutionSink(innerSink, new ExecutionSinkOptions());
+                bool? isFinishedDuringDispatch = default;
+                innerSink
+                    .OnMessageWithTypes(Arg.Any<IMessageSinkMessage>(), Arg.Any<HashSet<string>>())
+                    .Returns(callInfo =>
+                    {
+                        if (callInfo.Arg<IMessageSinkMessage>() is ITestAssemblyFinished)
+                            isFinishedDuringDispatch = sink.Finished.WaitOne(0);
+                        return true;
+                    });
+
+                sink.OnMessageWithTypes(Substitute.For<ITestAssemblyFinished>(), null);
+                var isFinishedAfterDispatch = sink.Finished.WaitOne(0);
+
+                Assert.False(isFinishedDuringDispatch);
+                Assert.True(isFinishedAfterDispatch);
+            }
+        }
+    }
+
     public class XmlCreation
     {
         readonly IMessageSinkWithTypes innerSink;