What's wrong
AsyncProcessStreamReader.Start (RunCommand/AsyncProcessStreamReader.cs:56) creates its cancellation signal as new TaskCompletionSource<bool>(), without TaskCreationOptions.RunContinuationsAsynchronously. The token registration calls TrySetResult from inside CancellationTokenSource.Cancel(), so everything waiting on that task runs synchronously on the cancelling thread:
- The
DrainOrAbandon continuation runs.
RunAsync resumes, and WaitForExitAsync(alreadyCancelledToken) throws.- The catch at
RunCommand.cs:504-512 runs TryKill, which kills the whole process tree.
ExecuteAsync faults with OperationCanceledException.- The caller's
await / catch continuation runs.
All five steps happen before Cancel() returns. For CancelAfter, they run on the timer thread.
Repro (net10.0)
The worker awaits ExecuteAsync("sleep", ["100"], …, cts.Token), and the main thread calls cts.Cancel():
caller continuation ran; insideCancel=True
Cancel() took 58.2ms on thread 4
continuation thread 4
In a second test, the cancelling thread holds a SemaphoreSlim that the worker's catch (OperationCanceledException) also needs:
baseline (Process.WaitForExitAsync(token)): Cancel() returned / OK
RunCommand: DEADLOCK: cts.Cancel() did not return within 3s
Why it matters
Callers expect Cancel() to be cheap and not to re-enter their code, and the BCL's own Process.WaitForExitAsync behaves that way. With RunCommand, a UI or service that cancels a command while holding a lock its cleanup also takes will deadlock with SemaphoreSlim or other non-reentrant locks. With Monitor, the cleanup silently re-enters the lock instead. Timer threads also end up running process-tree kills and arbitrary user code.
Suggested fix
TaskCompletionSource<bool> cancellationSource = new(TaskCreationOptions.RunContinuationsAsynchronously);
With this change applied locally, the deadlock repro prints Cancel() returned / OK, and the existing suite still passes (55 passed, 2 elevation tests skipped on Linux).
Acceptance criteria
- A test that cancels from a thread holding a
SemaphoreSlim needed by the awaiting code's catch completes without deadlock.
- A test asserting the caller's continuation does not run inside
Cancel().
What's wrong
AsyncProcessStreamReader.Start(RunCommand/AsyncProcessStreamReader.cs:56) creates its cancellation signal asnew TaskCompletionSource<bool>(), withoutTaskCreationOptions.RunContinuationsAsynchronously. The token registration callsTrySetResultfrom insideCancellationTokenSource.Cancel(), so everything waiting on that task runs synchronously on the cancelling thread:DrainOrAbandoncontinuation runs.RunAsyncresumes, andWaitForExitAsync(alreadyCancelledToken)throws.RunCommand.cs:504-512runsTryKill, which kills the whole process tree.ExecuteAsyncfaults withOperationCanceledException.await/catchcontinuation runs.All five steps happen before
Cancel()returns. ForCancelAfter, they run on the timer thread.Repro (net10.0)
The worker awaits
ExecuteAsync("sleep", ["100"], …, cts.Token), and the main thread callscts.Cancel():In a second test, the cancelling thread holds a
SemaphoreSlimthat the worker'scatch (OperationCanceledException)also needs:Why it matters
Callers expect
Cancel()to be cheap and not to re-enter their code, and the BCL's ownProcess.WaitForExitAsyncbehaves that way. With RunCommand, a UI or service that cancels a command while holding a lock its cleanup also takes will deadlock withSemaphoreSlimor other non-reentrant locks. WithMonitor, the cleanup silently re-enters the lock instead. Timer threads also end up running process-tree kills and arbitrary user code.Suggested fix
With this change applied locally, the deadlock repro prints
Cancel() returned / OK, and the existing suite still passes (55 passed, 2 elevation tests skipped on Linux).Acceptance criteria
SemaphoreSlimneeded by the awaiting code'scatchcompletes without deadlock.Cancel().