Implement WaitForExitAsync for System.Diagnostics.Process

[MattKotsenas]

This PR implements the API proposal dotnet/corefx#34689: Add a task-based WaitForExitAsync() for the Process class.

In order to ensure that the Exited event is never lost, it is the caller's responsibility to ensure that EnableRaisingEvents is true prior to calling WaitForExitAsync(). A new InvalidOperationException with an appropriate error message is introduced to enforce those semantics.

Additionally, per feedback, WaitForExitAsync returns a Task instead of more closely matching the sync version and returning Task<bool>. To determine if the process has exited,
callers can check HasExited after the await (also cancellation follows the async pattern of throwing an OperationCanceledException and setting canceled on the task).

Unit tests for WaitForExit() were duplicated to add async versions, and one sync test was missing an assert, so I added it.

[scalablecory]

lgtm, though minor efficiency change noted. I also think the tests can be shortened a little bit.

[stephentoub]

This isn't really doing what you want it do to, especially if you add the ThrowIfCancellationRequested I suggested earlier. I think this shouldn't use a CancellationToken at all, e.g.

Process p = CreateProcessLong();
p.Start();

Task[] tasks = (from i in Enumerable.Range(0, 10) select p.WaitForExitAsync()).ToArray();
Assert.All(tasks, t => !t.IsCompleted);
p.Kill();
Task.WaitAll(tasks);

[MattKotsenas]

I apologize, but I'm not following what it's doing versus should be doing. This test is basically an aync-ified clone of SingleProcess_TryWaitMultipleTimesBeforeCompleting.

Should the two tests diverge, or is there another test that should be added in addition?

[stephentoub]

The token passed to WaitForExitAsync is already canceled prior to the call. So it's going to get only to https://github.com/dotnet/runtime/pull/1278/files#diff-06c983bcaa17665febae824710b68dbeR1397 and never get any further. If the goal is to test, for example, the event handle hookup and unhookup, this won't do that. You would instead want to create a new CancellationTokenSource, pass its token to WaitForExitAsync, then cancel the token, and then await the previously returned task.

[MattKotsenas]

Thanks for the additional info. I updated the unit test so that both cases are covered.

I also added comments to denote the different cases:

• Token is already canceled, so return synchronous
• Task is created, then token is canceled, so exercise cleanup code
• Task is awaited, then process is killed (the happy path)
• Waiting with a token on an already exited process, so return synchronous

Please let me know if there's anything else I missed. Thanks!

[MattKotsenas]

Friendly ping to reviewers:

• @carlossanlop
• @stephentoub
• @MihaZupan

Is there any additional follow up need here? Or is this OK to merge?

[scalablecory]

This looks good to me now. @stephentoub anything else?

[scalablecory]

@ericstj who is area owner here?

[ericstj]

@krwq but he's out on leave. I trust @stephentoub and @carlossanlop to review here.

[MattKotsenas]

Friendly ping to reviewers:

• @stephentoub
• @carlossanlop

Is there any additional follow up need here? Or is this OK to merge?

[stephentoub]

Thanks!