diff --git a/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs b/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs index dc66db9ac..7e62d1668 100644 --- a/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs +++ b/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs @@ -125,16 +125,13 @@ public async Task ExecuteCommandAsync(string command return await this.ExecuteCommandAsync(command, additionalCandidateCommands, workingDirectory: null, CancellationToken.None, parameters); } - private static Task RunProcessAsync(string fileName, string parameters, DirectoryInfo workingDirectory = null) - { - return RunProcessAsync(fileName, parameters, workingDirectory, CancellationToken.None); - } - - private static Task RunProcessAsync(string fileName, string parameters, DirectoryInfo workingDirectory = null, CancellationToken cancellationToken = default) + private static async Task RunProcessAsync( + string fileName, + string parameters, + DirectoryInfo workingDirectory = null, + CancellationToken cancellationToken = default) { - var tcs = new TaskCompletionSource(); - - if (fileName.EndsWith(".cmd") || fileName.EndsWith(".bat")) + if (fileName.EndsWith(".cmd", StringComparison.OrdinalIgnoreCase) || fileName.EndsWith(".bat", StringComparison.OrdinalIgnoreCase)) { // If a script attempts to find its location using "%dp0", that can return the wrong path (current // working directory) unless the script is run via "cmd /C". An example is "ant.bat". @@ -142,7 +139,7 @@ private static Task RunProcessAsync(string fileName, fileName = "cmd.exe"; } - var process = new Process + using var process = new Process { StartInfo = { @@ -153,7 +150,6 @@ private static Task RunProcessAsync(string fileName, RedirectStandardError = true, RedirectStandardOutput = true, }, - EnableRaisingEvents = true, }; if (workingDirectory != null) @@ -161,44 +157,38 @@ private static Task RunProcessAsync(string fileName, process.StartInfo.WorkingDirectory = workingDirectory.FullName; } - var errorText = string.Empty; - var stdOutText = string.Empty; + process.Start(); - var t1 = new Task(() => - { - errorText = process.StandardError.ReadToEnd(); - }); - var t2 = new Task(() => - { - stdOutText = process.StandardOutput.ReadToEnd(); - }); + // Read both streams concurrently to avoid deadlocks if either fills its buffer. + var stdOutTask = process.StandardOutput.ReadToEndAsync(cancellationToken); + var stdErrTask = process.StandardError.ReadToEndAsync(cancellationToken); - process.Exited += (sender, args) => + try { - Task.WaitAll(t1, t2); - tcs.TrySetResult(new CommandLineExecutionResult { ExitCode = process.ExitCode, StdErr = errorText, StdOut = stdOutText }); - process.Dispose(); - }; - - process.Start(); - t1.Start(); - t2.Start(); - - cancellationToken.Register(() => + await process.WaitForExitAsync(cancellationToken); + } + catch (OperationCanceledException) { try { - process.Kill(); + process.Kill(entireProcessTree: true); } catch (InvalidOperationException) { - // swallow invalid operations, which indicate that there is no process associated with - // the process object, and therefore nothing to kill - // https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.kill?view=net-8.0#system-diagnostics-process-kill - return; + // Process already exited. } - }); - return tcs.Task; + throw; + } + + var stdOut = await stdOutTask; + var stdErr = await stdErrTask; + + return new CommandLineExecutionResult + { + ExitCode = process.ExitCode, + StdOut = stdOut, + StdErr = stdErr, + }; } }