Fix output race (#4911)
* Fix output race TestCommand starts the test process before wiring up stderr & stdout. This change delays process start until after the wireup is finished so that the test process cannot shut down before we have wired up output redirection. * Bypass stream forwarder when it fails to attach to a process CLI has tests failing errenously when, due to timing issues, the StreamForwarders fail to attach to a process because it managed to exit before attachment occurs. We tried attaching the forwarders prior to Process Start but this proved impossible because the OUT and ERR streams are not available to attach before the process starts. This exposes a fundamental flaw in our output redirection mechanisms. We should probably move to using https://msdn.microsoft.com/en-us/library/system.diagnostics.process.outputdatareceived.aspx or a similar mechanism. However, I don't know that @brthor hadn't considered and discarded this approach. For the time being I am attempting to make tests more deterministic by capturing the associated exceptions and moving to a different mechanism when StreamForwarders are not available. Opened https://github.com/dotnet/cli/issues/4913 to track the broader issue. * File.Copy is not atomic...
This commit is contained in:
parent
ac4f903731
commit
caad95491a
2 changed files with 108 additions and 19 deletions
|
@ -248,14 +248,12 @@ namespace Microsoft.DotNet.Cli.Utils
|
|||
|
||||
try
|
||||
{
|
||||
File.Copy(tempDepsFile, depsPath);
|
||||
File.Move(tempDepsFile, depsPath);
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
Reporter.Verbose.WriteLine($"unable to generate deps.json, it may have been already generated: {e.Message}");
|
||||
}
|
||||
finally
|
||||
{
|
||||
|
||||
try
|
||||
{
|
||||
File.Delete(tempDepsFile);
|
||||
|
|
|
@ -3,9 +3,10 @@
|
|||
|
||||
using Microsoft.DotNet.Cli.Utils;
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Diagnostics;
|
||||
using System.IO;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Runtime.InteropServices;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
|
@ -131,44 +132,134 @@ namespace Microsoft.DotNet.Tools.Test.Utilities
|
|||
|
||||
private CommandResult RunProcess(string executable, string args, StreamForwarder stdOut, StreamForwarder stdErr)
|
||||
{
|
||||
CurrentProcess = StartProcess(executable, args);
|
||||
var taskOut = stdOut.BeginRead(CurrentProcess.StandardOutput);
|
||||
var taskErr = stdErr.BeginRead(CurrentProcess.StandardError);
|
||||
Task taskOut = null;
|
||||
|
||||
Task taskErr = null;
|
||||
|
||||
CurrentProcess = CreateProcess(executable, args);
|
||||
|
||||
CurrentProcess.Start();
|
||||
|
||||
try
|
||||
{
|
||||
taskOut = stdOut.BeginRead(CurrentProcess.StandardOutput);
|
||||
}
|
||||
catch (System.InvalidOperationException e)
|
||||
{
|
||||
if (!e.Message.Equals("The collection has been marked as complete with regards to additions."))
|
||||
{
|
||||
throw;
|
||||
}
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
taskErr = stdErr.BeginRead(CurrentProcess.StandardError);
|
||||
}
|
||||
catch (System.InvalidOperationException e)
|
||||
{
|
||||
if (!e.Message.Equals("The collection has been marked as complete with regards to additions."))
|
||||
{
|
||||
throw;
|
||||
}
|
||||
}
|
||||
|
||||
CurrentProcess.WaitForExit();
|
||||
Task.WaitAll(taskOut, taskErr);
|
||||
|
||||
var tasksToAwait = new List<Task>();
|
||||
|
||||
if (taskOut != null)
|
||||
{
|
||||
tasksToAwait.Add(taskOut);
|
||||
}
|
||||
|
||||
if (taskErr != null)
|
||||
{
|
||||
tasksToAwait.Add(taskErr);
|
||||
}
|
||||
|
||||
if (tasksToAwait.Any())
|
||||
{
|
||||
Task.WaitAll(tasksToAwait.ToArray());
|
||||
}
|
||||
|
||||
var result = new CommandResult(
|
||||
CurrentProcess.StartInfo,
|
||||
CurrentProcess.ExitCode,
|
||||
stdOut.CapturedOutput,
|
||||
stdErr.CapturedOutput);
|
||||
stdOut?.CapturedOutput ?? CurrentProcess.StandardOutput.ReadToEnd(),
|
||||
stdErr?.CapturedOutput ?? CurrentProcess.StandardError.ReadToEnd());
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private Task<CommandResult> RunProcessAsync(string executable, string args, StreamForwarder stdOut, StreamForwarder stdErr)
|
||||
{
|
||||
CurrentProcess = StartProcess(executable, args);
|
||||
var taskOut = stdOut.BeginRead(CurrentProcess.StandardOutput);
|
||||
var taskErr = stdErr.BeginRead(CurrentProcess.StandardError);
|
||||
Task taskOut = null;
|
||||
|
||||
Task taskErr = null;
|
||||
|
||||
CurrentProcess = CreateProcess(executable, args);
|
||||
|
||||
CurrentProcess.Start();
|
||||
|
||||
try
|
||||
{
|
||||
taskOut = stdOut.BeginRead(CurrentProcess.StandardOutput);
|
||||
}
|
||||
catch (System.InvalidOperationException e)
|
||||
{
|
||||
if (!e.Message.Equals("The collection has been marked as complete with regards to additions."))
|
||||
{
|
||||
throw;
|
||||
}
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
taskErr = stdErr.BeginRead(CurrentProcess.StandardError);
|
||||
}
|
||||
catch (System.InvalidOperationException e)
|
||||
{
|
||||
if (!e.Message.Equals("The collection has been marked as complete with regards to additions."))
|
||||
{
|
||||
throw;
|
||||
}
|
||||
}
|
||||
|
||||
var tcs = new TaskCompletionSource<CommandResult>();
|
||||
|
||||
CurrentProcess.Exited += (sender, arg) =>
|
||||
{
|
||||
Task.WaitAll(taskOut, taskErr);
|
||||
var tasksToAwait = new List<Task>();
|
||||
|
||||
if (taskOut != null)
|
||||
{
|
||||
tasksToAwait.Add(taskOut);
|
||||
}
|
||||
|
||||
if (taskErr != null)
|
||||
{
|
||||
tasksToAwait.Add(taskErr);
|
||||
}
|
||||
|
||||
if (tasksToAwait.Any())
|
||||
{
|
||||
Task.WaitAll(tasksToAwait.ToArray());
|
||||
}
|
||||
|
||||
var result = new CommandResult(
|
||||
CurrentProcess.StartInfo,
|
||||
CurrentProcess.ExitCode,
|
||||
stdOut.CapturedOutput,
|
||||
stdErr.CapturedOutput);
|
||||
stdOut?.CapturedOutput ?? CurrentProcess.StandardOutput.ReadToEnd(),
|
||||
stdErr?.CapturedOutput ?? CurrentProcess.StandardError.ReadToEnd());
|
||||
|
||||
tcs.SetResult(result);
|
||||
};
|
||||
|
||||
return tcs.Task;
|
||||
}
|
||||
|
||||
private Process StartProcess(string executable, string args)
|
||||
private Process CreateProcess(string executable, string args)
|
||||
{
|
||||
var psi = new ProcessStartInfo
|
||||
{
|
||||
|
@ -202,7 +293,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities
|
|||
};
|
||||
|
||||
process.EnableRaisingEvents = true;
|
||||
process.Start();
|
||||
|
||||
return process;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue