Skip to content

Commit 4c6633a

Browse files
committed
Fix race condition in ExecuteInIsolationAsync causing net472 test host crash
The Process.Exited event fires as soon as the child process terminates, but OutputDataReceived/ErrorDataReceived callbacks can still fire afterward while the OS pipe buffers are being drained. Previously, the test awaited only the process exit, so the test method could return (and xunit could deactivate the TestOutputHelper) before all output was consumed. Late-arriving output then called logger.WriteLine() with no active test, causing xunit v3's strict TestOutputHelper to throw InvalidOperationException on a threadpool thread, crashing the test host process (exit code 7). This was exposed by the upgrade from xunit.runner.visualstudio (VSTest) to xunit.v3.mtp-v2 (MTP v2), which enforces that QueueTestOutput is only called while a test is active. Fix: track stdout/stderr EOF via TaskCompletionSources (signaled when e.Data is null) and wait for both stream completions in addition to the process exit before returning from the test.
1 parent 4a4b69b commit 4c6633a

1 file changed

Lines changed: 43 additions & 6 deletions

File tree

test/Nerdbank.Streams.Tests/TestBase.cs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,40 @@ internal Task<bool> ExecuteInIsolationAsync(string testClassName, string testMet
238238
{
239239
processExitCode.SetResult((IsolatedTestHost.ExitCodes)isolatedTestProcess.ExitCode);
240240
};
241+
242+
// Track when stdout/stderr have been fully read.
243+
// The Process.Exited event can fire before all redirected output is flushed.
244+
// If we return from the test before output is fully consumed, xunit v3's
245+
// TestOutputHelper throws InvalidOperationException ("no currently active test")
246+
// when late-arriving output is written, crashing the test host.
247+
TaskCompletionSource<bool>? stdoutDone = null;
248+
TaskCompletionSource<bool>? stderrDone = null;
241249
if (logger != null)
242250
{
243-
isolatedTestProcess.OutputDataReceived += (s, e) => logger.WriteLine(e.Data ?? string.Empty);
244-
isolatedTestProcess.ErrorDataReceived += (s, e) => logger.WriteLine(e.Data ?? string.Empty);
251+
stdoutDone = new TaskCompletionSource<bool>();
252+
stderrDone = new TaskCompletionSource<bool>();
253+
isolatedTestProcess.OutputDataReceived += (s, e) =>
254+
{
255+
if (e.Data is null)
256+
{
257+
stdoutDone.TrySetResult(true);
258+
}
259+
else
260+
{
261+
logger.WriteLine(e.Data);
262+
}
263+
};
264+
isolatedTestProcess.ErrorDataReceived += (s, e) =>
265+
{
266+
if (e.Data is null)
267+
{
268+
stderrDone.TrySetResult(true);
269+
}
270+
else
271+
{
272+
logger.WriteLine(e.Data);
273+
}
274+
};
245275
}
246276

247277
logger?.WriteLine("Test host launched with: \"{0}\" {1}", Path.GetFullPath(startInfo.FileName), startInfo.Arguments);
@@ -254,16 +284,23 @@ internal Task<bool> ExecuteInIsolationAsync(string testClassName, string testMet
254284
isolatedTestProcess.BeginErrorReadLine();
255285
}
256286

257-
return processExitCode.Task.ContinueWith(
258-
t =>
287+
// Wait for the process to exit AND for all redirected output to be consumed,
288+
// so no async output callbacks fire after the test is no longer active.
289+
Task allDone = stdoutDone != null
290+
? Task.WhenAll(processExitCode.Task, stdoutDone.Task, stderrDone!.Task)
291+
: (Task)processExitCode.Task;
292+
293+
return allDone.ContinueWith(
294+
_ =>
259295
{
260-
switch (t.Result)
296+
IsolatedTestHost.ExitCodes result = processExitCode.Task.Result;
297+
switch (result)
261298
{
262299
case IsolatedTestHost.ExitCodes.TestSkipped:
263300
throw SkipException.ForSkip("Test skipped. See output of isolated task for details.");
264301
case IsolatedTestHost.ExitCodes.TestPassed:
265302
default:
266-
Assert.Equal(IsolatedTestHost.ExitCodes.TestPassed, t.Result);
303+
Assert.Equal(IsolatedTestHost.ExitCodes.TestPassed, result);
267304
break;
268305
}
269306

0 commit comments

Comments
 (0)