Skip to content

Waiting for disposal is a Subscribe, never a .ToTask() — the #2301 deadlock - #2527

Merged
rbuergi merged 1 commit into
mainfrom
fix/2301-reactive-disposal
Aug 27, 2026
Merged

rbuergi merged 1 commit into
mainfrom
fix/2301-reactive-disposal

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #2301. Addresses sites 2–5 of #2488.

The mechanism: DEADLOCK, not a lost exception

await …ToTask() on a completion signal deadlocks, and that is the primary failure — the
UnobservedTaskException → HOST_CRASHED is what the deadlock does on its way out.

Rx completes a ToTask() TaskCompletionSource from inside the pipeline without
RunContinuationsAsynchronously
, so the awaiter resumes INLINE on the signalling thread. For
hub.DisposalCompleted that thread is the hub's disposal thread — and for a grain-hosted hub that
is the deactivating grain's own turn scheduler (CompleteActivation builds the hub
.WithTaskScheduler(grainScheduler)).

So OrleansGrainTeardownStragglerTest resumed onto that scheduler and then held it for 30 s,
polling the silo catalog for the very deactivation that needed the scheduler it was holding.

The signature is in the number. The failure was always exactly 30 s — the Timeout budget —
never a distribution around it, while a healthy activation leaves the catalog in 0.10 s. That is
a deadlock, not contention. It is also sticky: with no SynchronizationContext, await captures
TaskScheduler.Current, so one inline resumption routes every later await in the method
onto the same scheduler — which is why all four waits in that test are converted, not just the last.

The signal that was missing

GrainDeactivationCompleted (new, MeshWeaver.Messaging.Hub) joins the three existing
grain-lifetime callbacks on the hub configuration. Those three are what a straggler does to an
activation; this is what the activation tells anyone waiting for it to be gone.

MessageHubGrain publishes it from Orleans' own IGrainContext.Deactivated, which
ActivationData.FinishDeactivating sets on its last line — after UnregisterMessageTarget()
removed the activation from the catalog and DisposeAsync() set State=Invalid (verified by
decompiling Orleans 10.2.2, and now asserted by the test rather than assumed). AsyncSubject: fires
once, replays to late subscribers.

The 100 ms poll and its 30 s Timeout are gone. The test now asserts the catalog is empty with one
direct call — a question, not a wait.

The bridge, for signatures that are not ours

ReactiveCompletion.ObserveCompletion subscribes with an error arm and completes its task with
RunContinuationsAsynchronously — the line that stops a caller being resumed on the scheduler that
signalled. The subscription is deliberately not torn down on settle, so a fault arriving
afterwards is reported instead of orphaned.

Sites, with the deadlock question asked at each

