diff --git a/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs b/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs index 2ac6fb682cc2..d5d733a17044 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Diagnostics; using System.Globalization; using System.IO; @@ -57,14 +58,47 @@ public async Task RunAsync() // Note: even with 'process.StandardOutput.ReadToEndAsync()' or 'process.BeginOutputReadLine()', we ended up with // many TP threads just doing synchronous IO, slowing down the progress of the test run. // We want to read requests coming through the pipe and sending responses back to the test app as fast as possible. - var stdOutTask = Task.Factory.StartNew(static standardOutput => ((StreamReader)standardOutput!).ReadToEnd(), process.StandardOutput, TaskCreationOptions.LongRunning); - var stdErrTask = Task.Factory.StartNew(static standardError => ((StreamReader)standardError!).ReadToEnd(), process.StandardError, TaskCreationOptions.LongRunning); + // We are using ConcurrentQueue to avoid thread-safety issues for the timeout case. + // In the timeout case, we leave stdOutTask and stdErrTask running, just we stop observing them. + var stdOutBuilder = new ConcurrentQueue(); + var stdErrBuilder = new ConcurrentQueue(); - var outputAndError = await Task.WhenAll(stdOutTask, stdErrTask); + var stdOutTask = Task.Factory.StartNew(() => + { + var stdOut = process.StandardOutput; + string? currentLine; + while ((currentLine = stdOut.ReadLine()) is not null) + { + stdOutBuilder.Enqueue(currentLine); + } + }, TaskCreationOptions.LongRunning); + + var stdErrTask = Task.Factory.StartNew(() => + { + var stdErr = process.StandardError; + string? currentLine; + while ((currentLine = stdErr.ReadLine()) is not null) + { + stdErrBuilder.Enqueue(currentLine); + } + }, TaskCreationOptions.LongRunning); + + // WaitForExitAsync only waits for process exit (and doesn't wait for output) for our usage here. + // If we use BeginOutputReadLine/BeginErrorReadLine, it will also wait for output which can deadlock. await process.WaitForExitAsync(); + // At this point, process already exited. Allow for 5 seconds to consume stdout/stderr. + // We might not be able to consume all the output if the test app has exited but left a child process alive. + try + { + await Task.WhenAll(stdOutTask, stdErrTask).WaitAsync(TimeSpan.FromSeconds(5)); + } + catch (TimeoutException) + { + } + var exitCode = process.ExitCode; - _handler.OnTestProcessExited(exitCode, outputAndError[0], outputAndError[1]); + _handler.OnTestProcessExited(exitCode, string.Join(Environment.NewLine, stdOutBuilder), string.Join(Environment.NewLine, stdErrBuilder)); // This condition is to prevent considering the test app as successful when we didn't receive test session end. // We don't produce the exception if the exit code is already non-zero to avoid surfacing this exception when there is already a known failure. diff --git a/test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs b/test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs index 4b13f122d4ac..9bc513f6ff6c 100644 --- a/test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs +++ b/test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs @@ -19,6 +19,8 @@ public class SdkCommandSpec public bool RedirectStandardInput { get; set; } + public bool DisableOutputAndErrorRedirection { get; set; } + private string EscapeArgs() { // Note: this doesn't handle invoking .cmd files via "cmd /c" on Windows, which probably won't be necessary here diff --git a/test/Microsoft.NET.TestFramework/Commands/TestCommand.cs b/test/Microsoft.NET.TestFramework/Commands/TestCommand.cs index 62eafc09d6ca..56cf1002d2ae 100644 --- a/test/Microsoft.NET.TestFramework/Commands/TestCommand.cs +++ b/test/Microsoft.NET.TestFramework/Commands/TestCommand.cs @@ -22,6 +22,8 @@ public abstract class TestCommand public bool RedirectStandardInput { get; set; } + public bool DisableOutputAndErrorRedirection { get; set; } + // These only work via Execute(), not when using GetProcessStartInfo() public Action? CommandOutputHandler { get; set; } public Action? ProcessStartedHandler { get; set; } @@ -47,6 +49,12 @@ public TestCommand WithWorkingDirectory(string workingDirectory) return this; } + public TestCommand WithDisableOutputAndErrorRedirection() + { + DisableOutputAndErrorRedirection = true; + return this; + } + public TestCommand WithStandardInput(string stdin) { Debug.Assert(ProcessStartedHandler == null); @@ -107,6 +115,7 @@ private SdkCommandSpec CreateCommandSpec(IEnumerable args) } commandSpec.RedirectStandardInput = RedirectStandardInput; + commandSpec.DisableOutputAndErrorRedirection = DisableOutputAndErrorRedirection; return commandSpec; } @@ -147,24 +156,27 @@ public virtual CommandResult Execute(IEnumerable args) var spec = CreateCommandSpec(args); var command = spec - .ToCommand(_doNotEscapeArguments) - .CaptureStdOut() - .CaptureStdErr(); + .ToCommand(_doNotEscapeArguments); - command.OnOutputLine(line => + if (!spec.DisableOutputAndErrorRedirection) { - Log.WriteLine($"》{line}"); - CommandOutputHandler?.Invoke(line); - }); - - command.OnErrorLine(line => - { - Log.WriteLine($"❌{line}"); - }); - - if (StandardOutputEncoding is not null) - { - command.StandardOutputEncoding(StandardOutputEncoding); + command + .CaptureStdOut() + .CaptureStdErr() + .OnOutputLine(line => + { + Log.WriteLine($"》{line}"); + CommandOutputHandler?.Invoke(line); + }) + .OnErrorLine(line => + { + Log.WriteLine($"❌{line}"); + }); + + if (StandardOutputEncoding is not null) + { + command.StandardOutputEncoding(StandardOutputEncoding); + } } string fileToShow = Path.GetFileNameWithoutExtension(spec.FileName!).Equals("dotnet", StringComparison.OrdinalIgnoreCase) ? @@ -173,7 +185,7 @@ public virtual CommandResult Execute(IEnumerable args) var display = $"{fileToShow} {string.Join(" ", spec.Arguments)}"; Log.WriteLine($"Executing '{display}':"); - var result = ((Command)command).Execute(ProcessStartedHandler); + var result = command.Execute(ProcessStartedHandler); Log.WriteLine($"Command '{display}' exited with exit code {result.ExitCode}."); if (Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is string uploadRoot) diff --git a/test/TestAssets/TestProjects/MTPChildProcessHangTest/MTPChildProcessHangTest.csproj b/test/TestAssets/TestProjects/MTPChildProcessHangTest/MTPChildProcessHangTest.csproj new file mode 100644 index 000000000000..8699e5d53e86 --- /dev/null +++ b/test/TestAssets/TestProjects/MTPChildProcessHangTest/MTPChildProcessHangTest.csproj @@ -0,0 +1,17 @@ + + + + + $(CurrentTargetFramework) + Exe + + enable + enable + + false + + + + + + diff --git a/test/TestAssets/TestProjects/MTPChildProcessHangTest/Program.cs b/test/TestAssets/TestProjects/MTPChildProcessHangTest/Program.cs new file mode 100644 index 000000000000..30683efa1f10 --- /dev/null +++ b/test/TestAssets/TestProjects/MTPChildProcessHangTest/Program.cs @@ -0,0 +1,47 @@ +using System.Diagnostics; +using Microsoft.Testing.Platform.Builder; +using Microsoft.Testing.Platform.Capabilities.TestFramework; +using Microsoft.Testing.Platform.Extensions.TestFramework; + +if (args.Length == 1 && args[0] == "hang") +{ + var @event = new ManualResetEvent(false); + @event.WaitOne(); + return 0; +} + +var builder = await TestApplication.CreateBuilderAsync(args); +builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, _) => new MyTestFramework()); +using var testApp = await builder.BuildAsync(); +return await testApp.RunAsync(); + +internal class MyTestFramework : ITestFramework +{ + public string Uid => nameof(MyTestFramework); + public string Version => "1.0.0"; + public string DisplayName => nameof(MyTestFramework); + public string Description => DisplayName; + public Task CloseTestSessionAsync(CloseTestSessionContext context) + { + return Task.FromResult(new CloseTestSessionResult() { IsSuccess = true }); + } + public Task CreateTestSessionAsync(CreateTestSessionContext context) + => Task.FromResult(new CreateTestSessionResult() { IsSuccess = true }); + public Task ExecuteRequestAsync(ExecuteRequestContext context) + { + var fileName = Process.GetCurrentProcess().MainModule.FileName; + var p = Process.Start(new ProcessStartInfo(fileName, "hang") + { + RedirectStandardOutput = true, + RedirectStandardError = true, + }); + p.BeginOutputReadLine(); + p.BeginErrorReadLine(); + p.ErrorDataReceived += (sender, e) => { }; + p.OutputDataReceived += (sender, e) => { }; + context.Complete(); + return Task.CompletedTask; + } + public Task IsEnabledAsync() + => Task.FromResult(true); +} diff --git a/test/TestAssets/TestProjects/MTPChildProcessHangTest/global.json b/test/TestAssets/TestProjects/MTPChildProcessHangTest/global.json new file mode 100644 index 000000000000..9009caf0ba8f --- /dev/null +++ b/test/TestAssets/TestProjects/MTPChildProcessHangTest/global.json @@ -0,0 +1,5 @@ +{ + "test": { + "runner": "Microsoft.Testing.Platform" + } +} diff --git a/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs b/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs index 307e257ed6c1..dd2b8a359546 100644 --- a/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs +++ b/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs @@ -588,5 +588,23 @@ public void RunTestProjectWithEnvVariable(string configuration) result.ExitCode.Should().Be(ExitCodes.AtLeastOneTestFailed); } + + [InlineData(TestingConstants.Debug)] + [InlineData(TestingConstants.Release)] + [Theory] + public void DotnetTest_MTPChildProcessHangTestProject_ShouldNotHang(string configuration) + { + var testInstance = _testAssetsManager.CopyTestAsset("MTPChildProcessHangTest", Guid.NewGuid().ToString()) + .WithSource(); + + var result = new DotnetTestCommand(Log, disableNewOutput: false) + .WithWorkingDirectory(testInstance.Path) + // We need to disable output and error redirection so that the test infra doesn't + // hang because of a hanging child process that keeps out/err open. + .WithDisableOutputAndErrorRedirection() + .Execute("-c", configuration); + + result.ExitCode.Should().Be(ExitCodes.ZeroTests); + } } }