镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

A cancelled ExecuteAsync throws while an output delegate is still running, so the caller's catch block (or the next run on the same handler) runs at the same time as it: a gap in the #95 fix #125

Open

Description

@matt-edmondson

What's wrong

bf2d00c (#95) added a promise to OutputHandler (OutputHandler.cs:12-14):

the actions are never run at the same time: each one returns before the other is called, so both may share state, such as one list collecting all of a command's output, without locking.

The promise only holds while the run finishes normally. When the token is cancelled, DrainOrAbandon returns as soon as cancelled wins (AsyncProcessStreamReader.cs:100-106). That abandons any delivery already inside onData, and nothing waits for deliveryLock before Start returns. ExecuteAsync then disposes the reader and throws OperationCanceledException (RunCommand.cs:490-508) while the delegate is still running. deliveryLock belongs to the reader, so it only serialises deliveries within one run (AsyncProcessStreamReader.cs:162-165).

The fault branch has the same gap: when one stream's delegate throws, the other stream's in-flight delivery isn't waited for either.

Repro (Linux, net10.0, current main)

int count = 0, active = 0, maxActive = 0;
void Enter() { int a = Interlocked.Increment(ref active); maxActive = Math.Max(maxActive, a); }
void Exit() => Interlocked.Decrement(ref active);

var handler = new OutputHandler(
    s => { Enter(); if (++count == 2) Thread.Sleep(1500); Exit(); },   // slow on its 2nd chunk
    s => { Enter(); Thread.Sleep(50); Exit(); });

using (var cts = new CancellationTokenSource(300))
    try { await RunCommand.ExecuteAsync("sh", ["-c", "echo run1a; sleep 0.1; echo run1b; sleep 100"], handler, cts.Token); }
    catch (OperationCanceledException) { Console.WriteLine($"run 1 threw OCE; delegates active now = {active}"); }

await RunCommand.ExecuteAsync("sh", ["-c", "for i in 1 2 3 4 5; do echo run2-$i >&2; sleep 0.1; done"], handler);
Console.WriteLine($"max concurrent delegates = {maxActive}");

Actual:

[  124ms] enter stdout run1b (active=1)
[  347ms] run 1 threw OCE; delegates active now = 1
[  351ms] enter stderr run2-1 (active=2)
[  861ms] run 2 exit 0; max concurrent delegates = 2
[ 1624ms] exit  stdout run1b

Expected: ExecuteAsync throws only after the running delegate returns. No delegate from that run starts after the throw, and maxActive is 1.

Why it matters

Suggested fix

  • Add an abandoned flag to AsyncProcessStreamReader.
  • In DrainOrAbandon's cancel and fault branches, run lock (deliveryLock) { abandoned = true; } before returning or rethrowing. This waits for an in-flight delivery to finish.
  • In ReadToEnd, check if (abandoned) return; inside the lock before calling onData.

The wait is bounded by the caller's own delegate, not by a pipe, so the #78 guarantee is unaffected. The fix may also let the "finishes into the old buffer" workaround in LineOutputHandler.Reset (#93) go.

Acceptance criteria: a test with a slow stdout delegate and a cancelled token asserts that no delegate is running when ExecuteAsync throws and that none starts afterwards. A second test asserts the same for the fault path, where the stdout delegate throws while a stderr delegate is mid-call.

Related: #95, #93, #78, #89.

Activity

  1. matt-edmondson commented on Oct 29, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions