Skip to content

Commit 53600d2

Browse files
JamieMageeCopilot
andcommitted
refactor: modernize RunProcessAsync with WaitForExitAsync
Replace the legacy TaskCompletionSource + cold-Task pattern in CommandLineInvocationService.RunProcessAsync with Process.WaitForExitAsync(CancellationToken) (.NET 5+) and StreamReader.ReadToEndAsync(CancellationToken) (.NET 7+). Cancellation now kills the entire process tree and propagates OperationCanceledException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 87ae1f8 commit 53600d2

1 file changed

Lines changed: 29 additions & 39 deletions

File tree

src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -125,24 +125,21 @@ public async Task<CommandLineExecutionResult> ExecuteCommandAsync(string command
125125
return await this.ExecuteCommandAsync(command, additionalCandidateCommands, workingDirectory: null, CancellationToken.None, parameters);
126126
}
127127

128-
private static Task<CommandLineExecutionResult> RunProcessAsync(string fileName, string parameters, DirectoryInfo workingDirectory = null)
129-
{
130-
return RunProcessAsync(fileName, parameters, workingDirectory, CancellationToken.None);
131-
}
132-
133-
private static Task<CommandLineExecutionResult> RunProcessAsync(string fileName, string parameters, DirectoryInfo workingDirectory = null, CancellationToken cancellationToken = default)
128+
private static async Task<CommandLineExecutionResult> RunProcessAsync(
129+
string fileName,
130+
string parameters,
131+
DirectoryInfo workingDirectory = null,
132+
CancellationToken cancellationToken = default)
134133
{
135-
var tcs = new TaskCompletionSource<CommandLineExecutionResult>();
136-
137-
if (fileName.EndsWith(".cmd") || fileName.EndsWith(".bat"))
134+
if (fileName.EndsWith(".cmd", StringComparison.OrdinalIgnoreCase) || fileName.EndsWith(".bat", StringComparison.OrdinalIgnoreCase))
138135
{
139136
// If a script attempts to find its location using "%dp0", that can return the wrong path (current
140137
// working directory) unless the script is run via "cmd /C". An example is "ant.bat".
141138
parameters = $"/C {fileName} {parameters}";
142139
fileName = "cmd.exe";
143140
}
144141

145-
var process = new Process
142+
using var process = new Process
146143
{
147144
StartInfo =
148145
{
@@ -153,52 +150,45 @@ private static Task<CommandLineExecutionResult> RunProcessAsync(string fileName,
153150
RedirectStandardError = true,
154151
RedirectStandardOutput = true,
155152
},
156-
EnableRaisingEvents = true,
157153
};
158154

159155
if (workingDirectory != null)
160156
{
161157
process.StartInfo.WorkingDirectory = workingDirectory.FullName;
162158
}
163159

164-
var errorText = string.Empty;
165-
var stdOutText = string.Empty;
160+
process.Start();
166161

167-
var t1 = new Task(() =>
168-
{
169-
errorText = process.StandardError.ReadToEnd();
170-
});
171-
var t2 = new Task(() =>
172-
{
173-
stdOutText = process.StandardOutput.ReadToEnd();
174-
});
162+
// Read both streams concurrently to avoid deadlocks if either fills its buffer.
163+
var stdOutTask = process.StandardOutput.ReadToEndAsync(cancellationToken);
164+
var stdErrTask = process.StandardError.ReadToEndAsync(cancellationToken);
175165

176-
process.Exited += (sender, args) =>
166+
try
177167
{
178-
Task.WaitAll(t1, t2);
179-
tcs.TrySetResult(new CommandLineExecutionResult { ExitCode = process.ExitCode, StdErr = errorText, StdOut = stdOutText });
180-
process.Dispose();
181-
};
182-
183-
process.Start();
184-
t1.Start();
185-
t2.Start();
186-
187-
cancellationToken.Register(() =>
168+
await process.WaitForExitAsync(cancellationToken);
169+
}
170+
catch (OperationCanceledException)
188171
{
189172
try
190173
{
191-
process.Kill();
174+
process.Kill(entireProcessTree: true);
192175
}
193176
catch (InvalidOperationException)
194177
{
195-
// swallow invalid operations, which indicate that there is no process associated with
196-
// the process object, and therefore nothing to kill
197-
// https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.kill?view=net-8.0#system-diagnostics-process-kill
198-
return;
178+
// Process already exited.
199179
}
200-
});
201180

202-
return tcs.Task;
181+
throw;
182+
}
183+
184+
var stdOut = await stdOutTask;
185+
var stdErr = await stdErrTask;
186+
187+
return new CommandLineExecutionResult
188+
{
189+
ExitCode = process.ExitCode,
190+
StdOut = stdOut,
191+
StdErr = stdErr,
192+
};
203193
}
204194
}

0 commit comments

Comments
 (0)