Skip to content

ObserveCompletion does not cancel a pooled IIoPool leaf — cancelling the wait no longer releases the gate permit #2773

Closed as not planned

Description

@rbuergi

The regression

IoPool.InvokeStreamCore / InvokeObservable build their work with Observable.Create<T>(async (observer, subscriberCt) => …) and link the leaf's token to subscriberCt:

using var linked = CancellationTokenSource.CreateLinkedTokenSource(subscriberCt, _poolCts.Token);
var ct = linked.Token;
await _gate.WaitAsync(ct).ConfigureAwait(false);

subscriberCt fires when the subscription is disposed. So disposing the subscription is what cancels the pooled leaf and releases its gate permit.

Rx's .ToTask(ct) disposed the subscription on cancellation. ReactiveCompletion.ObserveCompletion deliberately does not — its own doc says "Cancels the WAIT, not the source. The error arm stays [attached]", and that non-disposal is the designed behaviour that lets a late fault still be reported instead of vanishing.

The consequence: at an ObserveCompletion boundary wrapping a pooled leaf, cancelling the caller no longer cancels the I/O or frees the pool slot. The leaf runs to completion holding its permit. PG pools are capped at 1 (pg:{adapter}), so the gate is the single Npgsql connection — an aborted request can leave the next caller waiting for a query nobody wants any more.

Measured blast radius — smaller than first reported

A first report put this at "core's own ~1,538 sites". That is not right, and the correction matters for prioritisation:

surface passes a real ct disposes on cancel?
ObservableAwait.Await (the 1,538 test conversions, #2750) yes ✅ yes — sub.Dispose() in its settle continuation, matching .ToTask()
ObserveCompletion in core src/ 20 of 37 sites ❌ no (by design)
…of those 20, in production code 2 ❌ no

The two production sites are both GenericUnpartitionedDataSource initial-store builds (.FirstAsync().ObserveCompletion(ReportLateInitFault, cancellationToken)). Their token is an initialization/lifetime token, so the realistic symptom is a delayed shutdown or a held slot during teardown, not steady-state exhaustion.

Proposed fix

An opt-in overload — the default must keep today's behaviour, because non-disposal is what preserves late-fault reporting:

// cancelSource: true  → also dispose the subscription, so cancellation reaches the pooled leaf
public static Task<T> ObserveCompletion<T>(
    this IObservable<T> source, Action<Exception> reportLateFault,
    CancellationToken cancellationToken = default, bool cancelSource = false)

Call sites wrapping an IIoPool leaf pass cancelSource: true; everything else is unchanged. The trade-off is explicit at each site: release the slot, or keep the late-fault arm.

Why this is filed rather than fixed inline

The choice between "release the slot" and "keep the late-fault arm" is a real per-site judgement, and getting it wrong silently loses a fault — which is the exact class of bug ObserveCompletion was written to prevent. It deserves its own change with its own test, not a rider on a conversion PR.

Found during the ToTask removal (#2748 / #2750 / Plugins #963), by review on Plugins #963 — recorded here so it does not live only in that thread.

Activity

  1. rbuergi commented on Aug 30, 2026

    @rbuergi
    ContributorAuthor

    Duplicate of #2772, which was filed first and carries the IoPool.cs evidence. I filed this before seeing it. The one thing this issue added — the corrected blast radius (the 1,538 test conversions go through ObservableAwait.Await, which DOES dispose, so only 2 production ObserveCompletion sites are exposed) — is now a comment on #2772. Closing here so there is one place to track it.

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

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions