Process: easy-to-use, reliable and efficient APIs for (some of the) common scenarios

[adamsitnik]

Background and motivation

System.Diagnostics.Process has plenty of design, usability, performance and reliablity issues. The API can not be 100% fixed without breaking changes, so I would like to introduce a set of new types related to process execution.

The motivation behind this proposal is to provide a set of easy-to-use, reliable and efficient APIs for common scenarios of process execution that do not have pitfalls of the current Process API.

If you are curious, please check this gist with an example of how easy it is to get into deadlock with the current APIs when you need to capture output.

The implementation is available here and you can give it a try.

API Proposal

These APIs do all of the heavy lifting so users don't have to:

• Kill the child process and its descendants on timeout/cancellation.
• Ensure the child process does not outlive the parent process.
• When reading OUT and ERR, both are drained to avoid deadlocks.
• Ensure the write handles provided as STD OUT/ERR are opened for sync IO, but their corresponding read handles are opened for async IO (to support cancellation on every platform).
• When reading OUT and ERR synchronously, they use multiplexing in order to avoid spawning background threads (see #81896).
• When waiting for EOF, they also monitor process exit to detect situation where the child process has started grand child that inherited the pipe handles and EOF is not signaled until all processes exit (see #51277).

namespace System.INeedName;

public static class ChildProcess
{
    // Executes the process with STD IN/OUT/ERR redirected to current process. Waits for its completion, returns exit status.
    public static ProcessExitStatus Inherit(ProcessStartOptions options, TimeSpan? timeout = default);
    public static Task<ProcessExitStatus> InheritAsync(ProcessStartOptions options, CancellationToken cancellationToken = default);

    // Executes the process with STD IN/OUT/ERR discarded. Waits for its completion, returns exit status.
    public static ProcessExitStatus Discard(ProcessStartOptions options, TimeSpan? timeout = default);
    public static Task<ProcessExitStatus> DiscardAsync(ProcessStartOptions options, CancellationToken cancellationToken = default);

    // Executes the process with STD IN/OUT/ERR redirected to specified files. Waits for its completion, returns exit status.
    public static ProcessExitStatus RedirectToFiles(ProcessStartOptions options, string? inputFile, string? outputFile, string? errorFile, TimeSpan? timeout = default);
    public static Task<ProcessExitStatus> RedirectToFilesAsync(ProcessStartOptions options, string? inputFile, string? outputFile, string? errorFile, CancellationToken cancellationToken = default);

    // Creates an instance of ProcessOutputLines to stream the output of the process.
    public static ProcessOutputLines StreamOutputLines(ProcessStartOptions options, TimeSpan? timeout = null, Encoding? encoding = null);

    // Executes the process and returns the standard output and error as strings.
    public static ProcessOutput CaptureOutput(ProcessStartOptions options, Encoding? encoding = null, SafeFileHandle? input = null, TimeSpan? timeout = null);
    public static Task<ProcessOutput> CaptureOutputAsync(ProcessStartOptions options, Encoding? encoding = null, SafeFileHandle? input = null, CancellationToken cancellationToken = default);

    // Executes the process and returns the combined output (stdout + stderr) as bytes.
    public static CombinedOutput CaptureCombined(ProcessStartOptions options, SafeFileHandle? input = null, TimeSpan? timeout = null);
    public static Task<CombinedOutput> CaptureCombinedAsync(ProcessStartOptions options, SafeFileHandle? input = null, CancellationToken cancellationToken = default);

    // Starts the process with STD IN/OUT/ERR redirected to specified handles. 
    // Does not wait for its completion, returns process id and cleans up the resources.
    public static int FireAndForget(ProcessStartOptions options, SafeFileHandle? input = null, SafeFileHandle? output = null, SafeFileHandle? error = null)
}

public readonly struct ProcessOutput
{
    public ProcessExitStatus ExitStatus { get; }
    public string StandardOutput { get; }
    public string StandardError { get; }
    public int ProcessId { get; }

    public ProcessOutput(ProcessExitStatus exitStatus, string standardOutput, string standardError, int processId);
}

public readonly struct ProcessOutputLine : IEquatable<ProcessOutputLine>
{
    public string Content { get; }
    public bool StandardError { get; }

    public ProcessOutputLine(string content, bool standardError);
}

public sealed class ProcessOutputLines : IAsyncEnumerable<ProcessOutputLine>, IEnumerable<ProcessOutputLine>
{
    public int ProcessId { get; } // Available after enumeration starts
    public ProcessExitStatus ExitStatus { get; } // Available after enumeration completes

    internal ProcessOutputLines(ProcessStartOptions options, TimeSpan? timeout, Encoding? encoding);

    public async IAsyncEnumerator<ProcessOutputLine> GetAsyncEnumerator(CancellationToken cancellationToken = default);
    public IEnumerator<ProcessOutputLine> GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator();
}

API Usage

This example runs dotnet restore, captures its output and throws an exception with standard error content when exit code is non-zero:

ProcessStartOptions info = new("dotnet")
{
    Arguments = { "restore" },
};

ProcessOutput processOutput = ChildProcess.CaptureOutput(info);
if (processOutput.ExitStatus.ExitCode != 0)
{
    throw new Exception($"dotnet restore failed: '{processOutput.StandardError}'");
}

This example streams output lines from dotnet build process as they are produced:

ProcessStartOptions info = new("dotnet")
{
    Arguments = { "build" },
};

await foreach (ProcessOutputLine line in ChildProcess.StreamOutputLines(info))
{
    if (line.StandardError)
    {
        Console.Error.WriteLine($"ERR: {line.Content}");
    }
    else
    {
        Console.WriteLine($"OUT: {line.Content}");
    }
}

Alternative Designs

The names could be longer to better reflect the purpose of the methods:

• Inherit -> RunWithInheritedIO
• Discard -> RunAndRedirectToNull
• RedirectToFiles -> RunAndRedirectToFiles
• StreamOutputLines -> RunAndStreamOutputAndErrorLines
• CaptureOutput -> RunAndCaptureOutputAndError
• CaptureCombined -> RunAndCaptureOutputAndErrorCombined

Note: Process is by default inherting the IO of the parent process. It's not always desired, as it sometimes leads to grand child processes holding pipe opened (see #51277). Some languages like golang, actually redirect to null by default. I don't believe that there is a single best default behavior, so I opted for explicit naming and forcing the users to choose.

Risks

It may be hard to discover that the class returned by StreamOutputLines implements both sync and async enumerables.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[alastairlundy]

@adamsitnik Unless I'm missing something I don't see the proposed API for ProcessExitStatus.

public sealed class ProcessOutputLines : IAsyncEnumerable, IEnumerable

Why should the ProcessOutputLines class be sealed? Ideally classes should be sealed as a last resort if not sealing them would cause major issues.

internal ProcessOutputLines(ProcessStartOptions options, TimeSpan? timeout, Encoding? encoding);

If the constructor for ProcessOutputLines is kept as internal it would be difficult for 3rd party Process related libraries to adapt to also use ProcessOutputLines. It would be helpful to have that constructor as public, or alternatively a different public constructor.

These APIs do all of the heavy lifting so users don't have to:

* Kill the child process and its descendants on timeout/cancellation.

* Ensure the child process does not outlive the parent process.

* When reading OUT and ERR, both are drained to avoid deadlocks.

* Ensure the write handles provided as STD OUT/ERR are opened for sync IO, but their corresponding read handles are opened for async IO.

Would the ChildProcess class methods also set Processor resource information after the Process starts and before it exits such as: ProcessorAffinity, Priority, PriorityBoost, MinWorkingSet etc?

The names could be longer to better reflect the purpose of the methods:

Inherit -> RunWithInheritedIO

I think RunWithInheritedIO along with the alternative API design names would be much clearer than the shorter versions.

RunAndCaptureOutputAndError could however be simplified to just RunAndCaptureOutput as it would likely be understood to also include Error information.

Do you still anticipate adding the HasStarted property to ChildProcess as discussed in #120605 ?

Thanks for the proposal.

[rolfbjarne]

// Executes the process and returns the combined output (stdout + stderr) as bytes.
public static CombinedOutput CaptureCombined(ProcessStartOptions options, SafeFileHandle? input = null, TimeSpan? timeout = null);
public static Task CaptureCombinedAsync(ProcessStartOptions options, SafeFileHandle? input = null, CancellationToken cancellationToken = default);

I was looking at the various ways of capturing stdout/stderr, and wondered if there's one of the options that can be used to implement all the other ones, and it turns out no, there isn't, you need at least two, because there's no way to capture stdout + stderr separately as bytes (which all the other options could be implemented with). Maybe that would make sense to add?

await foreach (ProcessOutputLine line in ChildProcess.StreamOutputLines(info))

Currently when listening for output using the Process class' events, the stdout and stderr events are called on different threads, so if I want to capture combined output (writing to the same StringBuilder for instance), I'll need to synchronize access.

Is this a concern here, or is the foreach guaranteed to run serially?

[adamsitnik]

Why should the ProcessOutputLines class be sealed? I

From my perspective, when we ship a sealed class it's easier to introduce changes in the future (there are fewer things to break).

If the constructor for ProcessOutputLines is kept as internal it would be difficult for 3rd party Process related libraries to adapt to also use ProcessOutputLines.

Could you please provide an example of a scenario that would be unlocked if the ctor was public?

FWIW I made sure that ProcessOutputLine and ProcessOutput provide public ctors in order to allow for easier testing/mocking.

ProcessOutputLine expected = new("Hello World", standardError: false);
Assert.Equal([expected], received); 

Would the ChildProcess class methods also set Processor resource information after the Process starts and before it exits such as: ProcessorAffinity, Priority, PriorityBoost, MinWorkingSet etc?

No. My northern star for this API is to keep it very simple and work always in exact same way on every OS (with minimal number of exceptions to this rule). The properties that you have mentioned are hard to implement for every OS and would add a lot of complexity.

Let's consider the following example:

ProcessStartOptions options = OperatingSystem.IsWindows()
    ? new("cmd") { Arguments = { "/c", "exit 42" } }
    : new("sh") { Arguments = { "-c", "exit 42" } };

This processes can exit immediately. On Unix, once we observe the process has exited and we read it's exit code/signal, the process is reaped and another brand new process can be started with the same id. We would need to ensure that we read all of the information before reaping it. To do that, we would need to perform few sys-calls and it would affect the performance for all the users, including those who don't consume this information. We could make it optional, but then we would end up in a situation similar to Process, where some APIs throw in certain situations or on specific platforms. I really want the new API to be very simple and avoid that. The users who need this information can keep using the Process API.

I also want to make it clear that the new API is not related to Diagnostics (like System.Diagnostics.Process does).

I think RunWithInheritedIO along with the alternative API design names would be much clearer than the shorter versions.

Thank you for your feedback!

Do you still anticipate adding the HasStarted property to ChildProcess as discussed in #120605 ?

No. Currently ChildProcess is static, but if we decide to introduce another Process-like type, we are going to expose only a running process. The way I imagine it is that the only way to create it would be to call Start or similar method. Or create it from some kind of SafeHandle which would throw if the process was not already running.

Again, thank you for your feedback @alastairlundy !

[adamsitnik]

I was looking at the various ways of capturing stdout/stderr, and wondered if there's one of the options that can be used to implement all the other ones, and it turns out no, there isn't, you need at least two, because there's no way to capture stdout + stderr separately as bytes (which all the other options could be implemented with). Maybe that would make sense to add?

Do I understand correctly that you would like to have the stdout and stderr returned as byte buffers? Something like this:

public readonly struct ProcessOutput
{
    public ProcessExitStatus ExitStatus { get; }
    public byte[] StandardOutput { get; }
    public byte[] StandardError { get; }
    public int ProcessId { get; }
}

In what situations you would like to have the bytes without the need to have the strings? I am asking because in the initial version of my prototype, I had it implemented like this:

public readonly struct ProcessOutput
{
    public byte[] StandardOutput { get; }
    public byte[] StandardError { get; }
    
    public string GetStandardOutputText(Encoding? encoding = null)
    {
        encoding ??= Encoding.UTF8;
        return encoding.GetString(StandardOutput );
    }

    public string GetStandardErrorText(Encoding? encoding = null)
    {
        encoding ??= Encoding.UTF8;
        return encoding.GetString(StandardError );
    }
}

But I gave up on this idea, as I assumed that too many users would call GetStandardOutputText/GetStandardErrorText more than once and not cache the result.

if (!string.IsNullOrEmpty(processOutput.GetStandardErrorText())
{
    _log.WriteError($"Process failed: {processOutput.GetStandardErrorText()}");
}

Currently when listening for output using the Process class' events, the stdout and stderr events are called on different threads, so if I want to capture combined output (writing to the same StringBuilder for instance), I'll need to synchronize access.

If you want to get the final strings, you can just call ChildProcess.CaptureOutput[Async] and it will execute the process and capture entire std out and err in a very performant way (my benchmarks show x10-x12 fewer allocations that the old API), no need to use StringBuilder.

Is this a concern here, or is the foreach guaranteed to run serially?

Items are produced one by one.

@rolfbjarne thank you for your feedback!

[rolfbjarne]

Do I understand correctly that you would like to have the stdout and stderr returned as byte buffers?

Preferably in some sort of streaming API (callbacks, foreach, etc.), because buffers can be problematic if there's a lot of output, but yes, I'd like to be able to read stdout/stderr as a stream of bytes instead of text.

In what situations you would like to have the bytes without the need to have the strings?

stdout/stderr doesn't have to be text output. Plenty of command-line tools write binary data.

[alastairlundy]

Could you please provide an example of a scenario that would be unlocked if the ctor was public?

If 3rd party libraries can't instantiate the class themselves then it means they can't use it without using ChildProcess - something that may not necessarily be desirable for them e.g. if they want to still use the Process class under the hood to be able to offer more advanced use cases than ChildProcess will handle but still return the same output type . I think having a standardized output type would be helpful and I would consider using it in my process related library if it was available and could be instantiated by me without needing to use ChildProcess.

This processes can exit immediately. On Unix, once we observe the process has exited and we read it's exit code/signal, the process is reaped and another brand new process can be started with the same id. We would need to ensure that we read all of the information before reaping it. To do that, we would need to perform few sys-calls and it would affect the performance for all the users, including those who don't consume this information. We could make it optional, but then we would end up in a situation similar to Process, where some APIs throw in certain situations or on specific platforms. I really want the new API to be very simple and avoid that. The users who need this information can keep using the Process API.

I also want to make it clear that the new API is not related to Diagnostics (like System.Diagnostics.Process does).

That's fair.

No. Currently ChildProcess is static, but if we decide to introduce another Process-like type, we are going to expose only a running process. The way I imagine it is that the only way to create it would be to call Start or similar method. Or create it from some kind of SafeHandle which would throw if the process was not already running.

That makes sense. It does make HasStarted moot.

Again, thank you for your feedback @alastairlundy !

No problem.