Site whose thread was blocked verdict
OrleansGrainTeardownStragglerTest ×4 the deactivating grain's turn scheduler converted — this is #2301
MeshTeardownExtensions ×2 the mesh hub's own disposal thread, which then ran ioPools.DrainAll() — a synchronous JOIN of every pooled leaf converted
MonolithMeshTestBase.WaitWithProgressAsync same, and it is the wait every test in the suite runs converted (progress is now an Observable.Interval stopping on the signal's terminal)
SharedOrleansFixture.CleanupClientAsync the client hub's disposal thread converted
IoPoolSiloTeardown.OnStop nobody's — it composes and RETURNS kept, verified. Orleans' ILifecycleObserver genuinely declares Task OnStop(CancellationToken); the bridge is at the outermost edge, which is the rule. Comment rewritten to record that rather than cite the doc below.

Two outcomes teardown used to discard are now data: TeardownReport.DisposalFault and
TeardownReport.ActivitiesQuiesced, both logged at Error. They deliberately do not flip
Clean — whether a faulted disposal should fail a test class is an escalation decision of its
own, and folding it in here would redden the trunk for a reason unrelated to this fix. The public
WaitForDisposalAndIoDrainAsync signature is unchanged (an added optional parameter would be
binary-breaking for an already-published module).

The documentation that caused the recurrence

IMessageHub.DisposalCompleted's own remarks prescribed the crashing shape:

"At a genuine async edge (test teardown, grain deactivation) bridge once with
DisposalCompleted.FirstOrDefaultAsync() / .ToTask()."

"test teardown, grain deactivation" is exactly where this crashed — and that sentence is why four
fixes all aimed at the timeout. Replaced, leading with the deadlock. AsynchronousCalls.md gains a
matching section.

Proof

DisposalWaitBridgeTest (8 tests, deterministic, no timing) pins both properties and includes the
controls
:

  • ObserveCompletion_NeverResumesItsCallerOnTheSignallingThread — the fix;
  • TheOldToTaskBridge_ResumesItsCallerInlineOnTheSignallingThread — the control, asserting that
    .ToTask() really does resume inline on the signalling thread;
  • LateFault_AfterTheCompletionValue_IsReported — a disposal that faults after the interesting
    value has been emitted is now reported;
  • LateFault_ThroughTheOldToTaskBridge_ReachesNobody — the control: the same fault reaches
    nobody at all, not even the .Catch meant to handle it.

Local runs (Release, DOTNET_PROCESSOR_COUNT=4):

  • MeshWeaver.Hosting.Orleans.Test 215/215; the straggler test alone completes in 1 s
    (against the 30 s timeout it hit in CI).
  • MeshWeaver.Messaging.Hub.Test 298/298 (+8 new).
  • MeshWeaver.Hosting.Monolith.Test 740/743 (3 skipped by design) — the largest consumer of the
    changed MonolithMeshTestBase.
  • DocumentationLinkIntegrityTest green.

Explicitly NOT in this PR

🤖 Generated with Claude Code

…adlock

`await …ToTask()` on a completion signal deadlocks, and that is the primary
failure in #2301 (four recurrences, the fourth on main with HOST_CRASHED) —
not the unobserved exception, which is what the deadlock does on its way out.

Rx completes a `ToTask()` TaskCompletionSource from inside the pipeline WITHOUT
RunContinuationsAsynchronously, so the awaiter resumes INLINE on the signalling
thread. For `hub.DisposalCompleted` that thread is the hub's disposal thread —
which, for a grain-hosted hub, runs on the deactivating grain's OWN turn
scheduler (CompleteActivation builds the hub `.WithTaskScheduler(grainScheduler)`).
OrleansGrainTeardownStragglerTest therefore resumed onto that scheduler and then
held it for 30 s polling the silo catalog for the very deactivation that needed
it. The failure was always exactly 30 s — the Timeout budget — never a
distribution around it, while a healthy activation leaves the catalog in 0.10 s.
That shape is a deadlock, not contention. It is also sticky: with no
SynchronizationContext, `await` captures TaskScheduler.Current, so ONE inline
resumption routes every later await in the method onto the same scheduler.

The signal that was missing
  GrainDeactivationCompleted (MeshWeaver.Messaging.Hub) joins the three existing
  grain-lifetime callbacks on the hub configuration. Those are what a straggler
  DOES to an activation; this is what the activation TELLS anyone waiting for it
  to be gone. MessageHubGrain publishes it from Orleans' own
  IGrainContext.Deactivated, which ActivationData.FinishDeactivating sets on its
  LAST line — after UnregisterMessageTarget() removed the activation from the
  catalog and DisposeAsync() set State=Invalid. AsyncSubject: fires once, replays
  to late subscribers. The 100 ms catalog poll and its 30 s Timeout are gone; the
  test asserts the catalog is empty with one direct call instead of waiting for it.

The bridge, for signatures that are not ours
  ReactiveCompletion.ObserveCompletion subscribes with an error arm and completes
  its task with RunContinuationsAsynchronously — the line that stops a caller
  being resumed on the hub/grain scheduler that signalled. The subscription is
  deliberately not torn down on settle, so a fault arriving afterwards is REPORTED
  rather than orphaned.

Sites converted (deadlock question asked at each: whose thread, and does the
signal need it?)
  - MeshTeardownExtensions x2 — resumed on the mesh hub's own disposal thread and
    then ran `ioPools.DrainAll()` there, a synchronous JOIN of every pooled leaf.
    A faulted disposal and an expired activity quiesce are now DATA on
    TeardownReport (DisposalFault / ActivitiesQuiesced) and logged, not folded
    into success by `.Catch(_ => Return(Unit))`.
  - MonolithMeshTestBase.WaitWithProgressAsync — the Task-poll loop every test in
    the suite runs; progress is now an Observable.Interval stopping on the
    signal's terminal.
  - SharedOrleansFixture.CleanupClientAsync — same bridge inside a bare `catch {}`.
  - IoPoolSiloTeardown.OnStop — VERIFIED and kept. Orleans' ILifecycleObserver
    genuinely declares `Task OnStop(CancellationToken)`, and the method composes
    and RETURNS: it blocks nothing, so the bridge is at the outermost edge, which
    is the rule. Comment rewritten to record that rather than cite the old doc.

Docs
  IMessageHub.DisposalCompleted's own remarks used to prescribe the crashing
  shape ("at a genuine async edge (test teardown, grain deactivation) bridge once
  with FirstOrDefaultAsync() / .ToTask()"). That sentence is why the issue
  recurred four times with every fix aimed at the timeout. Replaced, leading with
  the deadlock. AsynchronousCalls.md gains a matching section.

Proof
  DisposalWaitBridgeTest pins both properties deterministically, including a
  CONTROL asserting that `.ToTask()` really does resume its caller inline on the
  signalling thread and that a late fault through it reaches nobody at all.

Fixes #2301. Addresses sites 2-5 of #2488 (site 1, CompilationCacheService's
SpinWait, is untouched and still open).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the teardown deadlock behind #2301 by replacing .ToTask()-based waits on reactive completion signals with a safer Task bridge, and by introducing an explicit Orleans deactivation-completed signal so callers don’t have to poll the silo catalog.

Changes:

  • Add ReactiveCompletion.ObserveCompletion(...) to bridge completion signals to Task without inline continuations, and update teardown/test paths to use it.
  • Add GrainDeactivationCompleted and publish it from MessageHubGrain via GrainContext.Deactivated, removing the catalog polling loop in OrleansGrainTeardownStragglerTest.
  • Improve teardown reporting by carrying disposal faults/quiesce outcomes as data (instead of silently swallowing them), and update docs/What’s New accordingly.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
test/MeshWeaver.Messaging.Hub.Test/DisposalWaitBridgeTest.cs New regression tests pinning the .ToTask() inline-continuation behavior vs the new bridge.
test/MeshWeaver.Hosting.Orleans.Test/OrleansGrainTeardownStragglerTest.cs Replaces .ToTask() waits + catalog polling with ObserveCompletion + deactivation signal.
src/MeshWeaver.Messaging.Hub/ReactiveCompletion.cs New Task bridge for completion signals with late-fault reporting.
src/MeshWeaver.Messaging.Hub/IMessageHub.cs Updates DisposalCompleted docs to discourage .ToTask() bridging and describe the new pattern.
src/MeshWeaver.Messaging.Hub/GrainDeactivationCompleted.cs New hub-configuration payload exposing “activation fully deactivated” as an observable.
src/MeshWeaver.Hosting.Orleans/MessageHubGrain.cs Publishes GrainDeactivationCompleted based on Orleans’ GrainContext.Deactivated.
src/MeshWeaver.Mesh.Contract/Threading/MeshTeardownSignal.cs Extends teardown reporting to carry disposal faults + activity quiesce outcome.
src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs Converts teardown waits to ObserveCompletion, improves logging/reporting semantics.
src/MeshWeaver.Hosting.Monolith.TestBase/MonolithMeshTestBase.cs Reworks disposal-progress reporting to stay subscription-driven while waiting.
src/MeshWeaver.AI.Orleans.TestBase/SharedOrleansFixture.cs Updates client cleanup to use ObserveCompletion and report outcomes instead of swallowing.
src/MeshWeaver.Connection.Orleans/IoPoolSiloTeardown.cs Expands commentary around the remaining Task boundary on ILifecycleObserver.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-teardown-reports-its-faults.md New What’s New entry documenting improved shutdown visibility.
src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md New documentation section explaining why .ToTask() is unsafe for completion signals.
Suppressed comments (2)

src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs:110

  • This return await … keeps the teardown API Task-based inside src/. For lifecycle code in this repo, it’s safer to keep the implementation fully reactive and only adapt to Task at the outermost framework edge (tests / hosted-service / Orleans lifecycle).
        mesh.Dispose();

        return await DrainAsync(
            mesh, ioPools, asyncDisposeQueue, timeout, teardownSignal, activitiesQuiesced);

src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs:186

  • This await occurs in src/ teardown code while waiting for DisposalCompleted. Awaiting Task-shaped completions in lifecycle paths can still park the wrong scheduler if invoked from a hub/grain turn; the safer repo pattern is to keep the continuation in a reactive Subscribe and avoid Task waits within the mesh turn.
            try
            {
                await mesh.DisposalCompleted.ObserveCompletion(
                    ex => logger?.LogError(ex,
                        "Mesh {Address}: disposal faulted AFTER teardown stopped waiting on it. "

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/MeshWeaver.Messaging.Hub.Test/DisposalWaitBridgeTest.cs
Comment thread test/MeshWeaver.Messaging.Hub.Test/DisposalWaitBridgeTest.cs
Comment thread src/MeshWeaver.Messaging.Hub/ReactiveCompletion.cs
Comment thread src/MeshWeaver.Messaging.Hub/ReactiveCompletion.cs
Comment thread src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs
Comment thread src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs
Comment thread src/MeshWeaver.AI.Orleans.TestBase/SharedOrleansFixture.cs
Comment thread src/MeshWeaver.Hosting.Monolith.TestBase/MonolithMeshTestBase.cs
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

1 091 tests   1 091 ✅  12m 13s ⏱️
    7 suites      0 💤
    7 files        0 ❌

Results for commit 1b8836c.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 087 tests   895 ✅  4m 56s ⏱️
    6 suites  192 💤
    6 files      0 ❌

Results for commit 1b8836c.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

    8 files      8 suites   9m 30s ⏱️
4 408 tests 4 405 ✅ 3 💤 0 ❌
5 010 runs  5 007 ✅ 3 💤 0 ❌

Results for commit 1b8836c.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 599 tests   1 598 ✅  9m 31s ⏱️
    9 suites      1 💤
    9 files        0 ❌

Results for commit 1b8836c.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

1 312 tests   1 312 ✅  5m 52s ⏱️
    6 suites      0 💤
    6 files        0 ❌

Results for commit 1b8836c.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

739 tests   735 ✅  7m 21s ⏱️
  8 suites    4 💤
  8 files      0 ❌

Results for commit 1b8836c.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

    44 files      44 suites   49m 27s ⏱️
10 234 tests 10 034 ✅ 200 💤 0 ❌
10 838 runs  10 638 ✅ 200 💤 0 ❌

Results for commit 1b8836c.

@rbuergi
rbuergi merged commit 9fba37c into main Aug 27, 2026
24 checks passed
rbuergi added a commit that referenced this pull request Aug 27, 2026
Teardown must not resolve DI after Dispose — #2527 review follow-ups
@rbuergi
rbuergi deleted the fix/2301-reactive-disposal branch August 28, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flake: OrleansGrainTeardownStragglerTest — the activation never leaves the silo catalog (4 hits on 2026-08-25, all unrelated diffs)

2 participants