Repository navigation
A blocking bridge in a test wedges the whole shard: methodTimeout cannot abort a thread parked in a native wait #2013
Description
Found while diagnosing #1991's shard-2 timeout, and generalised because the signature is greppable and has almost certainly bitten before.
The mechanism
A test that bridges an observable to a blocking call — .ToEnumerable(), Rx's IObservable<T>.Wait(), .Result, .GetAwaiter().GetResult() — parks the calling thread on a semaphore until the source produces. When the source schedules onto that same thread, it self-deadlocks.
Which thread it lands on depends on the scheduler the operator picks, so it is intermittent: in #1991 one full local run passed with the offending test present, and two later runs wedged for 31 and 15 minutes at ~0% CPU.
Why it costs a whole shard rather than one test
🚨 xUnit's methodTimeout cannot abort a thread parked in a native wait. So instead of a 30 s test failure you get an unbounded host wedge:
[CI] MeshWeaver.Graph.Test exit=124 TIMEOUT (8m wall-clock cap hit — likely fixture/init hang)
— exit=124, no failing test named, and the marker's own guess ("fixture/init hang") points away from the real cause. That is why the 30 s bound appeared to have been bypassed.
The stack that identifies it:
SemaphoreSlim.Wait()
System.Reactive…ObservableImpl.GetEnumerator.MoveNext()
System.Linq.Enumerable.ToArray
<the test>
Why this is not just "the codebase bans .Wait()"
It is the same rule AGENTS.md already states for product code — async IS a deadlock; compose with .Select/.SelectMany/.Subscribe — but tests have been treated as exempt, and the consequence there is worse, not milder: product code that deadlocks fails one request, while a test that deadlocks takes the shard's remaining tests with it and reports a cause that is not the cause.
Current exposure
.ToEnumerable(): 0 remaining (the feat: presentation mode — a per-viewer, display-only privacy screen (#1803) #1991 instance is fixed on that branch)..Wait(): 37 call sites across 10+ test files —SystemScopeDoesNotEscapeTest,PostgreSqlTransientRetryTest,SyncedQueryPgTest,ContentChunkSearchTest,ChunkNavigationStoreTest,OverlayReEvaluationReadTest,NodeTypeBakeStatusTest,ThreadStreamingIdentityTest, …
Most are probably safe — e.g. pipeline.Wait().Should().Be(1) over an Observable.Return(1) completes before the wait blocks. This needs per-site judgement, not a blanket rewrite, which is exactly why it is filed rather than fixed.
What would close it
- Triage the 37: a site is safe only if the source cannot schedule onto the calling thread. Where it can, subscribe-and-collect on
ImmediateScheduler— the pattern feat: presentation mode — a per-viewer, display-only privacy screen (#1803) #1991 moved to, and the one the rest of that file already used. - Consider making the rule explicit for tests, so the next one is caught in review rather than as a wedged shard.
- Cheapest and most valuable: treat any
exit=124with no named test as "a blocking bridge in a test" first. That signature is specific enough to grep for, and today it was misattributed twice — once to CI: one Roslyn Emit poisons the whole process — every later compile NREs in Cci.MetadataWriter.GetConsolidatedTypeParameters, shard ends exit=124 #890 (Roslyn emit poisoning) and once to machine contention, both plausible and both wrong.
Related: #890 (a genuinely different exit=124, distinguished by a Cci.MetadataWriter NRE and the emit canary), #612, #1843.
🤖 Generated with Claude Code
Activity
rbuergi commentedon Aug 25, 2026
Re-triaged all 44 sites: three were real and are gone; the rest are ratcheted
PR #2259 does the triage this issue asked for (point 1) and the rule (point 2). Leaving this open
only for the residue named at the bottom.
.ToEnumerable(): 0 — confirmed, and the two remaining occurrences are comments explaining why it was removed.THREE sites were the real thing, and they are the worst possible shape
CodeEditRecompileTest,CompileSourceSnapshotWedgeTestandSourceDiscoveryUnavailableTesteachdeclared
IDisposableand did:That parks the disposing thread in a native wait on mesh teardown — the one operation in this
suite with a documented history of not completing (#613, the teardown-straggler cluster) — and does
it in
Dispose, i.e. outsidemethodTimeoutentirely, which cannot abort such a thread anyway.This is exactly the "costs a whole shard, reports
exit=124with no test named, and the markerguesses fixture/init hang" mechanism, sitting in the assembly where those markers actually appear.
All three now
public override async ValueTask DisposeAsync()andawait base.DisposeAsync().Every remaining
.Wait(): inspected, and none can park indefinitelyTwo categories, no third:
Observable.Return,Scheduler.Immediate, an NSubstitute stub returningObservable.Return(...), an in-memory fakestore, a configuration reader.
SystemScopeDoesNotEscapeTest,PostgreSqlTransientRetryTest,OverlayReEvaluationReadTest,NodeTypeBakeStatusTest,FeatureFlagReaderTest,PackageParameterResolutionTest, theContentCollections.Indexingembedder/store tests..Timeout(...)—AlcLeaseRegistryTest,NodeTypeAdoptionRegistryTest,OrleansClusterDisposal. The timeout's timer runs on the defaultscheduler, independent of the parked thread, so it always unblocks: a named
TimeoutException,never a wedge.
So the issue's own guess — "most are probably safe" — holds for the
.Wait()population. Thedanger was concentrated entirely in the
GetAwaiter().GetResult()spelling, which the originalcount of 37 did not include.
The rule, made mechanical (point 2)
BlockingBridgeInTestRatchetGuard+test/BlockingBridgeSites.allow(60 sites, 22 files) fail thebuild on a new file or a raised count, and report stale entries rather than failing them so
the list can shrink from two PRs at once. Markers:
.ToEnumerable(),.Wait(),.GetAwaiter().GetResult()..Resultis deliberately excluded — it is overwhelmingly a domainproperty here (
ToolCall.Result,PatchResult.Result) and a marker with a hundred false positivesis a marker people learn to ignore.
The scanner is extracted into
SourceScanand shared with the impersonation ratchet rather thancopied: the comment/string masker decides whether a guard measures code or its own documentation
(three test files quote these shapes verbatim while explaining why they no longer use them), and a
second subtly different copy would produce a second subtly wrong inventory.
🚨 What is NOT individually cleared — why this stays open
The
GetAwaiter().GetResult()setup helpers:watcher.StartAsync(default).GetAwaiter().GetResult()—ScheduledPostWatcherTest,OrleansScheduledPostTest,OrleansEventSubscriptionTimerTestpersistence.SaveNode(…).FirstAsync().ToTask().GetAwaiter().GetResult()—CreatableTypesIntegrationTest,MeshNodeVisibilityTests,DataContextIntegrationTest,MeshNodeVersionSyncTestCalled from synchronous setup with the xUnit sync context ambient, so they are safe only while the
awaited work happens to complete synchronously — a property of today's implementation, not a
guarantee. The allow file says so at the site. These are the lines to lower first, and the first
place to look if an
exit=124with no named test recurs.Point 3 of this issue — "treat any
exit=124with no named test as a blocking bridge first" — isnow written into the allow file's header, where someone triaging that marker will actually meet it.
rbuergi commentedon Aug 26, 2026
New occurrence, and this one was on
main— so it blocked image publication, not just a PR.Run 32939560960,
main@b55f680a9(the #2308 merge), 2026-08-26 ~07:19Z:Matches this issue's signature precisely rather than the ordinary-failure one:
Test failedannotation at all — the shard failed through the exit-marker gate, not through a trx failure. That is the distinguishing tell: the host died, so no test ever reported.exit=124— the per-projecttimeout 8mfired, i.e. nothing completed. Exactly the unbounded-wedge casemethodTimeoutcannot abort.MeshWeaver.Hosting.Monolith.Testhere vsMeshWeaver.Graph.Test), which supports this issue's "generalised, greppable, has almost certainly bitten before" framing — it is the bridge pattern, not one assembly.Why it deserves more than a flake ticket: a red
mainstops CD publishing, so this wedge has a blast radius beyond one PR's cycle.It is also categorically different from the two failures currently making noise on PRs, and should not be triaged with them:
exit=124.Timeout(...)flake populationThe one here produces no failing test at all, which is why it is easy to misfile as "CI was just slow".
Per AGENTS.md the 8m cap is not the thing to raise — over budget means stuck, and the fix is finding what never completes.
rbuergi commentedon Aug 26, 2026
Second occurrence today, and a THIRD distinct assembly — this is not assembly-specific.
Run 32943789488, PR #2318, shard 5, 2026-08-26 ~08:00Z:
Today's tally, same signature, three different assemblies:
MeshWeaver.Hosting.Monolith.Test(part 2/2)main@b55f680a9— blocked CDMeshWeaver.Threading.TestBoth
exit=124, both with no failing test recorded. That spread is the strongest evidence yet for this issue's original framing — it is the bridge pattern, not one bad test — andMeshWeaver.Threading.Testis a pointed data point given that assembly is precisely where the blocking-vs-reactive primitives live.Why it keeps getting mis-triaged: it produces no failing test name, so it does not appear in any "failed tests" summary. It reads as "CI was slow" and gets re-run. Three other failure classes are live in this repo right now and all three DO name a test, which makes this one the odd stripe that gets skipped:
exit=124.Timeout(...)flake population0 new failures, "stale allow entry"Per AGENTS.md the 8m cap is not the knob — over budget means stuck, and the fix is finding what never completes.
Merge pull request #2335 from Systemorph/fix/2013-blocking-bridge-wed…
The two new seed tests await the stream instead of blocking on it
test(#2684): the simulated CIFS stall is a cancellation-bounded spin,…
test: await the stream instead of blocking on it (BlockingBridgeInTes…