Runtime: API Proposal: WaitForExitAsync for System.Diagnostics.Process

[MattKotsenas]

Per discussion with @krwq, I'm splitting off an API proposal for a Task-based WaitForExitAsync method on System.Diagnostics.Process, since it is a building block API that is simpler to understand and use correctly, while the original issue (#12039) can continue to incubate additional convenience APIs.

Here's the proposed API:

public partial class Process
{
    public Task WaitForExitAsync(CancellationToken cancellationToken = default) { throw null; }
}

The method takes a single, optional parameter, a CancellationToken, to support timeouts, which matches WaitForExit(int timeout) semantics, while following Task-based programming best practices of allowing cancellation.

API Rationale / Design notes

API doesn't expose an WaitForExitAsync(int timeout) overload since CancellationToken has a constructor that takes a timeout, and there's CancellationToken.TimeoutAfter().

---

While the synchronous WaitForExit returns a bool to determine if the process exited or not, the async version returns a plain Task. Callers can determine that the process exited in two ways:

1. By checking the task's IsCompletedSuccessfully
2. By checking the process' HasExited

If the wait is cancelled, the caller can determine that in two ways:

1. If using await, the task will throw a TaskCanceledException
2. By checking the task's IsCanceled

---

Callers that want to emulate the old API more closely can add an extension method as follows:

public static class ProcessExtensions
{
    public static Task<bool> WaitForExitWithTimeoutAsync(this Process process, int timeout)
    {
        using (var cts = new CancellationTokenSource(timeout))
        {
            try
            {
                await process.WaitForExitAsync(cts.Token).ConfigureAwait(false);
                return process.HasExited;
            }
            catch (OperationCanceledException)
            {
                return false;
            }
        }
    }
}

This API isn't provided by the framework because:

1. It's targeted towards users converting code, rather than writing new idiomatic code
2. It increased API surface area without clear value
3. It's easy for developers to add if needed

---

Because the method internally relies on the Exited event and there's a potential race between setting EnableRaisingEvents and the process exiting, I introduced a new instance of InvalidOperationException informing the caller to set EnableRaisingEvents to make this explicit. If this is deemed undesirable coupling I can move the set into this method, at the expense of possibly throwing and catching the InvalidOperationException from GetProcessHandle(), which I'd rather avoid if possible.

Implementation

I have an implementation available here with tests to show sample usage: feature/34689-process-waitforexitasync.

[MattKotsenas]

@krwq, Thanks for tagging this! Is there anything else you need on my end for the API review?

[krwq]

@MattKotsenas, I don't believe so. We only need @terrajobst to pick this us during the API review. @terrajobst, any ETA on that?

[MattKotsenas]

Hello again! Another friendly ping here. If there's an ETA for review please let me know so I can plan accordingly. Thanks!

[danmosemsft]

@krwq you will have to talk to Immo directly probably

[krwq]

I'm also waiting on my own issues to get reviewed and have talked with him couple of times, perhaps we should have more API reviews.. - will talk to him about it

[karelz]

This is not a high-pri API (it is in Future, not in 3.0). It may need to wait couple of more weeks (or months) before we get to it ... just trying to set expectations.

[Daniel15]

Any updates on this? I'd really love to see this API in .NET Core. IMO it's one of the main pieces missing from the API.

[scalablecory]

@Daniel15: API review is being done oldest to newest, and it looks like this is pretty near the top so I'd expect the wait to not be too much longer.

[GSPP]

Would it be a good idea to also add a timeout parameter? Often, that's all you want. You don't want to deal with creating a token just for that call.

WaitForExit and WaitForExitAsync could be made symmetric. Both would get timeout and token overloads. That would be the simplest API because async or not async would be orthogonal to the feature set supported.

[terrajobst]

Video

Looks good as proposed.

C# namespace System.Diagnostics { public partial class Process { public Task WaitForExitAsync(CancellationToken cancellationToken = default); } }

[felipepessoto]

@terrajobst @krwq @MattKotsenas
Matt already sent a PR to this. But if you need help implementing it, I am available

[krwq]

@felipepessoto not sure I understand, where is the PR?

[davidfowl]

I'm so happy this is approved 😄

[felipepessoto]

@krwq, sorry, it is not a PR: https://github.com/dotnet/corefx/compare/master...MattKotsenas:feature/34689-process-waitforexitasync

[MattKotsenas]

@krwq, I'm happy to open a PR for this based on my branch.

With the new dotnet runtime repo should I move this code over to there, or is there some more automated process?

[krwq]

@MattKotsenas unfortunatelly you will need to move it, hopefully this will be just copy & paste since folder and file structure is almost the same (one extra subdirectory) but I'm not sure if there were some changes made there recently

[stephentoub]

This was implemented in https://github.com/dotnet/runtime/pull/1278.