Teardown disposal: release a shared resource on an admission count, not on "is anyone running?" by rbuergi · Pull Request #2238 · Systemorph/MeshWeaver

rbuergi · 2026-08-25T17:50:26Z

Fixes #2176, #2134, #2146. (#2177 / #2178 are duplicate filings of #2176; #2134 is a duplicate of the already-closed #2135, but its acquire-side remainder was still live — see below.)

Three issues, one shape: a shared resource released while a caller was still entitled to use it.

1. `IoPool` — the acquire side (#2146) is wider than the issue says

#2141 fixed the exit side: the last leaf out no longer disposes the gate and then releases it. The acquire side was untouched, and it has two windows, not one:

The counted-too-late window (IoPool: the gate can be disposed between WaitAsync completing and the leaf being counted #2146 as filed). _inFlight is raised only once WaitAsync has granted a permit. A leaf between the grant and the increment holds a permit and counts as zero — Dispose() sees an idle pool, releases _gate, and the resumed leaf calls Release() on it.
The cold-observable window, which the issue does not mention and which is the one prod actually hit. The _disposed fast path at the top of each entry point is read when the cold observable is built, while _poolCts.Token, _gate.WaitAsync and _blockingIdle.Reset() are touched on subscribe. Everything the pool returns is cold, so that gap is arbitrarily wide. A chain built during a render and subscribed as teardown lands threw ObjectDisposedException out of a reactive chain — which is exactly "Rendering failed for area Comments" on memex-cloud.

IoPoolTest.A_leaf_issued_after_disposal_is_cancelled_not_ObjectDisposed looks like it covered this and did not: it only ever built the leaf after disposal, where the fast path answers.

The fix — count admissions, not executions

Every path that may reach _gate / _poolCts / _blockingIdle — the four entry points, Drain(), and Dispose() itself — brackets its whole reach in TryEnterGateRegion() / LeaveGateRegion(), and disposal completes only at zero.

Entry is publish-then-recheck: the increment (a full fence) is published before _disposing is read, and Dispose() publishes _disposing before it reads the count. So of the two check-then-act orders at least one side always observes the other — either disposal defers because this caller is counted, or this caller refuses and answers OperationCanceledException. Never both blind.

The issue proposed guarding Release() against a disposed gate and rejected it. I agree, and did the opposite everywhere: two existing catch (ObjectDisposedException) band-aids are DELETED, because the region makes them dead code — around _poolCts.Cancel() in Dispose(), and around _poolCts.Token.Register(...) in SubscribeThroughPool. A catch there would now only hide a region that was never entered.

2. `MessageService` — an unclaimed deferral tracker, double-disposed (#2176)

The stack is exact: MessageService.cs:1872 is tracker.TimeoutCts.Cancel(), and The CancellationTokenSource has been disposed. is the CTS-specific message. The issue's two hypotheses (double HandleShutdownCore, a linked CTS) are both wrong — the phases are already RunLevel-guarded.

Five paths retire a parked deferral. Two claimed the entry atomically (TryRemove); three — NotifyStartupFailure, FailDeferredBacklog and Dispose — iterated the dictionary and cancelled + disposed each tracker IN PLACE, removing nothing until a trailing Clear(). So an already-disposed tracker stayed visible for the rest of the loop and to every other retirement path — including the deferral-timeout continuation, which fires on the ThreadPool and races shutdown by construction.

All three now go through one DrainDeferredDeliveries, where TryRemove IS the claim: exactly one caller ever owns a tracker, so cancel + dispose can only run once. The trailing Clear() is gone with it — it dropped anything added during the loop without answering it, the silent abandonment the surrounding comment exists to prevent.

Two more of the same class, fixed alongside:

The deferral-timeout continuation now claims its own (delivery, cts) pair. The dictionary is written with the indexer, so a re-deferred id replaces the entry and a by-key removal would have had a stale timer dispose the live tracker.
MessageService.Dispose() is now idempotent. MessageHub reaches it from both the ShutDown phase and the watchdog force-teardown; the second pass was re-cancelling a disposed hangDetectionCts and re-disposing the storm breaker, invisible only because both sit inside catch-and-log blocks. The IDisposable contract was being met by a swallow.

Tests — both repros verified RED on the unfixed code

Not assumed. I reverted each source file, rebuilt, and ran:

IoPoolDisposeAcquireRaceTest — 5 of 6 fail before, all 6 pass after.
Four are deterministic and thread-free: built-before-disposal, subscribed-after, one per entry point. The fifth drives the actual IoPool: the gate can be disposed between WaitAsync completing and the leaf being counted #2146 interleave — one permit, leaf A parked on a TCS, leaf B queued at the gate, dispose, then release A so the permit is handed straight to B. It failed on attempt 0.
DeferralTrackerClaimTest — fails before, passes after.
Two gates failed concurrently: FailGate runs its backlog drain outside the gate-state lock, so two unclaimed drains genuinely overlap. Pre-fix the second cancel throws ObjectDisposedException straight into the caller. The test also asserts every parked delivery still gets its terminal answer, which is what the removed Clear() used to put at risk. The backlog is parked deterministically — a probe message that passes both gate predicates proves, FIFO on the hub's single turn loop, that everything posted before it is already deferred. No sleep, no poll.

Green locally: IoPool 30/30 · MeshWeaver.Hosting.Test 231/231 · MeshWeaver.Messaging.Hub.Test 266/266 · IoPoolSiloTeardownTest 3/3 · doc link + What's New integrity 2/2. -c Release -warnaserror clean for MeshWeaver.Mesh.Contract, MeshWeaver.Messaging.Hub, MeshWeaver.Connection.Orleans and both test projects.

Docs

ControlledIoPooling.md gains "The pool releases its gate on an ADMISSION COUNT, never on 'is anyone running?'" — why CurrentInFlight and a _disposed flag both answer the wrong question, and why there is no catch (ObjectDisposedException) in the file.

What's New entry: Category: Fix — a user could notice both symptoms (an area rendering the error control during a restart, and a shutdown reporting a false failure).

🤖 Generated with Claude Code

…ot on "is anyone running?" Three issues, one shape — a resource released while a caller was still entitled to use it. ## IoPool: the acquire-side window (#2146) and the cold-observable window (#2134/#2135) #2141 fixed the EXIT side: the last leaf out no longer disposes the gate and then releases it. The acquire side was untouched, and it is wider than the issue says. `_inFlight` is raised only once `WaitAsync` has GRANTED a permit, so a leaf between the grant and the increment holds a permit and counts as zero — disposal releases `_gate` and the resumed leaf calls `Release()` on it. And the `_disposed` fast path at the top of each entry point is read when the COLD observable is BUILT, while `_poolCts.Token` / `_gate.WaitAsync` / `_blockingIdle.Reset()` are touched on SUBSCRIBE. Everything the pool returns is cold, so that gap is arbitrarily wide: a chain built during a render and subscribed as teardown lands threw ObjectDisposedException out of a reactive chain — which is exactly what prod saw as "Rendering failed for area Comments" on memex-cloud. The pool now counts ADMISSIONS, not executions. Every path that may reach those primitives — the four entry points, `Drain()`, and `Dispose()` itself — brackets its whole reach in `TryEnterGateRegion()` / `LeaveGateRegion()`, and disposal completes only at zero. Entry is publish-then-recheck (increment, then re-read the disposal flag; `Dispose` publishes the flag before it reads the count), so of the two check-then-act orders at least one side always observes the other: disposal defers, or the caller is refused and answers OperationCanceledException. Two band-aids DELETED rather than kept, because the region makes them dead: `catch (ObjectDisposedException)` around `_poolCts.Cancel()` in Dispose, and around `_poolCts.Token.Register(...)` in SubscribeThroughPool. ## MessageService: an unclaimed deferral tracker, double-disposed (#2176) `Dispose()` threw `ObjectDisposedException: The CancellationTokenSource has been disposed.` at MessageService.cs:1872 — `tracker.TimeoutCts.Cancel()` — and the hub logged "Error during shutdown of hub Store/Catalog" on a shutdown that had actually completed. Five paths retire a parked deferral. Two claimed the entry atomically (TryRemove); three — NotifyStartupFailure, FailDeferredBacklog and Dispose — iterated the dictionary and cancelled + disposed each tracker IN PLACE, removing nothing until a trailing `Clear()`. So an already-disposed tracker stayed visible for the rest of the loop and to every other retirement path, including the deferral-timeout continuation firing on the ThreadPool. All three now go through one `DrainDeferredDeliveries`, where TryRemove IS the claim: exactly one caller ever owns a tracker. The trailing `Clear()` is gone with it — it dropped anything added during the loop WITHOUT answering it, the silent abandonment the surrounding comment exists to prevent. Two more of the same class fixed alongside: the deferral-timeout continuation now claims its OWN (delivery, cts) pair (the dictionary is written with the indexer, so a re-deferred id would have had a stale timer dispose the live tracker), and `Dispose()` is idempotent — MessageHub reaches it from both the ShutDown phase and the watchdog force-teardown, and the second pass was re-cancelling a disposed hangDetectionCts behind a catch-and-log. ## Tests Both repros were verified RED on the unfixed code, not assumed: - `IoPoolDisposeAcquireRaceTest` — 5 of 6 fail before, all pass after. Four are deterministic and thread-free (built-before-disposal, subscribed-after, one per entry point). The fifth drives the actual #2146 interleave: one permit, leaf A parked on a TCS, leaf B queued at the gate, dispose, then release A so the permit is handed straight to B — it failed on attempt 0. - `DeferralTrackerClaimTest` — two gates failed concurrently (FailGate runs its backlog drain outside the gate-state lock), so two unclaimed drains overlap. Throws ObjectDisposedException straight into the caller before the fix; also asserts every parked delivery still gets its terminal answer. Full suites green: IoPool 30/30, Hosting.Test 231/231, Messaging.Hub.Test 266/266. Release -warnaserror clean for both touched projects and their dependents. Fixes #2176, #2134, #2146. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR hardens teardown/disposal behavior for two core primitives (IoPool and MessageService) so that shared resources are not released while callers are still entitled to use them, converting prior shutdown-time ObjectDisposedException failures into consistent cancellations and ensuring deferrals are retired exactly once.

Changes:

Update IoPool disposal/drain semantics to track an admission region that spans all touches of _gate/_poolCts/blocking-idle state, preventing acquire/dispose races from surfacing ObjectDisposedException.
Refactor MessageService deferred-delivery retirement to claim via TryRemove in one shared drain path, and make MessageService.Dispose() idempotent.
Add regression tests for the IoPool acquire-side window and for concurrent deferral retirement; document the behavioral contract and add a “What’s New” entry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/MeshWeaver.Messaging.Hub.Test/DeferralTrackerClaimTest.cs	Adds a concurrency repro ensuring deferred-delivery trackers are claimed before cancel/dispose and no deferred delivery is silently abandoned.
test/MeshWeaver.Hosting.Test/IoPoolDisposeAcquireRaceTest.cs	Adds deterministic + stress coverage for the “built before disposal, subscribed after” and acquire-side interleave windows.
src/MeshWeaver.Messaging.Hub/MessageService.cs	Centralizes deferred-delivery draining via `TryRemove` claims and makes `Dispose()` idempotent to avoid double-dispose races.
src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs	Introduces an admission-count region around all gate/CTS/idle-signal touches and adjusts drain/dispose sequencing to prevent teardown `ObjectDisposedException`s.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-25-teardown-disposal-races.md	Adds a release note describing the user-visible symptom reduction during shutdown/restarts.
src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md	Documents the “admission count” disposal rule and the rationale for avoiding `ObjectDisposedException` band-aids.

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

github-actions · 2026-08-25T18:08:19Z

Test Results (shard 3)

9 files ±0 9 suites ±0 6m 44s ⏱️ +6s
1 823 tests ±0 1 819 ✅ ±0 4 💤 ±0 0 ❌ ±0
2 354 runs +1 2 350 ✅ +1 4 💤 ±0 0 ❌ ±0

Results for commit ee8c919. ± Comparison against base commit e6a1945.