Kill the command and rethrow when an output read fails [patch]

[matt-edmondson]

Fixes #89

ExecuteAsync could hang forever if an output read failed. The command it started was left behind, blocked on a write.

Cause

A read fails in two ways: the handler throws, or a strict Encoding (throwOnInvalidBytes: true) rejects the bytes. Either way, that stream's ReadToEnd loop ends and nothing drains its pipe any more. Once the command fills the pipe it blocks on write(). Meanwhile:

• DrainOrAbandon waited on Task.WhenAll for both reads, and the other stream never reaches end of stream.
• RunAsync waited on Task.WhenAll(reader, WaitForExitAsync), and the process never exits.

The call never returned, and the exception never reached the caller.

Fix

• AsyncProcessStreamReader.DrainOrAbandon races the two reads and cancellation one at a time. When a read faults, it abandons both reads, reusing the existing Abandon so nothing goes unobserved. It then awaits the faulted read, so the original exception is what gets thrown.
• RunAsync awaits the reader first. If the reader throws, it calls TryKill(process) (the whole tree) and rethrows. The exit wait comes afterwards, on the success and cancellation paths only. Cancellation still ends the same way: the reader abandons its reads and returns, and WaitForExitAsync(token) throws.
• CLAUDE.md's Stream reading section now notes this behaviour.

Tests

The two new tests are the two repros from the issue. Each runs sh -c "echo $$ > pidfile; <fault>; sleep 0.5; exec head -c 1000000 /dev/zero". The exec keeps the blocked writer on the recorded pid. Each test requires three things:

• the call ends within 10 s
• it faults with the original exception, not OperationCanceledException
• the recorded process is gone, read from /proc/<pid>/stat with zombies counted as exited

The tests are:

• ExecuteAsyncShouldThrowAndKillTheCommandWhenTheOutputHandlerThrows
• ExecuteAsyncShouldThrowAndKillTheCommandWhenAStrictEncodingRejectsTheOutput

They are Linux only (Assert.Inconclusive on Windows), like the neighbouring procfs tests.

Results:

• Against main: both tests fail, hitting the 10 s bound, and the head process was still alive afterwards.
• With the fix: both pass in about 0.3 s, and passed 5 of 5 repeat runs.
• Full suite (net10.0): 53 passed, 2 skipped (the Windows-elevation self-skips).
• Build: every target builds with 0 warnings and 0 errors.

Note on #94

#94 wraps the same if (useElevation) … else … block in a try/catch (OperationCanceledException). Whichever PR merges second will get a small textual conflict there. The two changes compose: this PR's try/catch sits inside the else branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TRHs5nFW38XRh3KGTHYjE6

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud