Take advantage of the new Process APIs

[adamsitnik]

In this issue I am going to collect all the places where I believe SDK could benefit from using the new Process APIs (dotnet/runtime#125838 (comment)).

When I am done editing it, I am going to tag people

Process.StartAndForget (dotnet/runtime#126078)

Plenty of C# devs assume that when a Process gets disposed it's also getting killed. It's not true and we are introducing a dedicated method that is simply going to spawn the process, fetch PID and dispose the resources to prevent resource leaks.

We need to search for usages similar to this one:

sdk/src/Cli/dotnet/Commands/Clean/FileBasedAppArtifacts/CleanFileBasedAppArtifactsCommand.cs

Line 141 in 5be3937

Process.Start(startInfo);

and this one:

sdk/src/RazorSdk/Tool/ServerProtocol/ServerConnection.cs

Lines 399 to 400 in 5be3937

	var process = Process.Start(startInfo);
	processId = process.Id;

Where reference to process is not stored anywhere and/or not being used for anything and simply use the new API.

- Process.Start(startInfo); 
+_ = Process.StartAndForget (startInfo); 

Update: sent #54093 to address this

SafeProcessHandle.Signal (dotnet/runtime#126313

SafeProcessHandle was extended with Signal method that allows for signaling the process.

So usages similar to this one:

sdk/src/Dotnet.Watch/Watch/Process/ProcessRunner.cs

Line 372 in 5be3937

var error = ProcessUtilities.SendPosixSignal(state.ProcessId, signal: force ? ProcessUtilities.SIGKILL : ProcessUtilities.SIGTERM);

can be replaced with a call to process.SafeHandle.Signal(posixSignal); (in this particular case the exception handling needs to be different as well, as we the new process returns false if the process has already exited but throws Win32Exception in case it's running but we don't have the permissions to kill it (which should not be the case, since it's our own child process)

Update: done in #53920.

SafeProcessHandle.WaitForExitOrKillOnCancellationAsync (dotnet/runtime#126293)

SafeProcessHandle.WaitForExitOrKillOnCancellationAsync will allow us to ensure the process is killed once the token is cancelled.

It could be used in places similar to this one:

sdk/src/Dotnet.Watch/Watch/Process/ProcessRunner.cs

Lines 65 to 76 in 5be3937

	try
	{
	await state.Process.WaitForExitAsync(processTerminationToken);
	}
	catch (OperationCanceledException)
	{
	// Process termination requested via cancellation token.
	// Either Ctrl+C was pressed or the process is being restarted.
	// Non-cancellable to not leave orphaned processes around blocking resources:
	await TerminateProcessAsync(state.Process, processSpec, state, logger);
	}

(but in this particular case it can't be replaced 1:1 because WaitForExitOrKillOnCancellationAsync does not support graceful termination

SafeProcessHandle.WaitForExit (dotnet/runtime#126293)

SafeProcessHandle.WaitForExit is not going to wait for EOF, it's simply going to wait for the process to exit. So workarounds like this are not going to be needed.

ProcessStartInfo.KillOnParentExit (dotnet/runtime#101985)

So far the API was implemented only for Windows (dotnet/runtime#126699), but it will allow to get rid of ProcessReaper for .NET 11+ targets.

ProcessStartInfo.StandardInputHandle (dotnet/runtime#125848)

It's now possible to redirect standard handle to any SafeFileHandle, to usages like this may consider writing the blob to a file and then redirecting input to a file.

Process.Run[Async]

A high-level helper for just executing given process. We may use it places like here and here and at the same time avoid resource leaks (the process will get killed on timeout)

Process.RunAndCaptureTextOutput[Async]

Another high level helper that will do what this code does in a very performant way.

Process.ReadAllText (dotnet/runtime#126807)

Instead of trying to drain std out and err like here the new API allows for reading both at the same time using a single thread without the risk of getting into deadlock when there is a lot of data written to error.

Process.Kill(bool entireProcessTree)

We may make the Process.Kill() much faster, but when reading the code I've realized that we are not passing true here:

sdk/test/Microsoft.NET.Sdk.Publish.Tasks.Tests/EndToEnd/ProcessWrapper.cs

Line 77 in 5be3937

proc.Kill();

Update: sent #53919 to address this.

UseShellExecute

It's a very old API, but I think it should be used here instead of trying to find the right xdg-open thing.

Also, when it's not set to true, it's impossible for the Process.Start to return null. So checks like this:

sdk/src/Containers/Microsoft.NET.Build.Containers/LocalDaemons/DockerCli.cs

Lines 105 to 113 in 5be3937

	ProcessStartInfo loadInfo = new(commandPath, $"load")
	{
	RedirectStandardInput = true,
	RedirectStandardOutput = true,
	RedirectStandardError = true
	};
	using Process? loadProcess = Process.Start(loadInfo) ??
	throw new NotImplementedException(Resource.FormatString(Strings.ContainerRuntimeProcessCreationFailed, commandPath));

are invalid, because they are impossible.