Skip to content

fix(MeshNodeStreamCache): release synced-query connections on Dispose — sightings #13/#14 and the teardown chain walked (Plugins#1527) - #3737

Merged
meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/query-cache-connections-disposed-on-teardown
Sep 8, 2026
Merged

meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/query-cache-connections-disposed-on-teardown

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Tracks MeshWeaver.Plugins#1527 — MeshWeaver.FutuRe.Test SIGSEGV on two consecutive main runs (2026-09-08). Maintainer instruction: "check again disposal there … normally we access disposed objects after disposal ==> need to control that disposal chain works properly."

What the two dumps say (sightings #13 / #14, read from the ELF cores)

Both: si_signo=11 / si_code=1 (SEGV_MAPERR) / si_addr=0x0, TRAPNO=14, ERR=0x4, runtime 10.0.11, build-id 989b56df… read out of the crashed process's own libcoreclr mapping; both faulting threads are the dedicated background-GC thread, unwound along rbp, no managed frame:

RVA frame instruction source word
#13 run 34222933802 0x5cb171 (= #12) WKS::gc_heap::background_sweep()+0xa61 ← gc1 ← bgc_thread_function mov (%r15),%rax; and $-8,%rax; mov (%rax),%ecx, RAX=0 [R15] reads zero
#14 run 34222981863 0x5eb67a (new, 6th frame) WKS::gc_heap::revisit_written_page(…)+0x1aa ← revisit_written_pages ← background_mark_phase ← gc1 ← bgc_thread_function mov %r13,%rax; and $-8,%rax; mov (%rax),%ecx, R13=0 [R15] (page-aligned) reads zero

The zero-MethodTable-word family (#1–#12), not a use-after-unload (that yields a non-null unmapped pointer or a #GP on a non-canonical one) and not a managed exception (no Unwind: exception type in either job log). #14 died inside a teardown (16 DISPOSE_INVOKED / 15 DISPOSE_DONE, no DISPOSE_IOPOOL_DRAIN_START); #13 mid-test, 5.5 s after a clean teardown. All 45 teardowns across both processes reported teardown clean, leakedIoLeaves=0, no quiesce leak.

The disposal audit — one real defect, in core, and it is not the killer

MeshNodeStreamCache.Dispose() released the per-path read streams, update queues, probe cache, in-flight writes, breaker windows and pending self-writes — and never touched the synced-query chains. Each is Defer(…).SubscribeOn(TaskPoolScheduler.Default).Replay(1).AutoConnect(1): AutoConnect keeps the Connect() handle to itself, and the first subscriber only queues the upstream subscribe on the pool. No teardown phase joins a pool-queued Rx subscribe (DisposalCompleted = action blocks; IoPoolRegistry.DrainAll = IIoPool leaves; AsyncDisposeQueue = enqueued cleanup), so it ran whenever the pool reached it and resolved cacheHub.GetWorkspace() from a scope the teardown had already closed.

Measured: all 11 disposed-scope stragglers in run 34222933802's shard-1 artifact — FutuRe 2, Blazor.Views 5, ContentCollections.Indexing.Graph 4 — are that one Defer (GetQueryRaw → GetWorkspace, or its inner SyncedQueryMeshNodes.BuildReadStreamCore); the FutuRe pair landed 4 ms after DISPOSE_DONE … teardown clean. Its escalation shape when a subscriber has no error arm is Rx's rethrow → AppDomain.UnhandledException → xUnit's "Catastrophic failure … Failed: 0, exit 1" (Plugins#870 / #1390) — exit 1, not 139; and #14 crashed with no straggler at all.

The fix (src/MeshWeaver.Hosting/MeshNodeStreamCache.cs)

  • Each chain's connection is registered with the cache (AutoConnect(1, onConnect) into a CompositeDisposable).
  • Dispose() releases the registry (step 1b): a connected upstream is unsubscribed, a connect still queued on the pool is cancelled before it is dequeued, and a registration arriving after that is disposed as it is added (the late-connect race, resolved by construction). _queries / _optionsWrappedQueries are reset so a straggling GetQuery is never handed a dead chain.
  • A query opened after teardown terminates with ObjectDisposedException — the guard UpdateRaw already applies on the write side — instead of parking on a Replay(1) nothing will feed; the Defer carries the same guard for the sliver in which the pool has already started it.
  • A faulted chain's connection is released with its eviction (EvictFaultedQuery), so the registry holds live connections only.

No timeout widened, no retry, no GC setting, no catch {}; nothing async added.

The proof — test/MeshWeaver.Hosting.Test/QueryConnectionsReleasedOnTeardownTest.cs

  • MeshDispose_ReleasesEveryConnectedSyncedQuery: control arm asserts the registry sees the live connection (LiveQueryConnections == 1) BEFORE asserting Mesh.Dispose() released it.
  • AfterMeshDispose_ANewQueryTerminates_AndOpensNoUpstream: OnError of ObjectDisposedException, no upstream opened.
  • Local run (Release, fresh .trx): total=2 executed=2 passed=2 failed=0, 609 ms.
  • Negative control: with only the _queryConnections.Dispose() line removed, the control test fails at the assertion that names the release (… QueryConnectionsReleased … but found False), its own control arm still passing. Restored, both pass again.

Docs

Not merged on purpose

Merging is coordinated by the main-thread session today; this PR is left for it to land.

🤖 Generated with Claude Code

…; sightings #13/#14 + the teardown chain walked

The cache's Dispose() detached every per-path read stream, update queue,
in-flight write and probe cache — and never the synced-query chains.
Each is Defer(…).SubscribeOn(TaskPoolScheduler.Default).Replay(1)
.AutoConnect(1): AutoConnect keeps the Connect() handle to itself, and the
first subscriber only QUEUES the upstream subscribe on the pool, which no
teardown phase joins. The item then ran whenever the pool reached it and
resolved cacheHub.GetWorkspace() from a scope the teardown had closed —
all 11 disposed-scope stragglers in Plugins run 34222933802's shard-1
artifact, 4 ms after DISPOSE_DONE in FutuRe.Test.

Now: the connection is registered (AutoConnect(1, onConnect) into a
CompositeDisposable), Dispose() releases the registry (a queued connect is
cancelled, a late registration disposed as added), a query opened after
teardown terminates with ObjectDisposedException instead of parking, and a
faulted chain's connection is released with its eviction.

Pinned by QueryConnectionsReleasedOnTeardownTest (control arm + negative
control: removing only the release line fails the named assertion).

DebuggingNativeCrashes.md: sightings #13 (background_sweep+0xa61, = #12)
and #14 (revisit_written_page+0x1aa, a sixth frame) from two consecutive
main runs, both the zero-MethodTable family on the BGC thread; the
teardown chain phase by phase and the one late-touch site above.
Tracks Systemorph/MeshWeaver.Plugins#1527.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 8, 2026 14:06
@rbuergi
rbuergi marked this pull request as ready for review September 8, 2026 14:08
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge September 8, 2026 14:09

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.

🟡 Changes recommended

The query pipeline still uses an impersonation pattern (Observable.Using(...ImpersonateAsSystem...)) that can leak/latch identity across threads; it should be switched to the sealed RunAsSystem boundary as noted in review comments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens teardown behavior in MeshNodeStreamCache so synced-query upstream connections are explicitly tracked and released during cache disposal, preventing post-teardown background query work from running against already-closed scopes. It also adds a targeted regression test and records the operational context in documentation/What’s New.

Changes:

  • Track synced-query AutoConnect connection disposables and dispose them during MeshNodeStreamCache.Dispose(), plus guard GetQueryRaw to fail fast after disposal.
  • Add an integration test asserting that query connections are released on teardown and that post-dispose queries terminate rather than parking.
  • Add documentation updates (native-crash investigation notes + What’s New entry) describing the failure mode and the teardown chain.
File summaries
File Description
test/MeshWeaver.Hosting.Test/QueryConnectionsReleasedOnTeardownTest.cs New regression tests covering query-connection release and post-dispose query termination behavior.
src/MeshWeaver.Hosting/MeshNodeStreamCache.cs Implements connection registration + teardown release for synced-query chains and adds disposed-cache guards.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-08-a-shutdown-no-longer-leaves-a-query-running-behind-it.md Adds a user-visible fix note describing the shutdown behavior change.
src/MeshWeaver.Documentation/Data/Architecture/DebuggingNativeCrashes.md Records the investigation context and teardown-chain audit related to the SIGSEGV sightings.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 2594 to +2598
// Take(1) / FirstAsync after a runtime AccessAssignment
// write sees the STALE cached snapshot. AutoConnect(1)
// keeps the upstream connected forever once first subscribed.
.AutoConnect(1);
//
// 🚨 The connection is REGISTERED, not dropped (see _queryConnections): AutoConnect
Comment on lines 2527 to 2531
// Deferred + thread-pool subscribe-on + Replay(1).RefCount: a
// shared cached observable. The lambda inside Defer runs on the
// first subscriber's thread (TaskPoolScheduler thanks to
// SubscribeOn), constructs the SyncedQueryMeshNodes, and the
// Replay(1) caches its emissions for all later subscribers.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

254 tests   254 ✅  2m 18s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 2d2e821.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

494 tests   494 ✅  42s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 2d2e821.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 669 tests   1 475 ✅  2m 11s ⏱️
    4 suites    194 💤
    4 files        0 ❌

Results for commit 2d2e821.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

1 079 tests   1 079 ✅  1m 32s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 2d2e821.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    5 files      5 suites   2m 36s ⏱️
1 738 tests 1 738 ✅ 0 💤 0 ❌
1 739 runs  1 739 ✅ 0 💤 0 ❌

Results for commit 2d2e821.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 210 tests   1 210 ✅  4m 18s ⏱️
    3 suites      0 💤
    3 files        0 ❌

Results for commit 2d2e821.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test Results

   16 files     16 suites   13m 39s ⏱️
6 444 tests 6 250 ✅ 194 💤 0 ❌
6 445 runs  6 251 ✅ 194 💤 0 ❌

Results for commit 2d2e821.

@meshweaver-cloud
meshweaver-cloud Bot added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 9567a2e Sep 8, 2026
34 checks passed
@rbuergi
rbuergi deleted the fix/query-cache-connections-disposed-on-teardown branch September 13, 2026 10:19
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.

2 participants