Skip to content

Cancelling a command whose process tree has an unkillable descendant (sudo, elevated child) makes cts.Cancel() throw AggregateException, and ExecuteAsync throws it instead of OperationCanceledException #107

Description

@matt-edmondson

What's wrong

TryKill (RunCommand/RunCommand.cs:532-559) calls process.Kill(entireProcessTree: true) and catches only InvalidOperationException, Win32Exception and NotSupportedException.

When Process.Kill(bool entireProcessTree) cannot terminate one of the descendants, it does not throw a bare Win32Exception. It collects the per-process failures and throws AggregateException ("Not all processes in the process tree could be terminated", resource KillEntireProcessTree_TerminationIncomplete in System.Diagnostics.Process.dll). This happens on Unix when kill() fails with anything other than ESRCH, such as EPERM, and on Windows with ERROR_ACCESS_DENIED. None of the catches above handle it.

TryKill is called from three places, and the exception escapes from each:

  1. RunCommand.cs:476: cancellationToken.Register(() => TryKill(process)). An exception thrown by a registration callback propagates out of CancellationTokenSource.Cancel(), so the caller's cts.Cancel() throws. With CancelAfter, it is thrown on the timer thread instead.
  2. RunCommand.cs:504-512: catch (OperationCanceledException) { TryKill(process); throw; }. The AggregateException replaces the OperationCanceledException that the XML docs promise, so callers' catch (OperationCanceledException) misses it.
  3. RunCommand.cs:497: after a handler or decoder failure, the kill's AggregateException hides the handler's real exception.

When it happens

This needs a command whose process tree contains a process the caller may not signal:

  • Linux/macOS: sudo …, pkexec …, or anything else that leaves a root-owned process under a non-root caller. For example, ExecuteAsync("sh", ["-c", "sudo apt-get update"], handler, token) and then cancel.
  • Windows: a non-elevated caller whose command starts an elevated child, such as an installer or gsudo.

Expected: cts.Cancel() returns normally and ExecuteAsync throws OperationCanceledException, the same as when the whole tree can be killed. The killable parts of the tree die either way, because the runtime's tree kill carries on past individual failures.

Actual: cts.Cancel() throws AggregateException, and the awaited call faults with it.

Proof level: this is a code trace. It is not a runtime repro, because a sandboxed root environment cannot easily create a descendant the caller is forbidden to signal. The throwing path is the runtime's Kill(bool) tree-kill failure aggregation, and its resource string is present in the shipped .NET 10 System.Diagnostics.Process.dll.

Suggested fix

Treat a partially failed tree kill as best effort, like the other cases TryKill already swallows:

catch (AggregateException)
{
    // Kill(entireProcessTree) reports descendants it could not terminate (owned by another user,
    // elevated) this way; everything it could kill has already been killed.
}

Acceptance criteria

  • TryKill never throws, so RunCommand can never make cts.Cancel() throw.
  • A cancelled run whose tree contains an unkillable descendant still ends in OperationCanceledException.
  • When a handler fails and the follow-up kill only partly succeeds, the handler's exception is the one that surfaces.
  • A test pins this, for example by injecting a kill that throws AggregateException through a seam.

Activity

  1. matt-edmondson commented on Oct 5, 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

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions