From b3cf53c4c0423496e92735df382fb8b563d9d461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Sun, 30 Aug 2026 18:35:22 +0200 Subject: [PATCH] =?UTF-8?q?test:=20the=20test=20tree=20holds=20zero=20hand?= =?UTF-8?q?-woven=20concurrency=20gates=20=E2=80=94=2079=20=E2=86=92=200,?= =?UTF-8?q?=20allow=20file=20deleted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ratchet added yesterday froze the debt at a MEASURED 79 sites across 23 files, every one a `ManualResetEventSlim`, 32 of them in `IoPoolTest.cs`. All 79 are converted here, so `test` moves into `HandWovenGateRatchetGuard`'s `ProductionRoots` and `test/HandWovenGateSites.allow` is deleted — there is no allow file left, for any root. ## Two shapes, chosen by which way the signal travels - **Producer → test** (a handler reporting it started, a pooled leaf, a `.Finally`, a disposal callback) → an `AsyncSubject` the producer completes, awaited through the house assertion helpers: `await x.Should().Within(...).Emit(because)`, or `.NotEmit(within)` for the negative. Never `.Wait()` on it — that swaps this ratchet for `BlockingBridgeInTestRatchetGuard`, the exact mistake made on #2786 and caught. - **Test → a worker it deliberately parks** (a wedged action block, a leaf that ignores its cancellation token, a merge turn held so an ack provably cannot arrive) → the park STAYS, because the park is the subject; the kernel handle goes. A volatile `int` the worker polls under a bounded `SpinWait.SpinUntil(predicate, budget)` — already the house idiom in these same files — returning the same `bool` the event's `Wait(timeout)` did, so sites that MEASURED whether the release arrived keep their assertion unchanged. 🚨 Every release is written in a `finally`, several of which are new. Without it an assertion that throws first leaves the worker parked for its full bound, into the next test — the two-minute pool-thread leak the allow file's own header records. No assertion weakened, no timeout widened. `Thread.Join(timeout)` and `IsSet`-style state reads were left as they are: they answer "did it finish" / "has it happened", not "wait". ## Measured before / after | | before | after | |---|---|---| | sites (guard's own scanner) | 79 | **0** | | files | 23 | 0 | | `IoPoolTest.cs` | 32 | 0 | | allow file | 79 lines | deleted | Touched: Hosting.Test (IoPoolTest 32, LiveQueryHandoffDropTest 6, LiveQueryForeignTrampolineTest 1), Messaging.Hub.Test (MessageHubTest 6, + 4 files × 2), Hosting.Monolith.Test (7 files × 2), Hosting.Orleans.Test (RoutingGrainDeliveryRetryTest 3, IoPoolSiloTeardownTest 2, RoutingGrainTurnIsolationTest 1), Graph.Test (3), Layout.Test (1), Persistence.Test (1), Autocomplete.Test (1). ## Verified - `dotnet build -c Release -warnaserror`, one project per invocation, on all 9 touched test projects → `0 Warning(s) 0 Error(s)` each. - Every converted class run by filter: 105 tests, `Passed!` with 0 failures — Hosting 30, Autocomplete 23, Messaging.Hub 21, Orleans 11, Graph 10, Monolith 8, Layout 1, Persistence 1, plus the 20 guard tests. - `HandWovenGateRatchetGuard`, `BlockingBridgeInTestRatchetGuard`, `ImpersonationScopeSiteRatchetGuard`, `ObservableToTaskBridgeGuard`, `RouterAsTestRequestOriginRatchetGuard` — all green (20/20). ## Mutation proof — the ratchet still bites, twice 1. Planted `new ManualResetEventSlim(false)` in `QueryInRenderDeadlockTest.cs` → `ProductionCodeHasNoHandWovenGate_OutsideTheSanctionedOne` **FAILED**, naming `test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs (1)`. Reverted → green. 2. Re-created `test/HandWovenGateSites.allow` → the new `TheTestTreeAllowFileStaysDeleted` **FAILED** ("…is back"). Deleted → green (4/4). Work products conserved: `Doc/Architecture/RemovingHandWovenGates` carries the two shapes, the `finally` rule, the sweep's numbers and how the ratchet is proven; AGENTS.md, the `/async` skill and `Doc/Architecture/AsynchronousCalls` now state the rule as covering `test/` too. Co-Authored-By: Claude Fable 5 --- .claude/skills/async/SKILL.md | 17 +- AGENTS.md | 6 +- .../Data/Architecture/AsynchronousCalls.md | 2 +- .../Architecture/RemovingHandWovenGates.md | 169 +++++ ...e-test-tree-holds-zero-hand-woven-gates.md | 39 + test/HandWovenGateSites.allow | 53 -- .../AutocompleteIntegrationTest.cs | 33 +- .../HandWovenGateRatchetGuard.cs | 112 ++- .../NodeTypeAdoptionRegistryTest.cs | 34 +- .../SyncedQueryRlsForeignTrampolineTest.cs | 9 +- .../DeleteNodeBehaviorTest.cs | 6 +- .../LateNackReenqueueTest.cs | 22 +- .../LayoutRenderTeardownDrainTest.cs | 30 +- .../OwnerDisposalNackTest.cs | 23 +- .../QueuedWriteAdvancesOnHandoffTest.cs | 23 +- .../ReleaseRequestWatcherHighWaterTest.cs | 120 +-- ...ShutdownRoutingRejectClassificationTest.cs | 20 +- .../WriteWaitsForCommitVerdictTest.cs | 44 +- .../IoPoolSiloTeardownTest.cs | 18 +- .../RoutingGrainDeliveryRetryTest.cs | 47 +- .../RoutingGrainTurnIsolationTest.cs | 11 +- test/MeshWeaver.Hosting.Test/IoPoolTest.cs | 707 +++++++++++------- .../LiveQueryForeignTrampolineTest.cs | 9 +- .../LiveQueryHandoffDropTest.cs | 89 ++- .../QueryInRenderDeadlockTest.cs | 10 +- .../DisposalRaceNackTest.cs | 20 +- .../HostedHubDisposalJoinTest.cs | 63 +- .../InflightHubCreationDrainTest.cs | 61 +- .../MessageHubTest.cs | 112 ++- .../TeardownHubCreationFreezeTest.cs | 24 +- ...titionObjectsSubscriberIndependenceTest.cs | 13 +- 31 files changed, 1253 insertions(+), 693 deletions(-) create mode 100644 src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-the-test-tree-holds-zero-hand-woven-gates.md delete mode 100644 test/HandWovenGateSites.allow diff --git a/.claude/skills/async/SKILL.md b/.claude/skills/async/SKILL.md index 69851c8bf9..9308d8ab6b 100644 --- a/.claude/skills/async/SKILL.md +++ b/.claude/skills/async/SKILL.md @@ -191,8 +191,8 @@ Full reference: ### 🚨🚨🚨 ABSOLUTE: no hand-woven async/concurrency primitives — not even a `SemaphoreSlim` -**A `SemaphoreSlim` (or any hand-rolled async gate / lock-for-async / signal) anywhere in `src/` is -FORBIDDEN — outside the one place sealed inside `IoPool`.** `SemaphoreSlim.WaitAsync()` +**A `SemaphoreSlim` (or any hand-rolled async gate / lock-for-async / signal) anywhere in `src/` OR +`test/` is FORBIDDEN — outside the one place sealed inside `IoPool`.** `SemaphoreSlim.WaitAsync()` blocks/parks a thread and its continuation captures the awaiting scheduler. On a hub it parks the single-threaded action block (or a grain turn) → the message you're waiting on can never be processed → **deadlock**. This is the same defect class as `async`/`await`/`Task` in hub code; a @@ -220,6 +220,19 @@ hub with `ConfigureAwait(false)`. Everywhere else, a `SemaphoreSlim` is a bug to test: if your gate runs on (or is awaited from) a hub action block / grain turn / Blazor circuit, it deadlocks — channel it through a hub or `IIoPool` instead. +🚨 **`test/` is at ZERO too, as of 2026-08-30 — there is no allow file left.** The 79 seeded +`ManualResetEventSlim` sites across 23 test files were all converted, and `HandWovenGateRatchetGuard` +now scans `test` in the same zero-tolerance root list as `src`. Two shapes, chosen by which way the +signal travels: **producer → test** is an `AsyncSubject` the producer completes +(`OnNext` then `OnCompleted`), awaited through the assertion helpers +(`await x.Should().Within(...).Emit(because)`, or `.NotEmit(within)` for the negative) — **never +`.Wait()` on it, which merely trips `BlockingBridgeInTestRatchetGuard` instead**; **test → a worker +the test deliberately parks** (a wedged action block, a leaf that ignores its token) keeps the park, +because the park IS the subject, but drops the handle: a volatile `int` polled under a bounded +`SpinWait.SpinUntil(predicate, budget)`, written in a **`finally`** so an assertion that throws +first cannot strand that worker. Full treatment: +[RemovingHandWovenGates.md](../../../src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md). + ## Rule 2 — Carry the identity across every boundary Every framework write primitive (`meshService.CreateNode/UpdateNode/DeleteNode`, diff --git a/AGENTS.md b/AGENTS.md index fc2580bfee..43f793bc6b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,9 +62,11 @@ If active bleeding genuinely needs a stopgap first, say so EXPLICITLY ("this is ## 🚨🚨🚨 ABSOLUTE: No hand-woven async/concurrency primitives — the actor model does NOT tolerate `SemaphoreSlim` -**A `SemaphoreSlim` — or any hand-rolled async gate, lock-for-async, or signal (`TaskCompletionSource` as a gate, a `Task.Delay` timeout race, `ManualResetEventSlim`, `lock`-around-`await`) — anywhere in `src/` is FORBIDDEN, outside the one place sealed inside `IoPool`.** It parks the single-threaded action block (or grain turn), so the message you are waiting on can never be processed → deadlock. **Serialization channels through the hub** (a `Subject` + `.Select(Run).Concat().Subscribe(...)`, or `GetMeshNodeStream(path).Update(...)`), and **concurrency bounding / one-shot init channels through `IIoPool`** — `pool.Run(...)` held in an *instance* `PromiseCache`/`PromiseSlot`, never a `SemaphoreSlim(1,1)` and never a bare `ConcurrentDictionary>` (a ReplaySubject latches `OnError` and replays one transient fault forever, #1369). +**A `SemaphoreSlim` — or any hand-rolled async gate, lock-for-async, or signal (`TaskCompletionSource` as a gate, a `Task.Delay` timeout race, `ManualResetEventSlim`, `lock`-around-`await`) — anywhere in `src/` **or `test/`** is FORBIDDEN, outside the one place sealed inside `IoPool`.** It parks the single-threaded action block (or grain turn), so the message you are waiting on can never be processed → deadlock. **Serialization channels through the hub** (a `Subject` + `.Select(Run).Concat().Subscribe(...)`, or `GetMeshNodeStream(path).Update(...)`), and **concurrency bounding / one-shot init channels through `IIoPool`** — `pool.Run(...)` held in an *instance* `PromiseCache`/`PromiseSlot`, never a `SemaphoreSlim(1,1)` and never a bare `ConcurrentDictionary>` (a ReplaySubject latches `OnError` and replays one transient fault forever, #1369). -Full reference: [/async](.claude/skills/async/SKILL.md) · [ControlledIoPooling.md](src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md) · memory `feedback_no_semaphoreslim`. +🚨 **`test/` reached ZERO on 2026-08-30 and the allow file is DELETED** — every root is now scanned with no escape hatch. In a test the two shapes are: a producer→test signal is an `AsyncSubject` the producer completes, awaited through the assertion helpers (`await x.Should().Within(...).Emit(because)` / `.NotEmit(within)`) — **never `.Wait()` on it**, which only trips `BlockingBridgeInTestRatchetGuard` instead; and a release INTO a worker the test deliberately parks (the park being the subject) is a volatile `int` polled under a bounded `SpinWait.SpinUntil`, written in a **`finally`** so a failing assertion cannot strand it. + +Full reference: [/async](.claude/skills/async/SKILL.md) · [RemovingHandWovenGates.md](src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md) · [ControlledIoPooling.md](src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md) · memory `feedback_no_semaphoreslim`. ## 🚨🚨🚨 ABSOLUTE: Never hand-roll UI / data-binding / persistence / submit — use the framework diff --git a/src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md b/src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md index 67105e7c71..4f2fc434f4 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md @@ -675,7 +675,7 @@ If you find yourself using `.Take(N)` outside a `SelectMany` that immediately pr 9. **🚨 The async boundary lives at the real I/O edge — defer it as deep as possible.** `async` / `await` / `IAsyncEnumerable` bridge across a *genuine* I/O wait (Postgres, file-system, network). In-memory work is never async: anything that only touches in-process state projects synchronously and lifts to `IObservable` via `IEnumerable.ToObservable()`. An `async IAsyncEnumerable` method that never awaits I/O is a bug. Only the leaf that actually performs I/O is allowed to be async, and it bridges back to the observable contract at one sealed point (`Observable.Create` + `await foreach`), pooling at that edge via the shared **`IIoPool`** governor (`MeshWeaver.Mesh.Threading`). Litmus test: before writing `async`, name the I/O it awaits — if you can't, delete it and return `IObservable`. Full treatment: [Aggregating Providers → "The async boundary lives at the I/O edge"](/Doc/Architecture/AggregatingProviders). -10. **🚨 No hand-woven async/concurrency primitives — the actor model does NOT tolerate `SemaphoreSlim`.** A `SemaphoreSlim` / hand-rolled async gate / lock-for-async / `TaskCompletionSource`-as-a-signal / `ManualResetEventSlim` / `Task.Delay`-timeout-race anywhere in `src/` is FORBIDDEN, outside the one sealed inside `IoPool`. `WaitAsync()` blocks/parks a thread and its continuation captures the awaiting scheduler — on a hub it parks the single-threaded action block (or grain turn) so the message you're waiting on can never be processed → **deadlock** (the lock-shaped twin of rule 1). Serialization ("one at a time") is what the hub action block gives for free — channel through it: `Subject` + `.Select(Run).Concat().Subscribe(...)` (canonical: `KernelExecutor`'s REPL queue, which *replaced* a `SemaphoreSlim`; same shape: `RoutingServiceBase.ActivationSerializer`), or `GetMeshNodeStream(path).Update(...)`. Concurrency bound / one-shot init / connect handshake → `IIoPool` + the promise-cache (`pool.Run(...)` in an instance `ConcurrentDictionary`), never a `SemaphoreSlim(1,1)` `_initLock`/`_connectGate`. The ONLY sanctioned `SemaphoreSlim` is sealed inside `IoPool` (the off-hub I/O boundary). Full treatment: [ControlledIoPooling](/Doc/Architecture/ControlledIoPooling). +10. **🚨 No hand-woven async/concurrency primitives — the actor model does NOT tolerate `SemaphoreSlim`.** A `SemaphoreSlim` / hand-rolled async gate / lock-for-async / `TaskCompletionSource`-as-a-signal / `ManualResetEventSlim` / `Task.Delay`-timeout-race anywhere in `src/` **or `test/`** is FORBIDDEN, outside the one sealed inside `IoPool`. `WaitAsync()` blocks/parks a thread and its continuation captures the awaiting scheduler — on a hub it parks the single-threaded action block (or grain turn) so the message you're waiting on can never be processed → **deadlock** (the lock-shaped twin of rule 1). Serialization ("one at a time") is what the hub action block gives for free — channel through it: `Subject` + `.Select(Run).Concat().Subscribe(...)` (canonical: `KernelExecutor`'s REPL queue, which *replaced* a `SemaphoreSlim`; same shape: `RoutingServiceBase.ActivationSerializer`), or `GetMeshNodeStream(path).Update(...)`. Concurrency bound / one-shot init / connect handshake → `IIoPool` + the promise-cache (`pool.Run(...)` in an instance `ConcurrentDictionary`), never a `SemaphoreSlim(1,1)` `_initLock`/`_connectGate`. The ONLY sanctioned `SemaphoreSlim` is sealed inside `IoPool` (the off-hub I/O boundary). **In a TEST** the same primitive strands a deliberately blocked worker whenever an assertion throws before the release runs, so `test/` is held at zero too, with no allow file: a producer→test signal is an `AsyncSubject` awaited through the assertion helpers, and a release INTO a worker the test parks on purpose is a volatile flag polled under a bounded `SpinWait.SpinUntil`, written in a `finally`. Full treatment: [ControlledIoPooling](/Doc/Architecture/ControlledIoPooling) · [Removing Hand-Woven Concurrency Gates](/Doc/Architecture/RemovingHandWovenGates). --- diff --git a/src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md b/src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md new file mode 100644 index 0000000000..39a365c76f --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md @@ -0,0 +1,169 @@ +--- +Name: Removing Hand-Woven Concurrency Gates +Category: Architecture +Description: The two shapes that replace a ManualResetEventSlim in a test, why the release must live in a finally, what the 79-site sweep measured, and the ratchet that now holds every root at zero with no allow file. +Icon: +--- + +# Removing Hand-Woven Concurrency Gates + +A **hand-woven concurrency gate** is `SemaphoreSlim`, `ManualResetEventSlim`, `ManualResetEvent`, +`AutoResetEvent`, `CountdownEvent` or `Monitor.Wait` — anything that parks a thread until another +party signals it. [Asynchronous Calls](../AsynchronousCalls) carries the rule; this page is the +operational companion: **what to write instead**, **why the `finally` is load-bearing**, and **what +the sweep that emptied `test/` actually measured**. + +## Why a gate in a TEST is a defect, not a style choice + +In product code the failure is obvious: the parked thread is a single-threaded hub action block or +a grain turn, so the message the gate is waiting on can never be processed — deadlock. + +In a test the same primitive fails more quietly, and it has bitten here twice: + +- **A stranded worker.** `IoPoolResidualNamesItsPoolTest` put its `release.Set()` *after* an + `await` of the method-timeout token. On timeout that line never ran, so the leaf it had + deliberately blocked kept a pool thread for two full minutes — **into the next test**, which then + failed for reasons that had nothing to do with it. +- **A ratchet swap.** The first repair traded the event for an `AsyncSubject` plus `.Wait()`. That + is an observable→blocking bridge, which parks the thread just the same and trips + `BlockingBridgeInTestRatchetGuard` instead. Satisfying one guard by violating another is not a + fix. + +## The two shapes, and how to tell them apart + +Ask which way the signal travels. + +### 1. Producer → test: an `AsyncSubject` the producer completes + +A handler, a pooled leaf, a render body, a `.Finally(...)`, a `Disposable.Create(...)` — anything +inside the system telling the test "I got here". + +```csharp +var entered = new AsyncSubject(); + +pool.InvokeBlocking(_ => +{ + entered.OnNext(Unit.Default); + entered.OnCompleted(); // AsyncSubject replays after completion — a late awaiter still sees it + … +}); + +await entered.Should().Within(5.Seconds()).Emit("the leaf must be running before the drain"); +``` + +The negative direction — *nothing* should have happened yet — is the same subject with `NotEmit`, +which is the one place a fixed wait is correct because there is no positive signal to await: + +```csharp +await fired.Should().NotEmit(300.Milliseconds(), + "Disposed must not fire while the leaf is still running"); +``` + +🚨 **Never `.Wait()` the subject.** `await …Should()…Emit()` suspends the test; `.Wait()` parks its +thread and is the very defect the other ratchet exists for. Where an assertion helper does not fit, +`await x.Timeout(...).Await(ct)` (`ObservableAwait.Await`, in `MeshWeaver.Messaging.Hub`) is the one +sanctioned wait. + +Making the test `async Task` to accommodate this is normal and expected; several `void` tests were +converted in exactly that way. + +### 2. Test → a worker it deliberately parks: a volatile flag under a bounded `SpinUntil` + +Half of these tests exist *because* something blocks: a wedged action block, "a leaf that ignores +its cancellation token", a subscribe that never returns, a merge turn held open so an ack provably +cannot arrive. **The park stays — it is the subject.** What goes is the kernel handle: + +```csharp +var releaseGate = 0; + +primary.Update(_ => +{ + gateEntered.OnNext(Unit.Default); + gateEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseGate) == 1, TimeSpan.FromSeconds(60)); + return null; +}, _ => { }); + +try +{ + await gateEntered.Should().Within(10.Seconds()).Emit("the turn must be parked before the write"); + … + Volatile.Write(ref releaseGate, 1); + … +} +finally +{ + // 🚨 THE LOAD-BEARING LINE + Volatile.Write(ref releaseGate, 1); +} +``` + +`SpinWait.SpinUntil(predicate, timeout)` returns the same `bool` the event's `Wait(timeout)` did, so +a site that *measured* whether the release arrived (rather than merely waiting for it) keeps its +assertion unchanged. It allocates no handle, needs no `using`, and — like the event — never pumps a +`SynchronizationContext`, which matters where the test's whole point is that the blocked thread must +**not** run a queued continuation. + +### 3. "Did that thread finish inside a budget?" → `Thread.Join(timeout)` + +Not an event. `Join` returns only on real termination, whereas an event fired from a `finally` +signals *before* the thread has actually ended (#2792). + +### 4. No signal back at all, where the block is the whole subject + +When a test needs only "this leaf outlives the drain budget", a bounded `Thread.Sleep` **is** the +subject and needs no release — see `IoPoolResidualNamesItsPoolTest`. Prefer this when it does not +cost wall-clock time; prefer the flag when the release must be prompt. + +## 🚨 The release goes in a `finally` + +Every one of the ~20 converted parks in this sweep writes its release flag in a `finally`, and +several gained a `try` that was not there before. This is not tidiness: without it, an assertion +that throws *before* the release leaves a worker parked for its full bound — 20, 30, 60 seconds — and +that hold outlives the test. Where a helper owns the park (`WriteWaitsForCommitVerdictTest`'s +`OwnerGate`), `Dispose()` releases and the call site uses `using var gate = await Park…(path)`, which +gives the same guarantee. + +Idempotence is what makes this safe: a mid-test `Volatile.Write(ref flag, 1)` followed by the same +write in the `finally` is harmless, so the happy path stays readable. + +## What the sweep measured + +Seeded 2026-08-30 by the guard's own scanner (comments and string literals masked, so a doc comment +naming the ban is not a site): + +| | | +|---|---| +| Sites | **79 → 0** | +| Files | 23 | +| Primitive | `ManualResetEventSlim`, every one | +| Largest single file | `test/MeshWeaver.Hosting.Test/IoPoolTest.cs` — 32 | +| Projects | Hosting, Hosting.Monolith, Hosting.Orleans, Messaging.Hub, Layout, Graph, Persistence, Autocomplete | + +`src/`, `tools/`, `samples/`, `clients/` and `memex/` were already at zero and stayed there. + +## The ratchet after the sweep + +`HandWovenGateRatchetGuard` now runs **one tier, not two**. `test` sits in `ProductionRoots` +alongside every other root, held at zero **with no allow file anywhere** — there is nothing to +append a line to, which is the strongest form this rule can take. Two things keep that honest: + +- `TheTestTreeAllowFileStaysDeleted` fails if `test/HandWovenGateSites.allow` reappears. A cleared + inventory that is quietly re-seeded is exactly how one grows back, and the allow-file reader is + explicit that a *regenerated* file blesses whatever happens to be in the tree. +- `TheScannerSeesWhatItClaimsTo` plants a real temp tree and runs the **real** scan over it — + proving the scanner sees a genuine gate, sees a `.csx` script, and does **not** see a gate named + in a comment or a string literal. A guard whose self-test only exercises its regex can lose half + its scope and stay green. + +The only surviving exemptions are the two **verified** `SanctionedGates`: the one sealed inside +`IoPool` (the mesh's single async/IO boundary) and the standalone `ThumbnailGenerator` CLI, whose +exemption is re-checked every run against the premise it rests on — that its csproj references no +MeshWeaver assembly, so there is no hub in that process to park. + +## Related + +- [Asynchronous Calls](../AsynchronousCalls) — the rule itself, and the no-`async` contract it belongs to +- [Removing Observable-to-Task Bridges](../RemovingObservableToTaskBridges) — the sibling ratchet, and why `.Wait()` is not an escape from this one +- [Controlled I/O Pooling](../ControlledIoPooling) — where concurrency bounding legitimately lives +- [Writing Tests](../WritingTests) — the house rules the converted assertions follow diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-the-test-tree-holds-zero-hand-woven-gates.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-the-test-tree-holds-zero-hand-woven-gates.md new file mode 100644 index 0000000000..c1d1778eb7 --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-the-test-tree-holds-zero-hand-woven-gates.md @@ -0,0 +1,39 @@ +--- +Name: The test tree holds zero hand-woven concurrency gates, and the allow file is gone +Category: Feature +Description: All 79 seeded ManualResetEventSlim sites across 23 test files are converted to reactive signals and bounded volatile releases, so `test` now sits in the ratchet's zero-tolerance roots with no allow file left to append a line to. +Icon: Sparkle +Order: -20260830 +--- + +# The test tree holds zero hand-woven concurrency gates, and the allow file is gone + +A hand-woven gate — `ManualResetEventSlim` and its relatives — parks a thread until somebody +signals it. In product code that thread is a hub action block or a grain turn, and the deadlock is +immediate. In a **test** the same primitive fails more quietly: when an assertion throws before the +release runs, the worker it deliberately blocked stays blocked for its full bound and bleeds into +the next test. That is not hypothetical — one such site held a pool thread for two minutes and made +an unrelated test fail. + +The ratchet added a day earlier froze that debt at a measured **79 sites across 23 files**, every +one a `ManualResetEventSlim`, 32 of them in `IoPoolTest.cs`. All 79 are now converted, and +`test/HandWovenGateSites.allow` is deleted. + +Two shapes replaced them, chosen by which way the signal travels: + +- **Producer → test** (a handler reporting that it started, a pooled leaf, a `.Finally`, a + disposal callback) becomes an `AsyncSubject` the producer completes, awaited through the + house assertion helpers — `await x.Should().Within(...).Emit(because)`, or `NotEmit(within)` for + the negative case. That suspends the test instead of parking its thread, so it cannot self-deadlock + and cannot strand anything. +- **Test → a worker it deliberately parks** — a wedged action block, a leaf that ignores its + cancellation token, a merge turn held open so an ack provably cannot arrive — keeps the park, + because the park *is* the subject, but drops the kernel handle: the worker polls a volatile flag + under a bounded `SpinWait.SpinUntil`, and the release is written in a **`finally`** so a failing + assertion can never leave it held. + +No assertion was weakened and no timeout widened; several tests gained a `try`/`finally` they did +not have. `test` now sits in the guard's zero-tolerance roots alongside `src/`, `tools/`, `samples/`, +`clients/` and `memex/`, held at zero **with no allow file anywhere** — and a new check fails if the +deleted one ever comes back. The full account of the two shapes, the traps, and what the sweep +measured is in [Removing Hand-Woven Concurrency Gates](/Doc/Architecture/RemovingHandWovenGates). diff --git a/test/HandWovenGateSites.allow b/test/HandWovenGateSites.allow deleted file mode 100644 index 226b7ef8c1..0000000000 --- a/test/HandWovenGateSites.allow +++ /dev/null @@ -1,53 +0,0 @@ -# HandWovenGateSites.allow — the SEEDED inventory of hand-woven concurrency gates -# (SemaphoreSlim / ManualResetEventSlim / AutoResetEvent / CountdownEvent / Monitor.Wait), -# for HandWovenGateRatchetGuard. One `relative/path.cscount` per line; `#` starts a comment. -# -# 🚨 THIS LIST MAY ONLY SHRINK. Adding a line is not a fix — the guard fails on a new file, a -# raised count, or a raised TOTAL, and TotalBudget in the guard must be lowered by the same -# amount whenever an entry is deleted or lowered. -# -# It covers ONLY `test/`. `src/`, `tools/`, `samples/`, `clients/` and `memex/` are held at ZERO -# with NO allow file at all — the sole production gate is the one sealed inside IoPool, which the -# guard VERIFIES rather than lists (it fails if that file stops containing one). There is -# deliberately no line here you could add for them. -# -# 🚨 WHY THESE ARE DEBT AND NOT STYLE. A parked thread on a hub action block or a grain turn is a -# deadlock the mesh cannot recover from. In a TEST the same primitive is subtler: it strands a -# blocked worker when an assertion throws before the release runs. Measured 2026-08-30 — -# IoPoolResidualNamesItsPoolTest put `release.Set()` after an `await` of the method-timeout token, -# so on timeout the release never ran and the leaf held a pool thread for two minutes, into the -# next test. Replace with an AsyncSubject the producer completes, and release in a `finally`. -# -# Counts are produced by the guard's own scanner, with comments and string literals masked — so a -# doc comment naming the ban is not a site and must never be edited to satisfy a textual scan. -# -# 🚨 SEED AGAINST main's TIP, NOT YOUR BRANCH POINT. CI checks out the MERGE REF, so a file main -# gained after you branched is present when the guard runs but absent from a locally-taken seed — -# and the guard correctly reports it as a NEW SITE. -# -# When this file reaches zero, move `test` into the guard's ProductionRoots in the SAME change and -# delete this file. - -test/MeshWeaver.Autocomplete.Test/AutocompleteIntegrationTest.cs 1 -test/MeshWeaver.Graph.Test/NodeTypeAdoptionRegistryTest.cs 2 -test/MeshWeaver.Graph.Test/SyncedQueryRlsForeignTrampolineTest.cs 1 -test/MeshWeaver.Hosting.Monolith.Test/LateNackReenqueueTest.cs 2 -test/MeshWeaver.Hosting.Monolith.Test/LayoutRenderTeardownDrainTest.cs 2 -test/MeshWeaver.Hosting.Monolith.Test/OwnerDisposalNackTest.cs 2 -test/MeshWeaver.Hosting.Monolith.Test/QueuedWriteAdvancesOnHandoffTest.cs 2 -test/MeshWeaver.Hosting.Monolith.Test/ReleaseRequestWatcherHighWaterTest.cs 2 -test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs 2 -test/MeshWeaver.Hosting.Monolith.Test/WriteWaitsForCommitVerdictTest.cs 2 -test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs 2 -test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainDeliveryRetryTest.cs 3 -test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainTurnIsolationTest.cs 1 -test/MeshWeaver.Hosting.Test/IoPoolTest.cs 32 -test/MeshWeaver.Hosting.Test/LiveQueryForeignTrampolineTest.cs 1 -test/MeshWeaver.Hosting.Test/LiveQueryHandoffDropTest.cs 6 -test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs 1 -test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs 2 -test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs 2 -test/MeshWeaver.Messaging.Hub.Test/InflightHubCreationDrainTest.cs 2 -test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs 6 -test/MeshWeaver.Messaging.Hub.Test/TeardownHubCreationFreezeTest.cs 2 -test/MeshWeaver.Persistence.Test/PartitionObjectsSubscriberIndependenceTest.cs 1 diff --git a/test/MeshWeaver.Autocomplete.Test/AutocompleteIntegrationTest.cs b/test/MeshWeaver.Autocomplete.Test/AutocompleteIntegrationTest.cs index 08253a8adb..a58bb8dd5c 100644 --- a/test/MeshWeaver.Autocomplete.Test/AutocompleteIntegrationTest.cs +++ b/test/MeshWeaver.Autocomplete.Test/AutocompleteIntegrationTest.cs @@ -2,7 +2,9 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Reactive; @@ -424,7 +426,7 @@ public async Task ChatAutocomplete_GlobalFanOut_ReachesOtherPartitions() /// Not a pass/fail check — just timing diagnostics. /// [Fact(Timeout = 60000)] - public void TimingAnalysis_OrchestratorScenarios_RecordsLatencies() + public async Task TimingAnalysis_OrchestratorScenarios_RecordsLatencies() { var scenarios = new (string Label, string Query, string? Context)[] { @@ -443,7 +445,7 @@ public void TimingAnalysis_OrchestratorScenarios_RecordsLatencies() foreach (var (label, query, context) in scenarios) { var (firstBatchMs, perCategoryMs, completedMs) = - MeasureCompletionTimings(query, context); + await MeasureCompletionTimings(query, context); var perCategory = string.Join(", ", perCategoryMs.Select(kv => $"{kv.Key}={kv.Value:F0}")); @@ -453,7 +455,7 @@ public void TimingAnalysis_OrchestratorScenarios_RecordsLatencies() } } - private (double FirstBatchMs, IDictionary PerCategoryMs, double CompletedMs) + private async Task<(double FirstBatchMs, IDictionary PerCategoryMs, double CompletedMs)> MeasureCompletionTimings(string query, string? context) { var sw = System.Diagnostics.Stopwatch.StartNew(); @@ -461,10 +463,19 @@ public void TimingAnalysis_OrchestratorScenarios_RecordsLatencies() var completedMs = -1.0; var perCategory = new Dictionary(); - // Pure synchronous Subscribe + gate — measures the live observable's batch - // timings without bridging through a Task ("nothing async ever"). The gate - // releases on OnCompleted/OnError; a 15s cap matches the previous WaitAsync. - using var gate = new System.Threading.ManualResetEventSlim(false); + // 🚨 No hand-woven gate: `finished` is an AsyncSubject the subscription completes and the + // helper awaits reactively. Every timing is still stamped INSIDE the callbacks, off + // `sw.Elapsed`, so what is measured is unchanged — only the waiting is. A 15 s cap matches + // the previous bound, and NotEmit/Emit are not used here because a scenario that produces + // nothing is diagnostics, not a failure. + var finished = new AsyncSubject(); + void Finish() + { + completedMs = sw.Elapsed.TotalMilliseconds; + finished.OnNext(Unit.Default); + finished.OnCompleted(); + } + using (Orchestrator.GetCompletions(query, context).Subscribe( batch => { @@ -473,10 +484,12 @@ public void TimingAnalysis_OrchestratorScenarios_RecordsLatencies() if (!perCategory.ContainsKey(batch.Category)) perCategory[batch.Category] = t; }, - _ => { completedMs = sw.Elapsed.TotalMilliseconds; gate.Set(); }, - () => { completedMs = sw.Elapsed.TotalMilliseconds; gate.Set(); })) + _ => Finish(), + Finish)) { - gate.Wait(TimeSpan.FromSeconds(15)); + await finished + .Timeout(TimeSpan.FromSeconds(15), Observable.Return(Unit.Default)) + .Await(TestContext.Current.CancellationToken); } return (firstBatchMs, perCategory, completedMs); diff --git a/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs b/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs index 89900ff7a0..e983132add 100644 --- a/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs +++ b/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs @@ -19,20 +19,26 @@ namespace MeshWeaver.Documentation.Test; /// where the owning hub serialises every writer); concurrency bounding and one-shot init belong to /// IIoPool (pool.Run(...) held in an INSTANCE PromiseCache). /// -/// Two tiers, deliberately different in strength — the shape #2762 established, and -/// for the same reason: an allow file lists sites you tolerate and grows by appending a line, -/// whereas a VERIFIED register fails when an entry's subject moves or is fixed, and tells the next -/// author to delete it. -/// -/// — ZERO, with no allow file. The only permitted gates are -/// the entries, each re-checked against the tree on every run. -/// — test/ carries a seeded inventory that may only -/// SHRINK. Measured 2026-08-30: 81 sites across 24 files, every one a -/// ManualResetEventSlim, 30 of them in IoPoolTest.cs. The sweep lands in a later -/// wave; this freezes the debt so it cannot grow while that wave is written. When it empties, -/// move test into in the SAME change and delete the -/// allow file — its own header says so. -/// +/// ONE tier as of 2026-08-30 — the allow file is GONE. #2762's shape had two: a +/// verified register for production, and a seeded inventory for test/ that could only +/// shrink. It was seeded at 79 sites across 23 files, every one a +/// ManualResetEventSlim, 32 of them in IoPoolTest.cs. All 79 were converted in one +/// sweep and test/HandWovenGateSites.allow was deleted, so test now sits in +/// alongside everything else and is held at ZERO with no escape +/// hatch. Nothing to append a line to is the strongest form this rule has. +/// +/// What the sweep replaced them with, so the next author does not reach for an event +/// again. A signal a PRODUCER raises and a test consumes becomes an +/// AsyncSubject<Unit> the producer completes (OnNext then OnCompleted), +/// awaited through the house assertion helpers (await x.Should().Within(...).Emit(because), +/// or NotEmit(within) for the negative) — never .Wait() on an observable, which only +/// trades this ratchet for . A RELEASE travelling the +/// other way — into a worker a test deliberately parks, because "a leaf that ignores its +/// cancellation token" or "a wedged action block" IS the subject — becomes a volatile +/// int the parked worker polls under a bounded +/// SpinWait.SpinUntil(predicate, budget), written in a finally so an assertion that +/// throws first cannot strand that worker (the exact leak IoPoolResidualNamesItsPoolTest +/// produced: a two-minute pool-thread hold bleeding into the next test). /// /// 🚨 This guard is proven by MUTATION, not by passing — see /// , which plants a tree and runs the REAL scan over it. @@ -41,18 +47,12 @@ namespace MeshWeaver.Documentation.Test; /// public class HandWovenGateRatchetGuard { - private const string AllowFileName = "HandWovenGateSites.allow"; - - /// Held at ZERO. The only gates here are the verified . - private static readonly string[] ProductionRoots = ["src", "tools", "samples", "clients", "memex"]; - - /// Still carrying a seeded inventory; may only shrink. - private static readonly string[] RatchetedRoots = ["test"]; - /// - /// The sum of the seeded inventory. Lower it by exactly what you delete, in the same change. + /// Every root, held at ZERO with no allow file anywhere. The only permitted gates are the + /// entries, each re-checked against the tree on every run. /// - private const int TotalBudget = 79; + private static readonly string[] ProductionRoots = + ["src", "tools", "samples", "clients", "memex", "test"]; /// /// The primitives that park a thread. Monitor.Wait is included because it is the same @@ -99,12 +99,18 @@ public void ProductionCodeHasNoHandWovenGate_OutsideTheSanctionedOne() .ToList(); Assert.True(offenders.Count == 0, - "🚨 A hand-woven concurrency gate in production code parks a thread — on a hub action " - + "block or a grain turn that is the deadlock the mesh cannot recover from. Serialize " + "🚨 A hand-woven concurrency gate parks a thread — on a hub action block or a grain " + + "turn that is the deadlock the mesh cannot recover from, and in a TEST it strands a " + + "blocked worker whenever an assertion throws before the release runs. Serialize " + "through the hub (a Subject + .Select(Run).Concat(), or GetMeshNodeStream(path)" + ".Update, where the owner serialises writers); bound concurrency through IIoPool " - + "(pool.Run(...) in an INSTANCE PromiseCache). Do NOT add an allow entry — there is no " - + "allow file for these roots.\n" + + "(pool.Run(...) in an INSTANCE PromiseCache). IN A TEST: a producer→test signal is an " + + "AsyncSubject the producer completes, awaited through the assertion helpers " + + "(await x.Should().Within(...).Emit() / .NotEmit(within)) — never .Wait() on an " + + "observable, which just trips BlockingBridgeInTestRatchetGuard instead; a release " + + "INTO a worker the test deliberately parks is a volatile int polled under a bounded " + + "SpinWait.SpinUntil, written in a `finally`. Do NOT add an allow entry — there is no " + + "allow file, for any root.\n" + string.Join("\n", offenders.Select(o => $" {o.Path} ({o.Count})"))); } @@ -116,8 +122,8 @@ public void TheSanctionedRegisterIsStillAccurate() { var full = Path.Combine(root, file); Assert.True(File.Exists(full), - $"{AllowFileName}'s sanctioned register names {file}, which no longer exists. An " - + "entry that outlives its subject is a hole nobody can see — delete it."); + $"The sanctioned register names {file}, which no longer exists. An entry that " + + "outlives its subject is a hole nobody can see — delete it."); Assert.True(CountIn(full) > 0, $"{file} is registered as a sanctioned hand-woven gate but no longer contains one. " + $"Delete the entry — it now exempts nothing. Reason on file: {why}"); @@ -136,38 +142,26 @@ public void TheSanctionedRegisterIsStillAccurate() } } + /// + /// 🚨 The allow file must STAY deleted. A ratchet that empties and is then quietly re-seeded is + /// how a cleared inventory grows back, and is explicit + /// that a REGENERATED allow file blesses whatever is in the tree. There is nothing left to + /// tolerate — test is in — so the file's re-appearance is + /// itself the defect, and it fails here rather than silently widening the rule. + /// [Fact] - public void TheTestTreeInventoryOnlyShrinks() + public void TheTestTreeAllowFileStaysDeleted() { var root = SourceScan.FindRepoRoot(); - var allowed = SourceScan.ReadAllowFile( - Path.Combine(root, "test", AllowFileName), AllowFileName); - - var actual = SourceScan.SourceFiles(root, RatchetedRoots) - .Select(f => (Path: SourceScan.Relative(root, f), Count: CountIn(f))) - .Where(x => x.Count > 0) - .ToDictionary(x => x.Path, x => x.Count, StringComparer.Ordinal); - - var newFiles = actual.Keys.Where(k => !allowed.ContainsKey(k)).OrderBy(k => k).ToList(); - Assert.True(newFiles.Count == 0, - "🚨 NEW hand-woven gate(s) in the test tree. Wait on the condition reactively instead — " - + "an AsyncSubject the producer completes, awaited through the assertion helpers — " - + "and release it in a `finally` so a failing assertion cannot strand a blocked worker " - + "(that is exactly how IoPoolResidualNamesItsPoolTest leaked a pool thread). Do NOT add " - + "a line to " + AllowFileName + ".\n" - + string.Join("\n", newFiles.Select(f => $" NEW {f} ({actual[f]})"))); - - var grew = actual - .Where(kv => allowed.TryGetValue(kv.Key, out var b) && kv.Value > b) - .OrderBy(kv => kv.Key).ToList(); - Assert.True(grew.Count == 0, - "🚨 A file's hand-woven gate count GREW. The inventory may only shrink.\n" - + string.Join("\n", grew.Select(kv => $" {kv.Key}: {allowed[kv.Key]} -> {kv.Value}"))); - - var total = actual.Values.Sum(); - Assert.True(total <= TotalBudget, - $"🚨 TOTAL {total} > {TotalBudget} budgeted — the inventory GREW. Lower TotalBudget by " - + "exactly what you delete, in the same change."); + var allow = Path.Combine(root, "test", "HandWovenGateSites.allow"); + + Assert.False(File.Exists(allow), + "test/HandWovenGateSites.allow is back. It was deleted when the last of its 79 seeded " + + "sites was converted, and `test` moved into ProductionRoots in the same change — so " + + "nothing reads this file any more and its only possible effect is to make a new gate " + + "look sanctioned to a human reader. Convert the site instead: an AsyncSubject " + + "for a producer→test signal, a volatile int polled under a bounded SpinUntil (released " + + "in a `finally`) for a release into a deliberately parked worker."); } /// diff --git a/test/MeshWeaver.Graph.Test/NodeTypeAdoptionRegistryTest.cs b/test/MeshWeaver.Graph.Test/NodeTypeAdoptionRegistryTest.cs index 0679744201..4596b3ea05 100644 --- a/test/MeshWeaver.Graph.Test/NodeTypeAdoptionRegistryTest.cs +++ b/test/MeshWeaver.Graph.Test/NodeTypeAdoptionRegistryTest.cs @@ -1,8 +1,10 @@ #pragma warning disable CS1591 using System; +using System.Reactive; using System.Reactive.Linq; -using System.Threading; +using System.Reactive.Subjects; +using System.Threading.Tasks; using MeshWeaver.Graph.Configuration; using Xunit; @@ -31,21 +33,27 @@ public void NoReservation_IsClearImmediately() } [Fact] - public void AReservationHoldsTheWait_UntilItIsReleased() + public async Task AReservationHoldsTheWait_UntilItIsReleased() { var registry = new NodeTypeAdoptionRegistry(); var reservation = registry.Reserve("Widget/Thing"); Assert.True(registry.IsReserved("Widget/Thing")); - var cleared = new ManualResetEventSlim(); + // 🚨 An AsyncSubject the subscription completes, not a hand-woven gate — both waits below + // are reactive assertions on it, so nothing parks a thread. + var cleared = new AsyncSubject(); using var subscription = registry.WhenClear("Widget/Thing") - .Subscribe(_ => cleared.Set()); + .Subscribe(_ => + { + cleared.OnNext(Unit.Default); + cleared.OnCompleted(); + }); - Assert.False(cleared.Wait(TimeSpan.FromMilliseconds(200)), + await cleared.Should().NotEmit(200.Milliseconds(), "the kickoff must not proceed while an adoption is in flight — that is the whole race"); reservation.Dispose(); - Assert.True(cleared.Wait(Budget), + await cleared.Should().Within(Budget).Emit( "the kickoff must proceed once the adoption released its reservation; a missed release " + "costs the caller its ENTIRE wait budget, per type"); Assert.False(registry.IsReserved("Widget/Thing")); @@ -73,23 +81,27 @@ public void AReleaseThatLandedBeforeTheSubscription_IsNotLost() /// adoption and the second races exactly as before. /// [Fact] - public void ReservationsAreReferenceCounted() + public async Task ReservationsAreReferenceCounted() { var registry = new NodeTypeAdoptionRegistry(); var first = registry.Reserve("Widget/Thing"); var second = registry.Reserve("Widget/Thing"); - var cleared = new ManualResetEventSlim(); + var cleared = new AsyncSubject(); using var subscription = registry.WhenClear("Widget/Thing") - .Subscribe(_ => cleared.Set()); + .Subscribe(_ => + { + cleared.OnNext(Unit.Default); + cleared.OnCompleted(); + }); first.Dispose(); Assert.True(registry.IsReserved("Widget/Thing")); - Assert.False(cleared.Wait(TimeSpan.FromMilliseconds(200)), + await cleared.Should().NotEmit(200.Milliseconds(), "one of two in-flight adoptions finishing must not release the path"); second.Dispose(); - Assert.True(cleared.Wait(Budget)); + await cleared.Should().Within(Budget).Emit(); } /// A reservation on one path says nothing about another. diff --git a/test/MeshWeaver.Graph.Test/SyncedQueryRlsForeignTrampolineTest.cs b/test/MeshWeaver.Graph.Test/SyncedQueryRlsForeignTrampolineTest.cs index dee02106e4..688277e09b 100644 --- a/test/MeshWeaver.Graph.Test/SyncedQueryRlsForeignTrampolineTest.cs +++ b/test/MeshWeaver.Graph.Test/SyncedQueryRlsForeignTrampolineTest.cs @@ -81,7 +81,10 @@ private static void RunProbe(int nodeCount, bool granted, int expected) // point: it reproduces the delivery thread a live upstream emission actually arrives on. var upstream = Observable.Return>(snapshot); - using var arrived = new ManualResetEventSlim(false); + // 🚨 A volatile flag, not a hand-woven gate. Blocking the trampoline frame IS the subject + // (see below); a flag polled under a bounded SpinUntil says exactly that, with no kernel + // handle to dispose and no observable→blocking bridge. + var arrived = 0; IEnumerable? filtered = null; var insideTrampoline = false; @@ -92,13 +95,13 @@ private static void RunProbe(int nodeCount, bool granted, int expected) insideTrampoline = !CurrentThreadScheduler.IsScheduleRequired; using var sub = SyncedQueryDataSourceExtensions .FilterByReadPermission(upstream, _ => Observable.Return(granted)) - .Subscribe(result => { filtered = result; arrived.Set(); }); + .Subscribe(result => { filtered = result; Volatile.Write(ref arrived, 1); }); // 🚨 Blocking HERE is the point, not a shortcut: the stranded iteration could only ever // run after this frame returns to the trampoline that owns it, so a caller that waits // for its own snapshot is the shape that deadlocked. A budget generous enough that only // a never-scheduled iteration can exhaust it — this work is microseconds. - Assert.True(arrived.Wait(TimeSpan.FromSeconds(5)), + Assert.True(SpinWait.SpinUntil(() => Volatile.Read(ref arrived) == 1, TimeSpan.FromSeconds(5)), $"the filtered snapshot never arrived for {nodeCount} node(s) — the per-node " + "iteration was queued on the caller's trampoline instead of running inline, so it " + "can never run (#2087)"); diff --git a/test/MeshWeaver.Hosting.Monolith.Test/DeleteNodeBehaviorTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/DeleteNodeBehaviorTest.cs index 5a0a91d503..b72453955f 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/DeleteNodeBehaviorTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/DeleteNodeBehaviorTest.cs @@ -33,9 +33,9 @@ namespace MeshWeaver.Hosting.Monolith.Test; /// on a blocked delete, and on success every path really is gone. /// /// Fully reactive: a round-trip is observed via -/// client.Observe(req).Should().Within(...).Emit() — the blocking assertion -/// is a synchronous Subscribe + ManualResetEventSlim (no Rx→Task bridge), so it -/// does not starve the handler's response continuation. +/// client.Observe(req).Should().Within(...).Emit() — the assertion subscribes +/// synchronously and is awaited (no hand-woven gate, no Rx→Task bridge), so it +/// neither parks a thread nor starves the handler's response continuation. /// public class DeleteNodeBehaviorTest(ITestOutputHelper output) : MonolithMeshTestBase(output) { diff --git a/test/MeshWeaver.Hosting.Monolith.Test/LateNackReenqueueTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/LateNackReenqueueTest.cs index 9687554c10..d8b4c18f2a 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/LateNackReenqueueTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/LateNackReenqueueTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Data; @@ -68,19 +70,23 @@ await Observable.Interval(TimeSpan.FromMilliseconds(50)).StartWith(0L) var primary = nodeHub!.GetWorkspace().DataContext .GetDataSourceForType(typeof(MeshNode))! .GetStreamForPartition(null)!; - using var gateEntered = new ManualResetEventSlim(false); - using var releaseGate = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate. The turn → test signal is an AsyncSubject the parked turn + // completes; the release travels back INTO that deliberately parked executor turn, so it + // is a volatile flag polled under a bounded SpinUntil and written in the `finally` below. + var gateEntered = new AsyncSubject(); + var releaseGate = 0; primary.Update((Func?>)(_ => { - gateEntered.Set(); - releaseGate.Wait(TimeSpan.FromSeconds(60)); + gateEntered.OnNext(Unit.Default); + gateEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseGate) == 1, TimeSpan.FromSeconds(60)); return null; }), _ => { }); - gateEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue( - "the gated turn must be running on the primary stream's executor before the write"); - try { + await gateEntered.Should().Within(10.Seconds()).Emit( + "the gated turn must be running on the primary stream's executor before the write"); + // Cross-hub cache write — the production mirror path (UpdateRemote via the // per-path queue). With the owner's merge parked, no verdict can arrive inside the // caller's response bound, so the caller is NOT settled here (#2661): it stays open @@ -138,7 +144,7 @@ await Observable.Interval(TimeSpan.FromMilliseconds(100)).StartWith(0L) } finally { - releaseGate.Set(); + Volatile.Write(ref releaseGate, 1); } } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/LayoutRenderTeardownDrainTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/LayoutRenderTeardownDrainTest.cs index ab740e1421..ee547dfa4b 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/LayoutRenderTeardownDrainTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/LayoutRenderTeardownDrainTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -49,12 +51,13 @@ public class LayoutRenderTeardownDrainTest(ITestOutputHelper output) : MonolithM private const string BlockingArea = nameof(BlockingArea); private static readonly Address HostAddress = new("layout-render-drain-host"); - // Signalled by the render body once it is executing on the pool thread (holding a Layout-pool - // permit); the test proceeds only after the render is genuinely in-flight — no sleeps, no polling. - private readonly ManualResetEventSlim renderInFlight = new(false); - // Held by the render body until the test releases it, keeping the render subscribe in-flight for - // the duration of the assertions. - private readonly ManualResetEventSlim releaseRender = new(false); + // 🚨 No hand-woven gate. Completed by the render body once it is executing on the pool thread + // (holding a Layout-pool permit); the test awaits it reactively, so it proceeds only after the + // render is genuinely in-flight — no sleeps, no polling, and nothing to dispose. + private readonly AsyncSubject renderInFlight = new(); + // The other direction: the render body is DELIBERATELY parked (that occupancy is what the drain + // must join), so the release is a volatile flag it polls under a bounded SpinUntil. + private int releaseRender; /// /// A view generator that mirrors the production render body shape (a synchronous render that @@ -65,9 +68,10 @@ public class LayoutRenderTeardownDrainTest(ITestOutputHelper output) : MonolithM /// private IObservable BlockingView(LayoutAreaHost host, RenderingContext ctx) { - renderInFlight.Set(); + renderInFlight.OnNext(Unit.Default); + renderInFlight.OnCompleted(); // Bounded so a wiring regression can never hang the suite; the test releases well within this. - releaseRender.Wait(TimeSpan.FromSeconds(30)); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseRender) == 1, TimeSpan.FromSeconds(30)); // Same service-resolution shape as the real menu renderer whose GetService threw the disposed // -scope ODE at teardown. With the drain joining this render first, the scope is still alive. _ = host.Hub.ServiceProvider.GetService(); @@ -94,8 +98,8 @@ public async Task LayoutRenderSubscribe_RunsAsTrackedDrainableLeaf_AndDrainJoins using var areaSubscription = areaStream.GetControlStream(BlockingArea).Subscribe(_ => { }); // Wait until the render body is genuinely executing on the pool (deterministic — no sleep). - renderInFlight.Wait(TimeSpan.FromSeconds(30)) - .Should().BeTrue("the layout render must actually start executing"); + await renderInFlight.Should().Within(30.Seconds()) + .Emit("the layout render must actually start executing"); // (1) TRACKED: the in-flight render holds a Layout-pool permit. Pre-fix it ran on a bare // TaskPoolScheduler thread the pool never saw → CurrentInFlight == 0. @@ -114,7 +118,7 @@ public async Task LayoutRenderSubscribe_RunsAsTrackedDrainableLeaf_AndDrainJoins "executing is the teardown use-after-dispose/unload window this fix closes"); // Release the render; DrainAll can now acquire the freed permit and return. - releaseRender.Set(); + Volatile.Write(ref releaseRender, 1); await drainReturned.WaitAsync(TimeSpan.FromSeconds(30)); layoutPool.CurrentInFlight.Should().Be(0, "the render released its permit once it completed"); } @@ -123,9 +127,7 @@ public override async ValueTask DisposeAsync() { // Guarantee the render is never left blocked if an assertion aborts early (the [Fact] timeout // would otherwise wait on the 30s render bound before teardown). - releaseRender.Set(); + Volatile.Write(ref releaseRender, 1); await base.DisposeAsync(); - renderInFlight.Dispose(); - releaseRender.Dispose(); } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/OwnerDisposalNackTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/OwnerDisposalNackTest.cs index 0a7e33dc89..d0cad749e1 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/OwnerDisposalNackTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/OwnerDisposalNackTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Data; @@ -59,23 +61,28 @@ await Observable.Interval(TimeSpan.FromMilliseconds(50)).StartWith(0L) // Park the PRIMARY data-source stream's turn executor with a gated no-op turn: // the patch's merge turn queues BEHIND it on the same serialized executor, so it // provably cannot run before the hub is disposed. Test-side gating only — the - // same ManualResetEventSlim pattern as TeardownHubCreationFreezeTest. + // same AsyncSubject + volatile-flag pattern as TeardownHubCreationFreezeTest. var primary = nodeHub!.GetWorkspace().DataContext .GetDataSourceForType(typeof(MeshNode))! .GetStreamForPartition(null)!; - using var gateEntered = new ManualResetEventSlim(false); - using var releaseGate = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: turn → test is an AsyncSubject the parked turn completes; the + // release travels back INTO that turn, so it is a volatile flag polled under a bounded + // SpinUntil and written in the `finally` below. + var gateEntered = new AsyncSubject(); + var releaseGate = 0; primary.Update((Func?>)(_ => { - gateEntered.Set(); - releaseGate.Wait(TimeSpan.FromSeconds(60)); + gateEntered.OnNext(Unit.Default); + gateEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseGate) == 1, TimeSpan.FromSeconds(60)); return null; }), _ => { }); - gateEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue( - "the gated turn must be running on the primary stream's executor before the patch is posted"); try { + await gateEntered.Should().Within(10.Seconds()).Emit( + "the gated turn must be running on the primary stream's executor before the patch is posted"); + // Post the patch with a pre-registered response observable, then FENCE on a // GetDataRequest response from the same single-threaded action block — when the // fence answers, the patch handler has run: ack watcher + disposal NACK are @@ -111,7 +118,7 @@ await RequestHub.Observe(new GetDataRequest(new MeshNodeReference()), o => o.Wit } finally { - releaseGate.Set(); + Volatile.Write(ref releaseGate, 1); } } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/QueuedWriteAdvancesOnHandoffTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/QueuedWriteAdvancesOnHandoffTest.cs index 2429aa805c..faeda80e82 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/QueuedWriteAdvancesOnHandoffTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/QueuedWriteAdvancesOnHandoffTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Data; @@ -90,19 +92,24 @@ await Observable.Interval(TimeSpan.FromMilliseconds(50)).StartWith(0L) var primary = nodeHub!.GetWorkspace().DataContext .GetDataSourceForType(typeof(MeshNode))! .GetStreamForPartition(null)!; - using var gateEntered = new ManualResetEventSlim(false); - using var releaseGate = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: turn → test is an AsyncSubject the parked turn completes; the + // release travels back INTO that turn, so it is a volatile flag polled under a bounded + // SpinUntil and written in the `finally` below. + var gateEntered = new AsyncSubject(); + var releaseGate = 0; primary.Update((Func?>)(_ => { - gateEntered.Set(); - releaseGate.Wait(TimeSpan.FromSeconds(60)); + gateEntered.OnNext(Unit.Default); + gateEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseGate) == 1, TimeSpan.FromSeconds(60)); return null; }), _ => { }); - gateEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue( - "the gated turn must be running on the primary stream's executor before the writes"); try { + await gateEntered.Should().Within(10.Seconds()).Emit( + "the gated turn must be running on the primary stream's executor before the writes"); + // WRITE 1 — its ack cannot arrive while the merge turn is parked. Pre-#2346 the // caller's terminal at that bound is what released the queue slot; pre-#2661 that // terminal existed at all. Subscribe and fence on the patch being in flight instead: @@ -139,7 +146,7 @@ await Observable.Interval(TimeSpan.FromMilliseconds(50)).StartWith(0L) Output.WriteLine("[write 2] enqueued"); // Release: the owner drains the parked turn, then write 1's merge, then write 2's. - releaseGate.Set(); + Volatile.Write(ref releaseGate, 1); Output.WriteLine("[owner] merge executor released"); // GROUND TRUTH — the store must end on the SUCCESSOR's value. Pre-fix write 2 shipped @@ -186,7 +193,7 @@ await Observable.Interval(TimeSpan.FromMilliseconds(50)).StartWith(0L) } finally { - releaseGate.Set(); + Volatile.Write(ref releaseGate, 1); } } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/ReleaseRequestWatcherHighWaterTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/ReleaseRequestWatcherHighWaterTest.cs index c133039a29..bd1eb17fc7 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/ReleaseRequestWatcherHighWaterTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/ReleaseRequestWatcherHighWaterTest.cs @@ -1,6 +1,8 @@ using System; using System.IO; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Data; @@ -153,68 +155,80 @@ await Mesh.GetWorkspace().GetMeshNodeStream(typePath) // on the owner's serialized write path), and the test blocks on it // BEFORE enqueueing the triggers. Without it, on a busy runner the gate // write could still be queued when the triggers are posted and only - // execute after gate.Set() — nothing parked, the interleaving silently + // execute after the gate opens — nothing parked, the interleaving silently // degrades to ordinary timing, and the test could false-PASS a future - // regression. Cross-thread ManualResetEventSlim handshake is the - // project's established pattern for synchronous Subscribe-side signals - // (see DeleteNodeBehaviorTest); both waits are bounded so a broken run - // can never wedge the suite. - using var parked = new ManualResetEventSlim(false); - using var gate = new ManualResetEventSlim(false); + // regression. 🚨 No hand-woven gate in either direction: `parked` travels + // write-turn → test, so it is an AsyncSubject the turn completes and the + // test awaits reactively; the gate travels back INTO the deliberately + // parked turn, so it is a volatile flag polled under a bounded SpinUntil + // and opened in the `finally` below. Both waits are bounded so a broken + // run can never wedge the suite. + var parked = new AsyncSubject(); + var gate = 0; ownWs.GetMeshNodeStream().Update(curr => { // Handshake: the serialized write queue is now provably parked. - parked.Set(); - if (!gate.Wait(TimeSpan.FromSeconds(20))) + parked.OnNext(Unit.Default); + parked.OnCompleted(); + if (!SpinWait.SpinUntil(() => Volatile.Read(ref gate) == 1, TimeSpan.FromSeconds(20))) Output.WriteLine("!!! gate wait timed out — interleaving no longer forced"); return curr; // no-op }).Subscribe( _ => { }, ex => Output.WriteLine($"gate write failed: {ex}")); - parked.Wait(TimeSpan.FromSeconds(20)).Should().BeTrue( - "the gate write must be executing (owner write queue parked) before the " - + "triggers are enqueued — otherwise the lost-trigger interleaving is not forced"); - - var t1 = DateTimeOffset.UtcNow; - var t2 = t1.AddMilliseconds(200); - ownWs.GetMeshNodeStream().Update(curr => - curr.Content is not NodeTypeDefinition d - ? curr - : curr with - { - Content = d with { RequestedReleaseAt = t1, RequestedReleaseForce = true } - }) - .Subscribe(_ => { }, ex => Output.WriteLine($"P1 write failed: {ex}")); - ownWs.GetMeshNodeStream().Update(curr => - curr.Content is not NodeTypeDefinition d - ? curr - : curr with - { - Content = d with { RequestedReleaseAt = t2, RequestedReleaseForce = true } - }) - .Subscribe(_ => { }, ex => Output.WriteLine($"P2 write failed: {ex}")); - - Output.WriteLine($"=== triggers enqueued: t1={t1:O} t2={t2:O} — opening gate ==="); - gate.Set(); - - // 4. The CONTRACT: every trigger is eventually handled. The watcher's - // Update for T2 bails (status is Pending from T1's dispatch), so T2 - // must re-fire on the post-settle emission and be stamped. With the - // eager high-water advance, that re-fire is gated off and - // LastReleaseRequestHandledAt stays at t1 forever — this wait times - // out, pinning the lost trigger. - await Mesh.GetWorkspace().GetMeshNodeStream(typePath) - .Should().Within(45.Seconds()) - .Match(n => n?.Content is NodeTypeDefinition d - && d.LastReleaseRequestHandledAt is { } handled - && handled >= t2); - Output.WriteLine("=== t2 handled ==="); - - // And the whole machine settles cleanly afterwards. - await Mesh.GetWorkspace().GetMeshNodeStream(typePath) - .Should().Within(45.Seconds()) - .Match(n => n?.Content is NodeTypeDefinition d - && d.CompilationStatus == CompilationStatus.Ok); + try + { + await parked.Should().Within(20.Seconds()).Emit( + "the gate write must be executing (owner write queue parked) before the " + + "triggers are enqueued — otherwise the lost-trigger interleaving is not forced"); + + var t1 = DateTimeOffset.UtcNow; + var t2 = t1.AddMilliseconds(200); + ownWs.GetMeshNodeStream().Update(curr => + curr.Content is not NodeTypeDefinition d + ? curr + : curr with + { + Content = d with { RequestedReleaseAt = t1, RequestedReleaseForce = true } + }) + .Subscribe(_ => { }, ex => Output.WriteLine($"P1 write failed: {ex}")); + ownWs.GetMeshNodeStream().Update(curr => + curr.Content is not NodeTypeDefinition d + ? curr + : curr with + { + Content = d with { RequestedReleaseAt = t2, RequestedReleaseForce = true } + }) + .Subscribe(_ => { }, ex => Output.WriteLine($"P2 write failed: {ex}")); + + Output.WriteLine($"=== triggers enqueued: t1={t1:O} t2={t2:O} — opening gate ==="); + Volatile.Write(ref gate, 1); + + // 4. The CONTRACT: every trigger is eventually handled. The watcher's + // Update for T2 bails (status is Pending from T1's dispatch), so T2 + // must re-fire on the post-settle emission and be stamped. With the + // eager high-water advance, that re-fire is gated off and + // LastReleaseRequestHandledAt stays at t1 forever — this wait times + // out, pinning the lost trigger. + await Mesh.GetWorkspace().GetMeshNodeStream(typePath) + .Should().Within(45.Seconds()) + .Match(n => n?.Content is NodeTypeDefinition d + && d.LastReleaseRequestHandledAt is { } handled + && handled >= t2); + Output.WriteLine("=== t2 handled ==="); + + // And the whole machine settles cleanly afterwards. + await Mesh.GetWorkspace().GetMeshNodeStream(typePath) + .Should().Within(45.Seconds()) + .Match(n => n?.Content is NodeTypeDefinition d + && d.CompilationStatus == CompilationStatus.Ok); + } + finally + { + // 🚨 In a `finally`: an assertion that throws above must not leave the owner's write + // queue parked for the full 20 s bound, into the rest of the class. + Volatile.Write(ref gate, 1); + } } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs index 8f371b67e1..e6a46ad7bd 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Data; @@ -50,10 +52,13 @@ public class ShutdownRoutingRejectClassificationTest(ITestOutputHelper output) : /// Message whose handler stalls the mesh's action block for the duration of the setup. private record StallMeshActionBlock; - // Instance gates (never static — they die with the test instance, like every other + // Instance state (never static — it dies with the test instance, like every other // fixture-owned state in this suite). Test-side gating only: nothing in src/ is timed. - private readonly ManualResetEventSlim stallEntered = new(false); - private readonly ManualResetEventSlim releaseStall = new(false); + // 🚨 No hand-woven gate: handler → test is an AsyncSubject the stalling handler completes; + // the release travels back INTO the deliberately stalled action block, so it is a volatile + // flag polled under a bounded SpinUntil and written in the test's `finally`. + private readonly AsyncSubject stallEntered = new(); + private int releaseStall; protected override MeshBuilder ConfigureMesh(MeshBuilder builder) => base.ConfigureMesh(builder) @@ -70,11 +75,12 @@ protected override MeshBuilder ConfigureMesh(MeshBuilder builder) => .WithTypes(typeof(StallMeshActionBlock)) .WithHandler((_, request) => { - stallEntered.Set(); + stallEntered.OnNext(Unit.Default); + stallEntered.OnCompleted(); // Deliberately ignores cancellation (Dispose() calls CancelExecution): // the point is that the ShutdownRequest posted by Dispose() cannot be // processed until the test has queued everything behind it. - releaseStall.Wait(TimeSpan.FromSeconds(30)); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseStall) == 1, TimeSpan.FromSeconds(30)); return request.Processed(); })); @@ -109,7 +115,7 @@ await NodeFactory.CreateNode( { // 1. Stall the mesh action block. Everything posted from here on queues behind it. Mesh.Post(new StallMeshActionBlock(), o => o.WithTarget(Mesh.Address)); - stallEntered.Wait(TimeSpan.FromSeconds(20)).Should().BeTrue( + await stallEntered.Should().Within(20.Seconds()).Emit( "the stalling handler must own the action block before the interleaving is queued"); // 2. Queue the probe. Observe pre-registers the response callback before posting. @@ -131,7 +137,7 @@ await NodeFactory.CreateNode( } finally { - releaseStall.Set(); + Volatile.Write(ref releaseStall, 1); } var failure = await Assert.ThrowsAsync(() => probe); diff --git a/test/MeshWeaver.Hosting.Monolith.Test/WriteWaitsForCommitVerdictTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/WriteWaitsForCommitVerdictTest.cs index e6e48afbb1..2fc07871ea 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/WriteWaitsForCommitVerdictTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/WriteWaitsForCommitVerdictTest.cs @@ -1,6 +1,8 @@ using System; using System.Linq; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Data; @@ -69,7 +71,7 @@ await NodeFactory.CreateNode( .Should().Emit(); var workspace = await WarmMirror(path, ct); - using var gate = ParkOwnerMergeTurn(path); + using var gate = await ParkOwnerMergeTurn(path); try { @@ -145,7 +147,7 @@ await NodeFactory.CreateNode( var workspace = await WarmMirror(path, ct); var registry = Mesh.ServiceProvider.GetRequiredService(); - using var gate = ParkOwnerMergeTurn(path); + using var gate = await ParkOwnerMergeTurn(path); try { @@ -221,7 +223,7 @@ await RequestHub.Observe(new GetDataRequest(new MeshNodeReference()), o => o.Wit /// handler, but its merge provably cannot run, so no PatchDataResponse can arrive inside the /// caller's response bound. Same device as LateNackReenqueueTest / QueuedWriteAdvancesOnHandoffTest. /// - private OwnerGate ParkOwnerMergeTurn(string path) + private async Task ParkOwnerMergeTurn(string path) { var nodeHub = Mesh.GetHostedHub(new Address(path), HostedHubCreation.Never); nodeHub.Should().NotBeNull("the owner per-node hub must be live before its merge turn is parked"); @@ -231,22 +233,40 @@ private OwnerGate ParkOwnerMergeTurn(string path) var gate = new OwnerGate(); primary.Update((Func?>)(_ => { - gate.Entered.Set(); - gate.Released.Wait(TimeSpan.FromSeconds(120)); + gate.MarkEntered(); + SpinWait.SpinUntil(() => gate.IsReleased, TimeSpan.FromSeconds(120)); return null; }), _ => { }); - gate.Entered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue( + await gate.Entered.Should().Within(10.Seconds()).Emit( "the gated turn must be running on the primary stream's executor before the write"); return gate; } + /// + /// 🚨 No hand-woven gate. travels parked-turn → test, so it is an + /// the turn completes and the test awaits reactively; the release + /// travels back INTO that deliberately parked turn, so it is a volatile flag the turn polls + /// under a bounded SpinUntil at the call site. releases, so + /// using var gate = await ParkOwnerMergeTurn(...) cannot leave the owner's executor + /// parked when an assertion throws. + /// private sealed class OwnerGate : IDisposable { - public ManualResetEventSlim Entered { get; } = new(false); - public ManualResetEventSlim Released { get; } = new(false); - public void Release() => Released.Set(); - // Release only — never Dispose the events. The parked turn's thread is still inside - // Released.Wait when this runs, and disposing the handle out from under it throws there. - public void Dispose() => Released.Set(); + private readonly AsyncSubject entered = new(); + private int released; + + public IObservable Entered => entered; + public bool IsReleased => Volatile.Read(ref released) == 1; + + public void MarkEntered() + { + entered.OnNext(Unit.Default); + entered.OnCompleted(); + } + + public void Release() => Volatile.Write(ref released, 1); + // Release only — there is no handle to dispose, and the parked turn's thread may still be + // inside its SpinUntil when this runs; it simply observes the flag and returns. + public void Dispose() => Release(); } } diff --git a/test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs b/test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs index a672c10848..fdab97479b 100644 --- a/test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs +++ b/test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reactive; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Connection.Orleans; @@ -106,19 +108,23 @@ public async Task Stop_JoinsPooledIo_EvenAfterTheContainerHasBeenDisposed() // One pooled leaf in flight that ends ONLY when it is cancelled — the shape the drain // exists for (a live change-feed leaf never completes on its own, so a wait-without-cancel // would burn the budget and then release over live work). - using var entered = new ManualResetEventSlim(false); - using var unwound = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: `entered` travels leaf → test, so it is an AsyncSubject the leaf + // completes and the test awaits reactively; `unwound` is never WAITED on — it is read as + // state after OnStop returns — so a volatile flag says exactly that and nothing more. + var entered = new AsyncSubject(); + var unwound = 0; using var leaf = registry.Get(IoPoolNames.FileSystem) .InvokeBlocking(ct => { - entered.Set(); + entered.OnNext(Unit.Default); + entered.OnCompleted(); ct.WaitHandle.WaitOne(Budget); - unwound.Set(); + Volatile.Write(ref unwound, 1); return 0; }) .Subscribe(_ => { }, _ => { }); - entered.Wait(Budget).Should().BeTrue( + await entered.Should().Within(Budget).Emit( "precondition: the pooled leaf must actually be running before the drain is asked to join it"); // 🚨 The race, reproduced: host startup was aborted, so the root provider was disposed @@ -132,7 +138,7 @@ public async Task Stop_JoinsPooledIo_EvenAfterTheContainerHasBeenDisposed() Func stop = () => observer.OnStop(TestContext.Current.CancellationToken); await stop.Should().NotThrowAsync(); - unwound.IsSet.Should().BeTrue( + Volatile.Read(ref unwound).Should().Be(1, "OnStop must not RETURN until the pooled leaf has actually unwound — the join is the " + "whole point, and returning over live work is the use-after-unload SIGSEGV (#613)"); logger.Entries.Should().Contain( diff --git a/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainDeliveryRetryTest.cs b/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainDeliveryRetryTest.cs index 759ca110af..23cd8534ae 100644 --- a/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainDeliveryRetryTest.cs +++ b/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainDeliveryRetryTest.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using System.Reactive; using System.Reactive.Concurrency; +using System.Reactive.Subjects; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -112,29 +114,36 @@ await Assert.ThrowsAsync(() => } [Fact] - public void DeliverToGrainWithRetry_OnTerminalFault_NacksSenderFast_NotSilentDeadEnd() + public async Task DeliverToGrainWithRetry_OnTerminalFault_NacksSenderFast_NotSilentDeadEnd() { var nacks = new List<(string Message, ErrorType Type)>(); - using var done = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: the NACK callback → test signal is an AsyncSubject the producer + // completes, awaited through the assertion helpers. + var done = new AsyncSubject(); RoutingGrain.DeliverToGrainWithRetry( grainCall: () => Task.FromException(new InvalidOperationException("node type not registered")), grainKey: "X", addressPath: "X", deliveryId: "t4", - postFailureToSender: (m, t) => { nacks.Add((m, t)); done.Set(); }, + postFailureToSender: (m, t) => + { + nacks.Add((m, t)); + done.OnNext(Unit.Default); + done.OnCompleted(); + }, logger: NullLogger.Instance, backoff: NoBackoff, scheduler: Scheduler.Immediate); // The sender must be NACKed (deterministic OnError) — never left to a silent dead-end → 60 s timeout. - Assert.True(done.Wait(TimeSpan.FromSeconds(5))); + await done.Should().Within(5.Seconds()).Emit(); var nack = Assert.Single(nacks); Assert.Equal(ErrorType.Failed, nack.Type); } [Fact] - public void DeliverToGrainWithRetry_ExhaustedRetries_SurfacesRecordedActivationError_NotRawRejection() + public async Task DeliverToGrainWithRetry_ExhaustedRetries_SurfacesRecordedActivationError_NotRawRejection() { // Issue #464, Defect 3: a grain stuck in a PERSISTENT activation-fault loop rejects every // delivery with the raw Orleans rejection ("DeactivateOnIdle was called … Rejecting now"), @@ -144,7 +153,9 @@ public void DeliverToGrainWithRetry_ExhaustedRetries_SurfacesRecordedActivationE // OnError with an actionable message instead of Orleans internals. const string realCause = "Compilation failed for 'UWDeepfield/UwAttention': CS1501 no overload for 'GetFiles' takes 2 arguments"; var nacks = new List<(string Message, ErrorType Type)>(); - using var done = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: the NACK callback → test signal is an AsyncSubject the producer + // completes, awaited through the assertion helpers. + var done = new AsyncSubject(); RoutingGrain.DeliverToGrainWithRetry( // Every attempt lands in a deactivation window — the persistent-fault loop. @@ -152,7 +163,12 @@ public void DeliverToGrainWithRetry_ExhaustedRetries_SurfacesRecordedActivationE grainKey: "UWDeepfield/UwAttention", addressPath: "UWDeepfield/UwAttention", deliveryId: "t5", - postFailureToSender: (m, t) => { nacks.Add((m, t)); done.Set(); }, + postFailureToSender: (m, t) => + { + nacks.Add((m, t)); + done.OnNext(Unit.Default); + done.OnCompleted(); + }, logger: NullLogger.Instance, maxRetries: 2, backoff: NoBackoff, @@ -161,7 +177,7 @@ public void DeliverToGrainWithRetry_ExhaustedRetries_SurfacesRecordedActivationE resolveActivationError: gk => gk == "UWDeepfield/UwAttention" ? realCause : null); - Assert.True(done.Wait(TimeSpan.FromSeconds(5))); + await done.Should().Within(5.Seconds()).Emit(); var nack = Assert.Single(nacks); Assert.Equal(ErrorType.Failed, nack.Type); // The NACK must carry the REAL cause the grain recorded — not the raw rejection text. @@ -169,25 +185,32 @@ public void DeliverToGrainWithRetry_ExhaustedRetries_SurfacesRecordedActivationE } [Fact] - public void DeliverToGrainWithRetry_NoRecordedActivationError_FallsBackToRawException() + public async Task DeliverToGrainWithRetry_NoRecordedActivationError_FallsBackToRawException() { // When nothing was recorded for the grain key (the registry has no entry, or no registry is // wired at all), behaviour is unchanged: the NACK carries the underlying exception message. var nacks = new List<(string Message, ErrorType Type)>(); - using var done = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: the NACK callback → test signal is an AsyncSubject the producer + // completes, awaited through the assertion helpers. + var done = new AsyncSubject(); RoutingGrain.DeliverToGrainWithRetry( grainCall: () => Task.FromException(new InvalidOperationException("node type not registered")), grainKey: "X", addressPath: "X", deliveryId: "t6", - postFailureToSender: (m, t) => { nacks.Add((m, t)); done.Set(); }, + postFailureToSender: (m, t) => + { + nacks.Add((m, t)); + done.OnNext(Unit.Default); + done.OnCompleted(); + }, logger: NullLogger.Instance, backoff: NoBackoff, scheduler: Scheduler.Immediate, resolveActivationError: _ => null); - Assert.True(done.Wait(TimeSpan.FromSeconds(5))); + await done.Should().Within(5.Seconds()).Emit(); var nack = Assert.Single(nacks); Assert.Equal(ErrorType.Failed, nack.Type); Assert.Contains("node type not registered", nack.Message); diff --git a/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainTurnIsolationTest.cs b/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainTurnIsolationTest.cs index a4b054d70d..7315e5d88c 100644 --- a/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainTurnIsolationTest.cs +++ b/test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainTurnIsolationTest.cs @@ -140,7 +140,10 @@ internal sealed class StallingPathResolver(PathResolutionService inner) : IPathR /// private static readonly TimeSpan MaxStall = TimeSpan.FromSeconds(60); - private readonly ManualResetEventSlim released = new(false); + // 🚨 A volatile flag, not a hand-woven gate. The stall itself IS the injected fault, so the + // park stays — what goes is the kernel handle. `Release`/`Dispose` just flip the flag, and the + // bounded SpinUntil below can never outlive MaxStall even if neither is ever called. + private int released; private int stallsEntered; private int stallsCompleted; @@ -151,7 +154,7 @@ internal sealed class StallingPathResolver(PathResolutionService inner) : IPathR internal int StallsCompleted => Volatile.Read(ref stallsCompleted); /// Unblocks every stalled leg so nothing holds a thread or a pool permit into teardown. - internal void Release() => released.Set(); + internal void Release() => Volatile.Write(ref released, 1); public IObservable ResolvePath(string path) => Stall(path) ?? inner.ResolvePath(path); @@ -168,7 +171,7 @@ internal sealed class StallingPathResolver(PathResolutionService inner) : IPathR // 🚨 Deliberate fault INJECTION, not a wait-for-propagation sleep: this models a leaf // whose synchronous prologue never returns (the prod wedge). Pre-fix this runs on the // routing grain's activation thread and the whole silo stops routing. - released.Wait(MaxStall); + SpinWait.SpinUntil(() => Volatile.Read(ref released) == 1, MaxStall); Interlocked.Increment(ref stallsCompleted); // Unwind as an ordinary NotFound so the release path is clean. observer.OnNext(null); @@ -177,7 +180,7 @@ internal sealed class StallingPathResolver(PathResolutionService inner) : IPathR }); } - public void Dispose() => released.Set(); + public void Dispose() => Release(); } /// diff --git a/test/MeshWeaver.Hosting.Test/IoPoolTest.cs b/test/MeshWeaver.Hosting.Test/IoPoolTest.cs index 387579fa07..d05f04fa86 100644 --- a/test/MeshWeaver.Hosting.Test/IoPoolTest.cs +++ b/test/MeshWeaver.Hosting.Test/IoPoolTest.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using System.Reactive; using System.Reactive.Linq; using System.Reactive.Subjects; using MeshWeaver.Mesh.Threading; @@ -16,8 +17,15 @@ namespace MeshWeaver.Hosting.Test; /// pushes genuinely-async / sync-blocking leaf work off the hub schedulers onto a /// bounded slice of the shared ThreadPool. These prove the two properties the /// pool exists for: it caps concurrency, and it runs off the calling thread. -/// No Task.Delay-to-wait — every wait is a condition wait -/// ( / ). +/// No Task.Delay-to-wait — every wait is a condition wait. +/// +/// 🚨 And no hand-woven concurrency gate, in either direction. A signal a POOL LEAF produces +/// and the test consumes is an the leaf completes, awaited through +/// the assertion helpers (never a blocking bridge). A release that travels the other way — INTO a +/// leaf this test deliberately parks, because "a leaf that ignores its cancellation token" IS the +/// subject of half these tests — is a volatile flag the leaf polls under a bounded +/// , written in a finally so a failing +/// assertion cannot leave a pool thread parked into the next test. /// public class IoPoolTest { @@ -52,19 +60,20 @@ public class IoPoolTest public async Task Dispose_cancels_without_blocking_and_reports_through_Disposed() { var pool = new IoPool(2); - using var entered = new ManualResetEventSlim(false); + var entered = new AsyncSubject(); var observedCancellation = false; pool.InvokeBlocking(ct => { - entered.Set(); + entered.OnNext(Unit.Default); + entered.OnCompleted(); // Honours the token — so the pool's cancel is what ends it. ct.WaitHandle.WaitOne(Timeout10); observedCancellation = ct.IsCancellationRequested; return 0; }).Subscribe(_ => { }, _ => { }); - entered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); + await entered.Should().Within(Timeout5).Emit(); pool.CurrentInFlight.Should().Be(1); // Dispose must return PROMPTLY even with a leaf in flight — the whole point. @@ -91,27 +100,46 @@ public async Task Dispose_cancels_without_blocking_and_reports_through_Disposed( public async Task Disposed_waits_for_the_last_leaf_and_replays_to_a_late_subscriber() { var pool = new IoPool(2); - using var entered = new ManualResetEventSlim(false); - using var release = new ManualResetEventSlim(false); + var entered = new AsyncSubject(); + var release = 0; - pool.InvokeBlocking(_ => { entered.Set(); release.Wait(Timeout10); return 0; }) + pool.InvokeBlocking(_ => + { + entered.OnNext(Unit.Default); + entered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref release) == 1, Timeout10); + return 0; + }) .Subscribe(_ => { }, _ => { }); - entered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - - using var fired = new ManualResetEventSlim(false); - pool.Disposed.Subscribe(_ => fired.Set()); + await entered.Should().Within(Timeout5).Emit(); - pool.Dispose(); - fired.Wait(300, TestContext.Current.CancellationToken).Should().BeFalse( - "Disposed must not fire while the leaf is still running"); - - release.Set(); - await pool.Disposed.Timeout(Timeout10).FirstAsync().Await(TestContext.Current.CancellationToken); - fired.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); + // An EARLY subscriber — the property under test is that IT is not notified prematurely, + // which a late `pool.Disposed` read (AsyncSubject replays) could not distinguish. + var fired = new AsyncSubject(); + pool.Disposed.Subscribe(_ => + { + fired.OnNext(Unit.Default); + fired.OnCompleted(); + }); - var late = -1; - pool.Disposed.Subscribe(n => late = n); - late.Should().Be(0, "AsyncSubject replays the terminal report to a late subscriber"); + try + { + pool.Dispose(); + await fired.Should().NotEmit(300.Milliseconds(), + "Disposed must not fire while the leaf is still running"); + + Volatile.Write(ref release, 1); + await pool.Disposed.Timeout(Timeout10).FirstAsync().Await(TestContext.Current.CancellationToken); + await fired.Should().Within(Timeout5).Emit(); + + var late = -1; + pool.Disposed.Subscribe(n => late = n); + late.Should().Be(0, "AsyncSubject replays the terminal report to a late subscriber"); + } + finally + { + Volatile.Write(ref release, 1); + } } /// @@ -124,28 +152,53 @@ public async Task Registry_Disposed_reports_only_after_every_pool_has_finished() var registry = new IoPoolRegistry(); var a = registry.Get("pool-a"); var b = registry.Get("pool-b"); - using var enteredA = new ManualResetEventSlim(false); - using var enteredB = new ManualResetEventSlim(false); - using var releaseA = new ManualResetEventSlim(false); - using var releaseB = new ManualResetEventSlim(false); - - a.InvokeBlocking(_ => { enteredA.Set(); releaseA.Wait(Timeout10); return 0; }).Subscribe(_ => { }, _ => { }); - b.InvokeBlocking(_ => { enteredB.Set(); releaseB.Wait(Timeout10); return 0; }).Subscribe(_ => { }, _ => { }); - enteredA.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - enteredB.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - registry.TotalInFlight.Should().Be(2); - - using var fired = new ManualResetEventSlim(false); - registry.Disposed.Subscribe(_ => fired.Set()); - registry.Dispose(); + var enteredA = new AsyncSubject(); + var enteredB = new AsyncSubject(); + var releaseA = 0; + var releaseB = 0; + + a.InvokeBlocking(_ => + { + enteredA.OnNext(Unit.Default); + enteredA.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseA) == 1, Timeout10); + return 0; + }).Subscribe(_ => { }, _ => { }); + b.InvokeBlocking(_ => + { + enteredB.OnNext(Unit.Default); + enteredB.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseB) == 1, Timeout10); + return 0; + }).Subscribe(_ => { }, _ => { }); + + try + { + await enteredA.Should().Within(Timeout5).Emit(); + await enteredB.Should().Within(Timeout5).Emit(); + registry.TotalInFlight.Should().Be(2); + + var fired = new AsyncSubject(); + registry.Disposed.Subscribe(_ => + { + fired.OnNext(Unit.Default); + fired.OnCompleted(); + }); + registry.Dispose(); - releaseA.Set(); - fired.Wait(300, TestContext.Current.CancellationToken).Should().BeFalse( - "one pool finishing is not every pool finishing"); + Volatile.Write(ref releaseA, 1); + await fired.Should().NotEmit(300.Milliseconds(), + "one pool finishing is not every pool finishing"); - releaseB.Set(); - var total = await registry.Disposed.Timeout(Timeout10).FirstAsync().Await(TestContext.Current.CancellationToken); - total.Should().Be(0, "both pools joined — the silo may release"); + Volatile.Write(ref releaseB, 1); + var total = await registry.Disposed.Timeout(Timeout10).FirstAsync().Await(TestContext.Current.CancellationToken); + total.Should().Be(0, "both pools joined — the silo may release"); + } + finally + { + Volatile.Write(ref releaseA, 1); + Volatile.Write(ref releaseB, 1); + } } /// @@ -213,15 +266,16 @@ public async Task A_leaf_issued_after_disposal_is_cancelled_not_ObjectDisposed() public async Task SubscribeThroughPool_holds_a_slot_and_Drain_joins_the_in_flight_subscribe() { using var pool = new IoPool(4); - var subscribeEntered = new ManualResetEventSlim(); - var releaseSubscribe = new ManualResetEventSlim(); + var subscribeEntered = new AsyncSubject(); + var releaseSubscribe = 0; var subscribeFinished = false; // A source whose SUBSCRIBE blocks — stands in for the initial-emission → CreateHub window. var source = Observable.Create(obs => { - subscribeEntered.Set(); - releaseSubscribe.Wait(Timeout5); + subscribeEntered.OnNext(Unit.Default); + subscribeEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseSubscribe) == 1, Timeout5); subscribeFinished = true; obs.OnNext(1); return System.Reactive.Disposables.Disposable.Empty; @@ -229,22 +283,29 @@ public async Task SubscribeThroughPool_holds_a_slot_and_Drain_joins_the_in_fligh using var sub = pool.SubscribeThroughPool(source).Subscribe(_ => { }); - Assert.True(subscribeEntered.Wait(Timeout5), "the subscribe must run on the pool"); - Assert.True(SpinWait.SpinUntil(() => pool.CurrentInFlight == 1, Timeout5), - "the subscribe must hold a pool slot (tracked) for its duration — else the drain can't see it"); - - // Drain() must JOIN: block until the in-flight subscribe releases its slot. - var drainDone = Task.Run(() => pool.Drain()); - // Confirm Drain does NOT complete while the subscribe still holds a slot (the owning scope must - // not dispose yet) — a "wait to confirm nothing happened" negative check. - var winner = await Task.WhenAny(drainDone, Task.Delay(TimeSpan.FromMilliseconds(200))); - Assert.NotSame(drainDone, winner); - - releaseSubscribe.Set(); - - await drainDone.WaitAsync(Timeout5); - Assert.True(subscribeFinished, "Drain must have JOINED the in-flight subscribe before returning"); - Assert.Equal(0, pool.CurrentInFlight); + try + { + await subscribeEntered.Should().Within(Timeout5).Emit("the subscribe must run on the pool"); + Assert.True(SpinWait.SpinUntil(() => pool.CurrentInFlight == 1, Timeout5), + "the subscribe must hold a pool slot (tracked) for its duration — else the drain can't see it"); + + // Drain() must JOIN: block until the in-flight subscribe releases its slot. + var drainDone = Task.Run(() => pool.Drain()); + // Confirm Drain does NOT complete while the subscribe still holds a slot (the owning scope must + // not dispose yet) — a "wait to confirm nothing happened" negative check. + var winner = await Task.WhenAny(drainDone, Task.Delay(TimeSpan.FromMilliseconds(200))); + Assert.NotSame(drainDone, winner); + + Volatile.Write(ref releaseSubscribe, 1); + + await drainDone.WaitAsync(Timeout5); + Assert.True(subscribeFinished, "Drain must have JOINED the in-flight subscribe before returning"); + Assert.Equal(0, pool.CurrentInFlight); + } + finally + { + Volatile.Write(ref releaseSubscribe, 1); + } } [Fact] @@ -283,10 +344,10 @@ Task Run() => pool.Invoke(async ct => } [Fact] - public void Invoke_runs_the_leaf_on_the_threadpool_not_the_subscriber() + public async Task Invoke_runs_the_leaf_on_the_threadpool_not_the_subscriber() { using var pool = new IoPool(2); - AssertLeafRunsOffSubscriber(io => pool.Invoke(io)); + await AssertLeafRunsOffSubscriber(io => pool.Invoke(io)); } [Fact] @@ -295,7 +356,7 @@ public async Task InvokeBlocking_caps_concurrency_and_runs_off_thread() const int cap = 3; const int total = 12; using var pool = new IoPool(cap); - using var release = new ManualResetEventSlim(false); + var release = 0; var current = 0; var max = 0; var maxLock = new object(); @@ -308,21 +369,31 @@ public async Task InvokeBlocking_caps_concurrency_and_runs_off_thread() if (Environment.CurrentManagedThreadId == callingThread) ranOffThread = false; var c = Interlocked.Increment(ref current); lock (maxLock) { if (c > max) max = c; } - release.Wait(ct); // blocks a real scheduler thread + // blocks a real scheduler thread — that occupancy IS what the cap is measured on + SpinWait.SpinUntil( + () => Volatile.Read(ref release) == 1 || ct.IsCancellationRequested, Timeout10); + ct.ThrowIfCancellationRequested(); Interlocked.Decrement(ref current); return c; }).Await()).ToArray(); - SpinWait.SpinUntil(() => Volatile.Read(ref current) == cap, Timeout5) - .Should().BeTrue("the dedicated scheduler should admit exactly the cap concurrently"); - pool.CurrentInFlight.Should().Be(cap); - - release.Set(); - await Task.WhenAll(tasks).WaitAsync(Timeout10, TestContext.Current.CancellationToken); - - max.Should().Be(cap, "the limited-concurrency scheduler must never exceed the cap"); - ranOffThread.Should().BeTrue(); - pool.CurrentInFlight.Should().Be(0); + try + { + SpinWait.SpinUntil(() => Volatile.Read(ref current) == cap, Timeout5) + .Should().BeTrue("the dedicated scheduler should admit exactly the cap concurrently"); + pool.CurrentInFlight.Should().Be(cap); + + Volatile.Write(ref release, 1); + await Task.WhenAll(tasks).WaitAsync(Timeout10, TestContext.Current.CancellationToken); + + max.Should().Be(cap, "the limited-concurrency scheduler must never exceed the cap"); + ranOffThread.Should().BeTrue(); + pool.CurrentInFlight.Should().Be(0); + } + finally + { + Volatile.Write(ref release, 1); + } } // 🚨 REPRO for the #613 family: Drain()'s contract says "0 means the join is REAL: no pool @@ -338,36 +409,45 @@ public async Task InvokeBlocking_caps_concurrency_and_runs_off_thread() public async Task Drain_joins_InvokeBlocking_leaves_too() { using var pool = new IoPool(2); - using var entered = new ManualResetEventSlim(false); - using var release = new ManualResetEventSlim(false); + var entered = new AsyncSubject(); + var release = 0; pool.InvokeBlocking(_ => { - entered.Set(); - release.Wait(Timeout10); // deliberately ignores the token + entered.OnNext(Unit.Default); + entered.OnCompleted(); + // deliberately ignores the token — that is the case Drain's non-zero return reports + SpinWait.SpinUntil(() => Volatile.Read(ref release) == 1, Timeout10); return 0; }).Subscribe(_ => { }, _ => { }); - entered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - pool.CurrentInFlight.Should().Be(1, "a running blocking leaf must be visible as in-flight"); - - // Drain on ANOTHER thread and assert it does NOT return while the leaf runs. That is the - // contract — "0 means no pool thread is still running" — and it makes the repro fast: the - // earlier shape let Drain sit out the leaf's full 10s hold to observe the same thing. - // (Copilot review, #1334.) - var drain = Task.Run(pool.Drain, TestContext.Current.CancellationToken); - // await, never Task.Wait/.Result — CI's analyzers (xUnit1031) reject blocking task ops in a - // test, and they are right: this test exists to prove a JOIN, so it must not itself block. - var raced = await Task.WhenAny(drain, Task.Delay(300, TestContext.Current.CancellationToken)); - raced.Should().NotBeSameAs(drain, - "Drain must block while a blocking leaf is still executing — a blocking leaf holds no " - + "gate permit, so re-acquiring the gate joins nothing and Drain used to return 0 here"); - - release.Set(); - - var residual = await drain.WaitAsync(Timeout5, TestContext.Current.CancellationToken); - residual.Should().Be(0, "the leaf unwound within the budget — nothing to report"); - pool.CurrentInFlight.Should().Be(0); + try + { + await entered.Should().Within(Timeout5).Emit(); + pool.CurrentInFlight.Should().Be(1, "a running blocking leaf must be visible as in-flight"); + + // Drain on ANOTHER thread and assert it does NOT return while the leaf runs. That is the + // contract — "0 means no pool thread is still running" — and it makes the repro fast: the + // earlier shape let Drain sit out the leaf's full 10s hold to observe the same thing. + // (Copilot review, #1334.) + var drain = Task.Run(pool.Drain, TestContext.Current.CancellationToken); + // await, never Task.Wait/.Result — CI's analyzers (xUnit1031) reject blocking task ops in a + // test, and they are right: this test exists to prove a JOIN, so it must not itself block. + var raced = await Task.WhenAny(drain, Task.Delay(300, TestContext.Current.CancellationToken)); + raced.Should().NotBeSameAs(drain, + "Drain must block while a blocking leaf is still executing — a blocking leaf holds no " + + "gate permit, so re-acquiring the gate joins nothing and Drain used to return 0 here"); + + Volatile.Write(ref release, 1); + + var residual = await drain.WaitAsync(Timeout5, TestContext.Current.CancellationToken); + residual.Should().Be(0, "the leaf unwound within the budget — nothing to report"); + pool.CurrentInFlight.Should().Be(0); + } + finally + { + Volatile.Write(ref release, 1); + } } /// @@ -381,44 +461,54 @@ public async Task Drain_joins_InvokeBlocking_leaves_too() public async Task Drain_joinsABlockingLeaf_evenWhileAnAsyncLeafIsRunning() { using var pool = new IoPool(4); - using var asyncEntered = new ManualResetEventSlim(false); - using var blockingEntered = new ManualResetEventSlim(false); - using var release = new ManualResetEventSlim(false); + var asyncEntered = new AsyncSubject(); + var blockingEntered = new AsyncSubject(); + var release = 0; var asyncGate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - // An ASYNC leaf, in flight for the whole test — it holds the shared counter above zero. - pool.Invoke(_ => + try { - asyncEntered.Set(); - return asyncGate.Task; - }).Subscribe(_ => { }, _ => { }); - asyncEntered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - - // …and now a BLOCKING leaf on top of it. - pool.InvokeBlocking(_ => + // An ASYNC leaf, in flight for the whole test — it holds the shared counter above zero. + pool.Invoke(_ => + { + asyncEntered.OnNext(Unit.Default); + asyncEntered.OnCompleted(); + return asyncGate.Task; + }).Subscribe(_ => { }, _ => { }); + await asyncEntered.Should().Within(Timeout5).Emit(); + + // …and now a BLOCKING leaf on top of it. + pool.InvokeBlocking(_ => + { + blockingEntered.OnNext(Unit.Default); + blockingEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref release) == 1, Timeout10); + return 0; + }).Subscribe(_ => { }, _ => { }); + await blockingEntered.Should().Within(Timeout5).Emit(); + + var drain = Task.Run(pool.Drain, TestContext.Current.CancellationToken); + var raced = await Task.WhenAny(drain, Task.Delay(300, TestContext.Current.CancellationToken)); + raced.Should().NotBeSameAs(drain, + "the blocking leaf is still running, and a concurrently-running ASYNC leaf must not make " + + "the blocking join believe the pool is idle"); + + Volatile.Write(ref release, 1); + asyncGate.TrySetResult(0); + + // Assert the VALUE, not just that it returned: both leaves unwound inside the budget, so a + // non-zero residual would mean the join reported a survivor that had already finished — the + // other direction of the shared-counter defect. (Copilot review, #1338.) + var residual = await drain.WaitAsync(Timeout5, TestContext.Current.CancellationToken); + residual.Should().Be(0, + "both the blocking and the async leaf unwound within the budget — nothing to report"); + pool.CurrentInFlight.Should().Be(0); + } + finally { - blockingEntered.Set(); - release.Wait(Timeout10); - return 0; - }).Subscribe(_ => { }, _ => { }); - blockingEntered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - - var drain = Task.Run(pool.Drain, TestContext.Current.CancellationToken); - var raced = await Task.WhenAny(drain, Task.Delay(300, TestContext.Current.CancellationToken)); - raced.Should().NotBeSameAs(drain, - "the blocking leaf is still running, and a concurrently-running ASYNC leaf must not make " - + "the blocking join believe the pool is idle"); - - release.Set(); - asyncGate.TrySetResult(0); - - // Assert the VALUE, not just that it returned: both leaves unwound inside the budget, so a - // non-zero residual would mean the join reported a survivor that had already finished — the - // other direction of the shared-counter defect. (Copilot review, #1338.) - var residual = await drain.WaitAsync(Timeout5, TestContext.Current.CancellationToken); - residual.Should().Be(0, - "both the blocking and the async leaf unwound within the budget — nothing to report"); - pool.CurrentInFlight.Should().Be(0); + Volatile.Write(ref release, 1); + asyncGate.TrySetResult(0); + } } @@ -452,19 +542,20 @@ public async Task Invoke_releases_the_slot_on_exception() } [Fact] - public void Invoke_releases_the_slot_when_subscription_disposed_before_completion() + public async Task Invoke_releases_the_slot_when_subscription_disposed_before_completion() { using var pool = new IoPool(1); - using var entered = new ManualResetEventSlim(false); + var entered = new AsyncSubject(); var sub = pool.Invoke(async ct => { - entered.Set(); + entered.OnNext(Unit.Default); + entered.OnCompleted(); await Task.Delay(System.Threading.Timeout.Infinite, ct); // cancellable hold return 0; }).Subscribe(_ => { }, _ => { }); - entered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); + await entered.Should().Within(Timeout5).Emit(); pool.CurrentInFlight.Should().Be(1); sub.Dispose(); // cancels ct → Task.Delay throws → finally releases the slot @@ -482,18 +573,19 @@ public void Invoke_releases_the_slot_when_subscription_disposed_before_completio public async Task Drain_cancels_in_flight_leaves_and_joins_synchronously() { using var pool = new IoPool(2); - using var entered = new ManualResetEventSlim(false); + var entered = new AsyncSubject(); var cancelled = false; pool.Invoke(async ct => { - entered.Set(); + entered.OnNext(Unit.Default); + entered.OnCompleted(); try { await Task.Delay(System.Threading.Timeout.Infinite, ct); } // never completes on its own catch (OperationCanceledException) { cancelled = true; throw; } return 0; }).Subscribe(_ => { }, _ => { }); - entered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); + await entered.Should().Within(Timeout5).Emit(); pool.CurrentInFlight.Should().Be(1); pool.Drain(); // cancel the leaf + JOIN — returns only once it has unwound @@ -537,8 +629,8 @@ public async Task Drain_cancels_in_flight_leaves_and_joins_synchronously() public async Task Drain_terminates_a_SubscribeThroughPool_leg_it_cancels() { using var pool = new IoPool(1); - var legAEntered = new ManualResetEventSlim(); - var releaseLegA = new ManualResetEventSlim(); + var legAEntered = new AsyncSubject(); + var releaseLegA = 0; // Whether leg A was RELEASED rather than timing out. If the wait ever timed out, leg A would // free the permit on its own and the "leg B is parked on the gate" premise would be false — // the test could then pass for the wrong reason. Asserted on the test thread below. @@ -546,70 +638,86 @@ public async Task Drain_terminates_a_SubscribeThroughPool_leg_it_cancels() var legA = Observable.Create(obs => { - legAEntered.Set(); - if (releaseLegA.Wait(Timeout10)) Interlocked.Exchange(ref legAReleasedCleanly, 1); + legAEntered.OnNext(Unit.Default); + legAEntered.OnCompleted(); + if (SpinWait.SpinUntil(() => Volatile.Read(ref releaseLegA) == 1, Timeout10)) + Interlocked.Exchange(ref legAReleasedCleanly, 1); obs.OnNext(1); return System.Reactive.Disposables.Disposable.Empty; }); using var subA = pool.SubscribeThroughPool(legA).Subscribe(_ => { }); - Assert.True(legAEntered.Wait(Timeout5), "leg A must be inside its subscribe, holding the only permit"); - - // Leg B can never acquire the permit — it is parked on the gate when the drain cancels. - var legBTerminated = new ManualResetEventSlim(); - // Written from the pool's subscribe thread, read from the test thread — Interlocked/Volatile, - // not a plain bool: a stale read of `false` is the PASSING value here, so an unsynchronised - // field could hide a real regression (leg B being subscribed despite the drain). - var legBSubscribed = 0; - using var subB = pool - .SubscribeThroughPool(Observable.Create(obs => - { - Interlocked.Exchange(ref legBSubscribed, 1); - obs.OnNext(2); - return System.Reactive.Disposables.Disposable.Empty; - })) - .Finally(legBTerminated.Set) - .Subscribe(_ => { }, _ => { }); - - Assert.True(SpinWait.SpinUntil(() => pool.CurrentInFlight == 1, Timeout5), - "exactly leg A holds the permit; leg B must still be waiting on the gate"); - - var drainDone = Task.Run(pool.Drain, TestContext.Current.CancellationToken); - - // 🚨 ESTABLISH the ordering the assertions below depend on — do not assume it. - // - // `Task.Run` only SCHEDULES Drain; it says nothing about whether Drain has reached its - // `_poolCts.Cancel()`. Releasing leg A here — as this test used to — frees the only permit - // while the cancel may still be queued behind a busy thread pool, and SemaphoreSlim then - // legitimately hands that permit to the leg B waiter already queued on it. Leg B's - // `ct.ThrowIfCancellationRequested()` guard passes (nothing is cancelled yet), it subscribes, - // and the test fails on `legBSubscribed == 1` — reporting a product regression when the - // product did exactly the right thing. Observed on a loaded CI runner (six shards on one - // box); the assertion asserted an ordering the test never created. - // - // Leg B's TERMINATION is the observable proof that the cancel has happened: it is parked in - // `_gate.WaitAsync(linked)`, so `_poolCts.Cancel()` is what completes that wait as cancelled - // → OnCompleted → this `.Finally`. Waiting for it costs nothing when the pool is healthy and - // is what makes "the drain cancels BEFORE the permit is granted" true by construction rather - // than by scheduling luck. It does not deadlock: Drain blocks re-acquiring leg A's permit, - // which is independent of leg B's cancellation continuation. - Assert.True(legBTerminated.Wait(Timeout5), - "a leg the drain cancels MUST terminate (OnCompleted) so its .Finally runs — that callback " - + "is what releases RoutingGrain's in-flight route slot and advances OrderedRouteDispatcher's " - + "per-destination FIFO; swallowing the cancellation silently leaked both"); - - releaseLegA.Set(); - await drainDone.WaitAsync(Timeout10, TestContext.Current.CancellationToken); - - Volatile.Read(ref legAReleasedCleanly).Should().Be(1, - "leg A must have been RELEASED, not timed out — a timed-out wait would free the permit on " - + "its own and leg B would no longer be parked on the gate, so the test would prove nothing"); - // 🚨 THE REGRESSION GUARD — and it is now asserted on a cancellation that PROVABLY preceded - // the permit becoming free (see the wait above), so a pass means the drain refused leg B - // rather than merely outrunning it. Before the termination fix this never fired at all and - // the test hung to its timeout. - Volatile.Read(ref legBSubscribed).Should().Be(0, - "the drain cancels before the permit is granted, so leg B's source must never be subscribed"); + try + { + await legAEntered.Should().Within(Timeout5) + .Emit("leg A must be inside its subscribe, holding the only permit"); + + // Leg B can never acquire the permit — it is parked on the gate when the drain cancels. + var legBTerminated = new AsyncSubject(); + // Written from the pool's subscribe thread, read from the test thread — Interlocked/Volatile, + // not a plain bool: a stale read of `false` is the PASSING value here, so an unsynchronised + // field could hide a real regression (leg B being subscribed despite the drain). + var legBSubscribed = 0; + using var subB = pool + .SubscribeThroughPool(Observable.Create(obs => + { + Interlocked.Exchange(ref legBSubscribed, 1); + obs.OnNext(2); + return System.Reactive.Disposables.Disposable.Empty; + })) + .Finally(() => + { + legBTerminated.OnNext(Unit.Default); + legBTerminated.OnCompleted(); + }) + .Subscribe(_ => { }, _ => { }); + + Assert.True(SpinWait.SpinUntil(() => pool.CurrentInFlight == 1, Timeout5), + "exactly leg A holds the permit; leg B must still be waiting on the gate"); + + var drainDone = Task.Run(pool.Drain, TestContext.Current.CancellationToken); + + // 🚨 ESTABLISH the ordering the assertions below depend on — do not assume it. + // + // `Task.Run` only SCHEDULES Drain; it says nothing about whether Drain has reached its + // `_poolCts.Cancel()`. Releasing leg A here — as this test used to — frees the only permit + // while the cancel may still be queued behind a busy thread pool, and SemaphoreSlim then + // legitimately hands that permit to the leg B waiter already queued on it. Leg B's + // `ct.ThrowIfCancellationRequested()` guard passes (nothing is cancelled yet), it subscribes, + // and the test fails on `legBSubscribed == 1` — reporting a product regression when the + // product did exactly the right thing. Observed on a loaded CI runner (six shards on one + // box); the assertion asserted an ordering the test never created. + // + // Leg B's TERMINATION is the observable proof that the cancel has happened: it is parked in + // `_gate.WaitAsync(linked)`, so `_poolCts.Cancel()` is what completes that wait as cancelled + // → OnCompleted → this `.Finally`. Waiting for it costs nothing when the pool is healthy and + // is what makes "the drain cancels BEFORE the permit is granted" true by construction rather + // than by scheduling luck. It does not deadlock: Drain blocks re-acquiring leg A's permit, + // which is independent of leg B's cancellation continuation. + await legBTerminated.Should().Within(Timeout5).Emit( + "a leg the drain cancels MUST terminate (OnCompleted) so its .Finally runs — that callback " + + "is what releases RoutingGrain's in-flight route slot and advances OrderedRouteDispatcher's " + + "per-destination FIFO; swallowing the cancellation silently leaked both"); + + Volatile.Write(ref releaseLegA, 1); + await drainDone.WaitAsync(Timeout10, TestContext.Current.CancellationToken); + + Volatile.Read(ref legAReleasedCleanly).Should().Be(1, + "leg A must have been RELEASED, not timed out — a timed-out wait would free the permit on " + + "its own and leg B would no longer be parked on the gate, so the test would prove nothing"); + // 🚨 THE REGRESSION GUARD — and it is now asserted on a cancellation that PROVABLY preceded + // the permit becoming free (see the wait above), so a pass means the drain refused leg B + // rather than merely outrunning it. Before the termination fix this never fired at all and + // the test hung to its timeout. + Volatile.Read(ref legBSubscribed).Should().Be(0, + "the drain cancels before the permit is granted, so leg B's source must never be subscribed"); + } + finally + { + // 🚨 Leg A is a deliberately parked subscribe. Releasing it here means an assertion that + // throws above cannot leave it holding a pool thread for the full Timeout10. + Volatile.Write(ref releaseLegA, 1); + } } /// @@ -640,9 +748,11 @@ public async Task Drain_terminates_a_SubscribeThroughPool_leg_it_cancels() public async Task Drain_terminates_a_SubscribeThroughPool_leg_that_already_subscribed() { using var pool = new IoPool(2); - var subscribed = new ManualResetEventSlim(); - var innerDisposed = new ManualResetEventSlim(); - var terminated = new ManualResetEventSlim(); + // Three signals the PRODUCT emits and the test consumes — nothing is parked here, so all + // three are AsyncSubjects awaited through the assertion helpers. + var subscribed = new AsyncSubject(); + var innerDisposed = new AsyncSubject(); + var terminated = new AsyncSubject(); // A LONG-LIVED source, the shape every real SubscribeThroughPool caller has: the subscribe // returns promptly (releasing the pool permit) and the subscription then stays open, @@ -650,15 +760,24 @@ public async Task Drain_terminates_a_SubscribeThroughPool_leg_that_already_subsc // drains, which is what host shutdown does to every live route. var source = Observable.Create(obs => { - subscribed.Set(); - return System.Reactive.Disposables.Disposable.Create(innerDisposed.Set); + subscribed.OnNext(Unit.Default); + subscribed.OnCompleted(); + return System.Reactive.Disposables.Disposable.Create(() => + { + innerDisposed.OnNext(Unit.Default); + innerDisposed.OnCompleted(); + }); }); using var sub = pool.SubscribeThroughPool(source) - .Finally(terminated.Set) + .Finally(() => + { + terminated.OnNext(Unit.Default); + terminated.OnCompleted(); + }) .Subscribe(_ => { }, _ => { }); - Assert.True(subscribed.Wait(Timeout5), + await subscribed.Should().Within(Timeout5).Emit( "the source must have been subscribed before the drain — this test is about the window " + "AFTER the subscribe completed"); Assert.True(SpinWait.SpinUntil(() => pool.CurrentInFlight == 0, Timeout5), @@ -667,12 +786,12 @@ public async Task Drain_terminates_a_SubscribeThroughPool_leg_that_already_subsc var drainDone = Task.Run(pool.Drain, TestContext.Current.CancellationToken); - Assert.True(innerDisposed.Wait(Timeout5), + await innerDisposed.Should().Within(Timeout5).Emit( "the drain must tear the live inner subscription down — asserted FIRST so a failure on " + "termination below is attributable to the missing terminal, not to a drain that never ran"); // 🚨 THE REGRESSION GUARD. Before the fix this never fired: the drain disposed the inner // subscription and emitted nothing, so `.Finally` never ran and this waited out its budget. - Assert.True(terminated.Wait(Timeout5), + await terminated.Should().Within(Timeout5).Emit( "a leg the drain tears down AFTER its subscribe completed MUST terminate (OnCompleted) so " + "its .Finally runs — that callback releases RoutingGrain's in-flight route slot and " + "advances OrderedRouteDispatcher's per-destination FIFO; disposing the subscription " @@ -685,9 +804,9 @@ public async Task Drain_terminates_a_SubscribeThroughPool_leg_that_already_subsc } [Fact] - public void Unbounded_fallback_runs_the_leaf_on_the_threadpool() + public async Task Unbounded_fallback_runs_the_leaf_on_the_threadpool() { - AssertLeafRunsOffSubscriber(io => IoPool.Unbounded.Invoke(io)); + await AssertLeafRunsOffSubscriber(io => IoPool.Unbounded.Invoke(io)); IoPool.Unbounded.CurrentInFlight.Should().Be(0); } @@ -746,13 +865,19 @@ public async Task Unbounded_fallback_carries_caller_AsyncLocal_into_the_pooled_b // dedicated Thread is never a ThreadPool thread, so checking // IsThreadPoolThread on the body avoids the flaky "different thread id" guess // (the ThreadPool can otherwise reuse an awaiting caller's thread). - private static void AssertLeafRunsOffSubscriber( + private static async Task AssertLeafRunsOffSubscriber( Func>, IObservable> invoke) { var subscriberThread = -1; var bodyThread = -1; var bodyOnThreadPool = false; - using var done = new ManualResetEventSlim(false); + var done = new AsyncSubject(); + + void Done() + { + done.OnNext(Unit.Default); + done.OnCompleted(); + } var t = new Thread(() => { @@ -762,11 +887,11 @@ private static void AssertLeafRunsOffSubscriber( bodyThread = Environment.CurrentManagedThreadId; bodyOnThreadPool = Thread.CurrentThread.IsThreadPoolThread; return Task.FromResult(0); - }).Subscribe(_ => done.Set(), _ => done.Set()); + }).Subscribe(_ => Done(), _ => Done()); }) { IsBackground = true }; t.Start(); - done.Wait(Timeout5).Should().BeTrue("the leaf should complete"); + await done.Should().Within(Timeout5).Emit("the leaf should complete"); bodyOnThreadPool.Should().BeTrue("the leaf must run on the ThreadPool, not the subscriber's thread"); bodyThread.Should().NotBe(subscriberThread); } @@ -794,8 +919,8 @@ public async Task Drain_runsPooledSubscriptionTeardown_offTheCallersThread() { using var pool = new IoPool(4); var source = new Subject(); - using var teardownEntered = new ManualResetEventSlim(false); - using var releaseTeardown = new ManualResetEventSlim(false); + var teardownEntered = new AsyncSubject(); + var releaseTeardown = 0; var teardownThread = 0; using var subscription = pool.SubscribeThroughPool(source).Subscribe( @@ -803,34 +928,44 @@ public async Task Drain_runsPooledSubscriptionTeardown_offTheCallersThread() () => { teardownThread = Environment.CurrentManagedThreadId; - teardownEntered.Set(); - releaseTeardown.Wait(Timeout10); + teardownEntered.OnNext(Unit.Default); + teardownEntered.OnCompleted(); + // The teardown leg simply does not return promptly — that is the whole + // precondition of the wedge this test repros. + SpinWait.SpinUntil(() => Volatile.Read(ref releaseTeardown) == 1, Timeout10); }); - SpinWait.SpinUntil(() => source.HasObservers, Timeout5) - .Should().BeTrue("the pooled subscribe must have completed before the drain"); - - var drainThread = 0; - var drain = Task.Run( - () => - { - drainThread = Environment.CurrentManagedThreadId; - return pool.Drain(); - }, - TestContext.Current.CancellationToken); - - teardownEntered.Wait(Timeout5, TestContext.Current.CancellationToken) - .Should().BeTrue("Drain cancels the pool, which terminates every pooled subscription"); - - teardownThread.Should().NotBe(drainThread, - "a pooled subscription's downstream teardown must never run on the thread that called " - + "Drain() — that thread is the mesh-teardown thread, and Cancel() executes registered " - + "callbacks synchronously on whoever calls it (#2394)"); - - releaseTeardown.Set(); - - var residual = await drain.WaitAsync(Timeout10, TestContext.Current.CancellationToken); - residual.Should().Be(0, "the teardown finished inside the budget — nothing to report"); + try + { + SpinWait.SpinUntil(() => source.HasObservers, Timeout5) + .Should().BeTrue("the pooled subscribe must have completed before the drain"); + + var drainThread = 0; + var drain = Task.Run( + () => + { + drainThread = Environment.CurrentManagedThreadId; + return pool.Drain(); + }, + TestContext.Current.CancellationToken); + + await teardownEntered.Should().Within(Timeout5) + .Emit("Drain cancels the pool, which terminates every pooled subscription"); + + teardownThread.Should().NotBe(drainThread, + "a pooled subscription's downstream teardown must never run on the thread that called " + + "Drain() — that thread is the mesh-teardown thread, and Cancel() executes registered " + + "callbacks synchronously on whoever calls it (#2394)"); + + Volatile.Write(ref releaseTeardown, 1); + + var residual = await drain.WaitAsync(Timeout10, TestContext.Current.CancellationToken); + residual.Should().Be(0, "the teardown finished inside the budget — nothing to report"); + } + finally + { + Volatile.Write(ref releaseTeardown, 1); + } } /// @@ -841,34 +976,42 @@ public async Task Drain_runsPooledSubscriptionTeardown_offTheCallersThread() /// unwinding needs pool threads — the starvation deadlock the method's own comment describes. /// [Fact(Timeout = 30_000)] - public void Dispose_doesNotBlockOnASlowPooledSubscriptionTeardown() + public async Task Dispose_doesNotBlockOnASlowPooledSubscriptionTeardown() { var pool = new IoPool(4); var source = new Subject(); - using var teardownEntered = new ManualResetEventSlim(false); - using var releaseTeardown = new ManualResetEventSlim(false); + var teardownEntered = new AsyncSubject(); + var releaseTeardown = 0; using var subscription = pool.SubscribeThroughPool(source).Subscribe( _ => { }, () => { - teardownEntered.Set(); - releaseTeardown.Wait(Timeout5); + teardownEntered.OnNext(Unit.Default); + teardownEntered.OnCompleted(); + // A SLOW teardown leg — the condition under which Dispose used to block. + SpinWait.SpinUntil(() => Volatile.Read(ref releaseTeardown) == 1, Timeout5); }); - SpinWait.SpinUntil(() => source.HasObservers, Timeout5) - .Should().BeTrue("the pooled subscribe must have completed before disposal"); - - var sw = Stopwatch.StartNew(); - pool.Dispose(); - sw.Stop(); - - teardownEntered.Wait(Timeout5).Should().BeTrue("disposal must still terminate the subscription"); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(2), - "Dispose must return immediately — the WAIT belongs on Disposed, and running the " - + "pooled subscriptions' teardown inline made Dispose block for as long as they took"); - - releaseTeardown.Set(); + try + { + SpinWait.SpinUntil(() => source.HasObservers, Timeout5) + .Should().BeTrue("the pooled subscribe must have completed before disposal"); + + var sw = Stopwatch.StartNew(); + pool.Dispose(); + sw.Stop(); + + await teardownEntered.Should().Within(Timeout5) + .Emit("disposal must still terminate the subscription"); + sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(2), + "Dispose must return immediately — the WAIT belongs on Disposed, and running the " + + "pooled subscriptions' teardown inline made Dispose block for as long as they took"); + } + finally + { + Volatile.Write(ref releaseTeardown, 1); + } } /// @@ -879,30 +1022,38 @@ public void Dispose_doesNotBlockOnASlowPooledSubscriptionTeardown() /// the level it stopped at. /// [Fact] - public void AResidualNamesTheLeafThatWouldNotUnwind() + public async Task AResidualNamesTheLeafThatWouldNotUnwind() { var pool = new IoPool(2); - using var entered = new ManualResetEventSlim(false); - using var release = new ManualResetEventSlim(false); + var entered = new AsyncSubject(); + var release = 0; // A leaf that ignores its token — exactly the defect the residual exists to report. pool.InvokeBlocking(_ => { - entered.Set(); - release.Wait(Timeout10); + entered.OnNext(Unit.Default); + entered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref release) == 1, Timeout10); return 0; }).Subscribe(_ => { }, _ => { }); - entered.Wait(Timeout5, TestContext.Current.CancellationToken).Should().BeTrue(); - - var sites = pool.PendingLeafSites; - sites.Should().NotBeEmpty("a leaf is in flight, so the pool must be able to say WHICH"); - string.Join(" | ", sites).Should().Contain(nameof(AResidualNamesTheLeafThatWouldNotUnwind), - "the lambda's compiler-generated name carries its ENCLOSING method — that is what turns " - + "'AgentStore=1' into a pointer at the operation to fix"); - - release.Set(); - pool.Dispose(); + try + { + await entered.Should().Within(Timeout5).Emit(); + + var sites = pool.PendingLeafSites; + sites.Should().NotBeEmpty("a leaf is in flight, so the pool must be able to say WHICH"); + string.Join(" | ", sites).Should().Contain(nameof(AResidualNamesTheLeafThatWouldNotUnwind), + "the lambda's compiler-generated name carries its ENCLOSING method — that is what turns " + + "'AgentStore=1' into a pointer at the operation to fix"); + } + finally + { + // 🚨 In a `finally`: an assertion that throws above must not leave the leaf holding a + // pool thread for the full Timeout10, into the next test. + Volatile.Write(ref release, 1); + pool.Dispose(); + } } /// A pool that drains cleanly reports no sites — the diagnostic is not noise. diff --git a/test/MeshWeaver.Hosting.Test/LiveQueryForeignTrampolineTest.cs b/test/MeshWeaver.Hosting.Test/LiveQueryForeignTrampolineTest.cs index 62b50a0dc5..d54ad963a6 100644 --- a/test/MeshWeaver.Hosting.Test/LiveQueryForeignTrampolineTest.cs +++ b/test/MeshWeaver.Hosting.Test/LiveQueryForeignTrampolineTest.cs @@ -71,7 +71,10 @@ private static void RunProbe(string query) adapter.Write(new MeshNode("one", "probe/_Usage") { NodeType = "TokenUsage" }, Options) .Subscribe(); var provider = new StorageAdapterMeshQueryProvider(adapter); - using var initial = new ManualResetEventSlim(false); + // 🚨 A volatile flag, not a hand-woven gate. Blocking the trampoline frame IS the subject + // (see below), and a flag polled under a bounded SpinUntil expresses that with no kernel + // handle to dispose and no observable→blocking bridge. + var initial = 0; var insideTrampoline = false; // Schedule() IS the trampoline: inside this action CurrentThreadScheduler reports that one @@ -81,13 +84,13 @@ private static void RunProbe(string query) insideTrampoline = !CurrentThreadScheduler.IsScheduleRequired; using var sub = provider .Query(MeshQueryRequest.FromQueries([query], "system-security"), Options) - .Subscribe(c => { if (c.ChangeType == QueryChangeType.Initial) initial.Set(); }); + .Subscribe(c => { if (c.ChangeType == QueryChangeType.Initial) Volatile.Write(ref initial, 1); }); // 🚨 Blocking HERE is the point, not a shortcut: the stranded walk could only ever run // after this frame returns to the trampoline that owns it, so a caller that waits for // its own Initial is the shape that deadlocked. A budget generous enough that only a // never-scheduled walk can exhaust it — this work is microseconds. - Assert.True(initial.Wait(TimeSpan.FromSeconds(5)), + Assert.True(SpinWait.SpinUntil(() => Volatile.Read(ref initial) == 1, TimeSpan.FromSeconds(5)), $"Initial never arrived for [{query}] — the scope walk was queued on the caller's " + "trampoline instead of running inline, so it can never run (#2377)"); }); diff --git a/test/MeshWeaver.Hosting.Test/LiveQueryHandoffDropTest.cs b/test/MeshWeaver.Hosting.Test/LiveQueryHandoffDropTest.cs index 117a530e90..efec671034 100644 --- a/test/MeshWeaver.Hosting.Test/LiveQueryHandoffDropTest.cs +++ b/test/MeshWeaver.Hosting.Test/LiveQueryHandoffDropTest.cs @@ -4,8 +4,10 @@ using System.Linq; using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Text.Json; using System.Threading; +using System.Threading.Tasks; using MeshWeaver.Hosting.Persistence; using MeshWeaver.Hosting.Persistence.Query; using MeshWeaver.Mesh; @@ -121,8 +123,24 @@ public Action PublishDeferred(DataChangeNotification n) private sealed class GatedAdapter(InMemoryStorageAdapter inner, SnapshotThenDeliverFeed feed) : IStorageAdapter { - public readonly ManualResetEventSlim ReadEntered = new(false); - public readonly ManualResetEventSlim ReleaseRead = new(true); + /// + /// 🚨 No hand-woven gate. The walk → test signal is an the + /// walk completes and the test awaits reactively; the test → walk release is a volatile + /// flag the parked walk polls under a bounded SpinUntil. Parking the walk IS the subject + /// here, so the park stays — what goes is the kernel handle and the disposal it needed. + /// + private readonly AsyncSubject readEntered = new(); + private int releaseRead = 1; + + /// Completes once the gated walk has parked. + public IObservable ReadEntered => readEntered; + + /// Closes the gate, so the next matching walk parks. + public void HoldRead() => Volatile.Write(ref releaseRead, 0); + + /// Opens the gate; idempotent, so a `finally` can always call it. + public void ReleaseRead() => Volatile.Write(ref releaseRead, 1); + /// Holds the FIRST walk of the query's own base path only — later reads run free. public string? GateOn; private int gateArmed = 1; @@ -147,8 +165,9 @@ private sealed class GatedAdapter(InMemoryStorageAdapter inner, SnapshotThenDeli { if (!string.Equals(parentPath, GateOn, StringComparison.OrdinalIgnoreCase)) return; if (Interlocked.Exchange(ref gateArmed, 0) != 1) return; - ReadEntered.Set(); - if (!ReleaseRead.Wait(TimeSpan.FromSeconds(10))) + readEntered.OnNext(Unit.Default); + readEntered.OnCompleted(); + if (!SpinWait.SpinUntil(() => Volatile.Read(ref releaseRead) == 1, TimeSpan.FromSeconds(10))) Volatile.Write(ref gateTimedOut, 1); }); @@ -193,23 +212,32 @@ public IObservable DeletePartitionObjects(string nodePath, string? subPath /// stay at 30 s and both stay FAIL-FAST: falling through silently would run the gated scenario /// against a still-cold pipeline and let it misreport its own timeout as a dropped row. /// - private static void WarmUpJitAndTypeLoad() + private static async Task WarmUpJitAndTypeLoad() { var warmInner = new InMemoryStorageAdapter(); var warmProvider = new StorageAdapterMeshQueryProvider(warmInner); - // Disposed deterministically (not left to Wait()'s lazily-allocated kernel WaitHandle to - // finalize) — this helper is also exercised repeatedly by ad-hoc repro loops, and an - // undisposed ManualResetEventSlim leaks its handle once Wait() forces it into existence. - using var warmInitial = new ManualResetEventSlim(false); - using var warmLive = new ManualResetEventSlim(false); + // 🚨 AsyncSubjects, not events: nothing to dispose, nothing that can leak a kernel handle, + // and the waits below are reactive rather than parked on the calling thread. (This helper + // is also exercised repeatedly by ad-hoc repro loops, which is what made the undisposed + // ManualResetEventSlim handle visible in the first place.) + var warmInitial = new AsyncSubject(); + var warmLive = new AsyncSubject(); using (warmProvider .Query( MeshQueryRequest.FromQueries(["path:warmup/_Usage scope:children"], "system-security"), Options) .Subscribe(c => { - if (c.ChangeType == QueryChangeType.Initial) warmInitial.Set(); - else warmLive.Set(); + if (c.ChangeType == QueryChangeType.Initial) + { + warmInitial.OnNext(Unit.Default); + warmInitial.OnCompleted(); + } + else + { + warmLive.OnNext(Unit.Default); + warmLive.OnCompleted(); + } })) { // Generous, uncontested-by-design budget: this phase primes the JIT, it does not pin @@ -218,21 +246,21 @@ private static void WarmUpJitAndTypeLoad() // would run the gated scenario below against a STILL-cold pipeline and could then // misreport its own timeout as "dropped row" — exactly the misleading signal this // warm-up exists to prevent. - Assert.True(warmInitial.Wait(TimeSpan.FromSeconds(30)), + await warmInitial.Should().Within(30.Seconds()).Emit( "warm-up: Initial never arrived — the environment is too degraded to even JIT-prime " + "this pipeline, so the gated assertions below cannot be trusted either"); warmInner.Write(new MeshNode("x", "warmup/_Usage") { NodeType = "TokenUsage" }, Options) .Subscribe(); - Assert.True(warmLive.Wait(TimeSpan.FromSeconds(30)), + await warmLive.Should().Within(30.Seconds()).Emit( "warm-up: live update never arrived — the environment is too degraded to even " + "JIT-prime this pipeline, so the gated assertions below cannot be trusted either"); } } [Fact] - public void A_notification_snapshotted_before_Initial_and_delivered_after_still_reaches_the_query() + public async Task A_notification_snapshotted_before_Initial_and_delivered_after_still_reaches_the_query() { - WarmUpJitAndTypeLoad(); + await WarmUpJitAndTypeLoad(); var inner = new InMemoryStorageAdapter(); var feed = new SnapshotThenDeliverFeed(); @@ -240,13 +268,15 @@ public void A_notification_snapshotted_before_Initial_and_delivered_after_still_ var provider = new StorageAdapterMeshQueryProvider(adapter); var changes = new List>(); - var initial = new ManualResetEventSlim(false); - var added = new ManualResetEventSlim(false); + // 🚨 Signals the subscriber thread produces and this test consumes — AsyncSubjects the + // producer completes, awaited through the assertion helpers; no hand-woven gate. + var initial = new AsyncSubject(); + var added = new AsyncSubject(); // Hold the query's own scope walk open so the Initial snapshot cannot be established // until we say so. adapter.GateOn = Base; - adapter.ReleaseRead.Reset(); + adapter.HoldRead(); // 🚨 Subscribe OFF the test thread. The provider's scope walk runs inline on the // subscribing thread, so parking it from the test thread would park the test itself. @@ -259,9 +289,16 @@ public void A_notification_snapshotted_before_Initial_and_delivered_after_still_ .Subscribe(c => { lock (changes) changes.Add(c); - if (c.ChangeType == QueryChangeType.Initial) initial.Set(); + if (c.ChangeType == QueryChangeType.Initial) + { + initial.OnNext(Unit.Default); + initial.OnCompleted(); + } else if (c.Items.Any(n => string.Equals(n.Path, ChildPath, StringComparison.OrdinalIgnoreCase))) - added.Set(); + { + added.OnNext(Unit.Default); + added.OnCompleted(); + } })) { IsBackground = true }; subscriber.Start(); @@ -269,7 +306,7 @@ public void A_notification_snapshotted_before_Initial_and_delivered_after_still_ try { // The query's ONE feed subscription is attached and its scope walk is parked. - Assert.True(adapter.ReadEntered.Wait(TimeSpan.FromSeconds(10)), + await adapter.ReadEntered.Should().Within(10.Seconds()).Emit( "the query never started its scope walk"); // The write COMMITS to the store (so any re-query can see it) … @@ -282,8 +319,8 @@ public void A_notification_snapshotted_before_Initial_and_delivered_after_still_ // Now let the Initial land. Its rows were read before the write, so it is EMPTY — the // query's only route to the row is the notification whose snapshot we already hold. - adapter.ReleaseRead.Set(); - Assert.True(initial.Wait(TimeSpan.FromSeconds(10)), "the query never emitted its Initial"); + adapter.ReleaseRead(); + await initial.Should().Within(10.Seconds()).Emit("the query never emitted its Initial"); // 🚨 The gate must have been RELEASED, not given up on. A timed-out gate lets the walk // continue by itself, which un-pins the Initial from the notification's snapshot — the @@ -302,7 +339,7 @@ public void A_notification_snapshotted_before_Initial_and_delivered_after_still_ // two-subscription handoff dropped on the floor. deliver(); - Assert.True(added.Wait(TimeSpan.FromSeconds(10)), + await added.Should().Within(10.Seconds()).Emit( "a change notification whose fan-out snapshot predates the Initial, delivered after " + "it, must still reach the live re-query. Dropping it loses the row PERMANENTLY: " + "nothing re-triggers the query, so the subscription never learns of a node a " @@ -310,7 +347,7 @@ public void A_notification_snapshotted_before_Initial_and_delivered_after_still_ } finally { - adapter.ReleaseRead.Set(); + adapter.ReleaseRead(); subscriber.Join(TimeSpan.FromSeconds(10)); sub?.Dispose(); } diff --git a/test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs b/test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs index 5dde9cdb11..9fd93d9876 100644 --- a/test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs +++ b/test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs @@ -80,17 +80,21 @@ protected override MessageHubConfiguration ConfigureClient(MessageHubConfigurati // on its own action block). If THIS subscribe is running on that hub's turn, the ping sits // in the inbox behind us and cannot be dequeued until we return — but we are (below) // waiting for it: the deadlock. Off the turn (the fix), the hub is free to answer it. - var pong = new ManualResetEventSlim(false); + // 🚨 A volatile flag, not a hand-woven gate: blocking the subscribing thread IS the + // subject here, but nothing needs a kernel handle to express it, and there is no + // observable→blocking bridge either. SpinUntil answers the same question the event's + // Wait(timeout) did — "did the round-trip resolve inside the budget" — as a bool. + var pong = 0; using var sub = host.Stream.Hub .Observe(new PingRequest(), o => o.WithTarget(host.Stream.Hub.Address)) .Take(1) - .Subscribe(_ => pong.Set()); + .Subscribe(_ => Volatile.Write(ref pong, 1)); // Blocks the SUBSCRIBING thread until the round-trip resolves — the sync-over-mesh shape a // query-in-render collapses to when it is on the wrong scheduler. Bounded so a genuinely // wedged hub surfaces as "area never rendered" (null control → the .Within deadline fails // the test) instead of hanging the whole run. - var answered = pong.Wait(TimeSpan.FromSeconds(15)); + var answered = SpinWait.SpinUntil(() => Volatile.Read(ref pong) == 1, TimeSpan.FromSeconds(15)); observer.OnNext(answered ? (UiControl?)Controls.Markdown("QUERY_IN_RENDER_RESOLVED") diff --git a/test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs b/test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs index 08003d60ae..17cb4ac913 100644 --- a/test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs +++ b/test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Fixture; @@ -71,8 +73,11 @@ public async Task FaultingRequest_IsNacked_WhenTheHandlerRacesDisposal() private async Task RunRace(Address victimAddress, bool faulting) { - using var handlerEntered = new ManualResetEventSlim(false); - using var releaseHandler = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: handler → test is an AsyncSubject the producer completes; the + // release travels INTO the deliberately parked action block, so it is a volatile flag + // polled under a bounded SpinUntil and written in the `finally` below. + var handlerEntered = new AsyncSubject(); + var releaseHandler = 0; var host = GetHost(); var victim = host.GetHostedHub(victimAddress, c => c @@ -84,8 +89,9 @@ private async Task RunRace(Address victimAddress, bool faulting) // made deterministic. .WithHandler((_, d) => { - handlerEntered.Set(); - releaseHandler.Wait(TimeSpan.FromSeconds(60)); + handlerEntered.OnNext(Unit.Default); + handlerEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseHandler) == 1, TimeSpan.FromSeconds(60)); return d.Processed(); }) // Both handlers WOULD answer / would be entered, so a pass can only come from the @@ -103,7 +109,7 @@ private async Task RunRace(Address victimAddress, bool faulting) { // 1. Stall the victim's turn loop. host.Post(new Blocker(), o => o.WithTarget(victimAddress)); - handlerEntered.Wait(TimeSpan.FromSeconds(20)).Should().BeTrue( + await handlerEntered.Should().Within(20.Seconds()).Emit( "the blocker handler must be holding the victim's action block"); // 2. Accepted while the hub is healthy, so it lands in the MAIN queue behind the @@ -123,7 +129,7 @@ await WaitUntil(() => victim.RunLevel >= MessageHubRunLevel.ShutDown, Output.WriteLine($"victim RunLevel at release: {victim.RunLevel}"); // 4. Let the turn loop reach our request. - releaseHandler.Set(); + Volatile.Write(ref releaseHandler, 1); // WITHOUT the fix this task never completes and the [Fact] timeout fires with no // explanation — exactly the production symptom (an install that spins for 60 s, a @@ -145,7 +151,7 @@ await WaitUntil(() => victim.RunLevel >= MessageHubRunLevel.ShutDown, } finally { - releaseHandler.Set(); + Volatile.Write(ref releaseHandler, 1); } } diff --git a/test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs b/test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs index f7f9d9280e..257b32fca8 100644 --- a/test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs +++ b/test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Fixture; @@ -51,47 +53,58 @@ public async Task DisposalWaitsForAnInflightConstruction_PastTheOldFiveSecondCap { var client = GetClient(); - using var constructionEntered = new ManualResetEventSlim(false); - using var releaseConstruction = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate: construction → test is an AsyncSubject the producer completes; + // the release travels INTO the thread parked inside Build, so it is a volatile flag polled + // under a bounded SpinUntil and written in the `finally` below. + var constructionEntered = new AsyncSubject(); + var releaseConstruction = 0; - // Same gate/latch shape as InflightHubCreationDrainTest: the WithInitialization hook runs + // Same latch shape as InflightHubCreationDrainTest: the WithInitialization hook runs // synchronously inside Build, so this thread parks INSIDE construction — where the routed - // straggler emissions of the #613 class sit when teardown begins. The gate, not a timer, + // straggler emissions of the #613 class sit when teardown begins. The flag, not a timer, // is what holds the window open, so nothing here samples a race. var creation = Task.Run(() => client.GetHostedHub( new Address("slow-inflight", "1"), c => c.WithInitialization(_ => { - constructionEntered.Set(); - releaseConstruction.Wait(TimeSpan.FromSeconds(30)); + constructionEntered.OnNext(Unit.Default); + constructionEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseConstruction) == 1, TimeSpan.FromSeconds(30)); }), HostedHubCreation.Always)); - constructionEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue( - "the hosted-hub construction must have started before disposal begins"); + try + { + await constructionEntered.Should().Within(10.Seconds()).Emit( + "the hosted-hub construction must have started before disposal begins"); - var disposalCompleted = client.DisposalCompleted.Take(1).Await(); - client.Dispose(); + var disposalCompleted = client.DisposalCompleted.Take(1).Await(); + client.Dispose(); - // Sanctioned negative wait: the finding IS that nothing happened. There is no positive - // signal to filter for — "the owner did not declare itself done" has no emission. - var winner = await Task.WhenAny(disposalCompleted, Task.Delay(PastTheOldCap)); + // Sanctioned negative wait: the finding IS that nothing happened. There is no positive + // signal to filter for — "the owner did not declare itself done" has no emission. + var winner = await Task.WhenAny(disposalCompleted, Task.Delay(PastTheOldCap)); - winner.Should().NotBe(disposalCompleted, - "the owner declared its hosted hubs disposed while a construction it started was " - + "still running — so it went on to tear down the container underneath that Build. " - + "A join must not out-run the answers it is joining: the child's completion is " - + "already guaranteed terminal by MessageHub's own disposal watchdog, and a second, " - + "SHORTER budget layered above it can only ever pre-empt that guarantee"); + winner.Should().NotBe(disposalCompleted, + "the owner declared its hosted hubs disposed while a construction it started was " + + "still running — so it went on to tear down the container underneath that Build. " + + "A join must not out-run the answers it is joining: the child's completion is " + + "already guaranteed terminal by MessageHub's own disposal watchdog, and a second, " + + "SHORTER budget layered above it can only ever pre-empt that guarantee"); - releaseConstruction.Set(); + Volatile.Write(ref releaseConstruction, 1); - await disposalCompleted.WaitAsync(TimeSpan.FromSeconds(15)); + await disposalCompleted.WaitAsync(TimeSpan.FromSeconds(15)); - var lateHub = await creation.WaitAsync(TimeSpan.FromSeconds(10)); - lateHub.Should().NotBeNull( - "the creation was started before disposal, so the contract is to finish it, not refuse it"); - await lateHub!.DisposalCompleted.Take(1).Await().WaitAsync(TimeSpan.FromSeconds(10)); + var lateHub = await creation.WaitAsync(TimeSpan.FromSeconds(10)); + lateHub.Should().NotBeNull( + "the creation was started before disposal, so the contract is to finish it, not refuse it"); + await lateHub!.DisposalCompleted.Take(1).Await().WaitAsync(TimeSpan.FromSeconds(10)); + } + finally + { + Volatile.Write(ref releaseConstruction, 1); + } } /// diff --git a/test/MeshWeaver.Messaging.Hub.Test/InflightHubCreationDrainTest.cs b/test/MeshWeaver.Messaging.Hub.Test/InflightHubCreationDrainTest.cs index 702ba24220..973dabb861 100644 --- a/test/MeshWeaver.Messaging.Hub.Test/InflightHubCreationDrainTest.cs +++ b/test/MeshWeaver.Messaging.Hub.Test/InflightHubCreationDrainTest.cs @@ -1,5 +1,7 @@ using System; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; using System.Threading; using System.Threading.Tasks; using MeshWeaver.Fixture; @@ -29,8 +31,13 @@ public async Task Disposal_WaitsForInflightConstruction_ThenDisposesTheLateHub() { var client = GetClient(); - using var constructionEntered = new ManualResetEventSlim(false); - using var releaseConstruction = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate. `constructionEntered` travels construction → test, so it is an + // AsyncSubject the producer completes and the test awaits through the assertion helpers; + // the release travels INTO the thread parked inside Build, so it is a volatile flag polled + // under a bounded SpinUntil and written in the `finally` below — the shape that stops a + // failing assertion from stranding that thread for the full 30 s. + var constructionEntered = new AsyncSubject(); + var releaseConstruction = 0; // Kick the creation off a background thread: the WithInitialization hook runs // synchronously inside MessageHubConfiguration.Build, so this thread parks inside @@ -40,35 +47,43 @@ public async Task Disposal_WaitsForInflightConstruction_ThenDisposesTheLateHub() new Address("inflight", "1"), c => c.WithInitialization(_ => { - constructionEntered.Set(); - releaseConstruction.Wait(TimeSpan.FromSeconds(30)); + constructionEntered.OnNext(Unit.Default); + constructionEntered.OnCompleted(); + SpinWait.SpinUntil(() => Volatile.Read(ref releaseConstruction) == 1, TimeSpan.FromSeconds(30)); }), HostedHubCreation.Always)); - constructionEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue( - "the hosted-hub construction must have started before disposal begins"); + try + { + await constructionEntered.Should().Within(10.Seconds()).Emit( + "the hosted-hub construction must have started before disposal begins"); - var disposalCompleted = client.DisposalCompleted.Take(1).Await(); - client.Dispose(); + var disposalCompleted = client.DisposalCompleted.Take(1).Await(); + client.Dispose(); - // Negative wait (sanctioned "confirm nothing happened" shape): disposal must NOT - // complete while the construction it started is still in flight — completing here is - // the bug, because the owner would proceed to tear down the container under the - // running Build. - var winner = await Task.WhenAny(disposalCompleted, Task.Delay(TimeSpan.FromSeconds(1))); - winner.Should().NotBe(disposalCompleted, - "disposal must wait for the in-flight hosted-hub construction to finish"); + // Negative wait (sanctioned "confirm nothing happened" shape): disposal must NOT + // complete while the construction it started is still in flight — completing here is + // the bug, because the owner would proceed to tear down the container under the + // running Build. + var winner = await Task.WhenAny(disposalCompleted, Task.Delay(TimeSpan.FromSeconds(1))); + winner.Should().NotBe(disposalCompleted, + "disposal must wait for the in-flight hosted-hub construction to finish"); - releaseConstruction.Set(); + Volatile.Write(ref releaseConstruction, 1); - // Now disposal finishes the started request and completes. - await disposalCompleted.WaitAsync(TimeSpan.FromSeconds(10)); + // Now disposal finishes the started request and completes. + await disposalCompleted.WaitAsync(TimeSpan.FromSeconds(10)); - // And the late-constructed hub was disposed with the collection — not leaked as a - // zombie outside the disposal snapshot. - var lateHub = await creation.WaitAsync(TimeSpan.FromSeconds(10)); - lateHub.Should().NotBeNull("the in-flight creation was started before disposal and must be finished, not refused"); - await lateHub!.DisposalCompleted.Take(1).Await().WaitAsync(TimeSpan.FromSeconds(10)); + // And the late-constructed hub was disposed with the collection — not leaked as a + // zombie outside the disposal snapshot. + var lateHub = await creation.WaitAsync(TimeSpan.FromSeconds(10)); + lateHub.Should().NotBeNull("the in-flight creation was started before disposal and must be finished, not refused"); + await lateHub!.DisposalCompleted.Take(1).Await().WaitAsync(TimeSpan.FromSeconds(10)); + } + finally + { + Volatile.Write(ref releaseConstruction, 1); + } } [Fact] diff --git a/test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs b/test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs index 7f08b05cec..34fc7fc0d6 100644 --- a/test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs +++ b/test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs @@ -3,8 +3,10 @@ using MeshWeaver.Fixture; using Xunit; +using System.Reactive; using System.Reactive.Disposables; using System.Reactive.Linq; +using System.Reactive.Subjects; namespace MeshWeaver.Messaging.Hub.Test; public class MessageHubTest(ITestOutputHelper output) : HubTestBase(output) @@ -327,16 +329,24 @@ public async Task ManyDistinctMissingSubscribes_DoNotWedge() { const int watermark = 25; const int flood = 200; - using var blockHandlerEntered = new ManualResetEventSlim(false); - using var releaseBlock = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate. `blockHandlerEntered` travels handler → test, so it is an + // AsyncSubject the producer completes and the test awaits through the assertion helpers. + // The release travels the other way, INTO a turn thread this test deliberately parks (that + // park IS the subject), so it is a volatile flag the parked turn polls under a bounded + // SpinUntil: no handle to dispose, no `Wait()` bridge, and nothing that can outlive its + // bound if an assertion throws before the release. + var blockHandlerEntered = new AsyncSubject(); + var releaseBlock = 0; var victim = (MessageHub)Mesh.GetHostedHub(new Address("victim", "1"), c => c .WithPostingIdentity(PostingIdentity.System) .WithAggregateWatermark(watermark) .WithHandler((hub, d) => { - blockHandlerEntered.Set(); - releaseBlock.Wait(TimeSpan.FromSeconds(30)); // hold the single turn thread + blockHandlerEntered.OnNext(Unit.Default); + blockHandlerEntered.OnCompleted(); + // hold the single turn thread + SpinWait.SpinUntil(() => Volatile.Read(ref releaseBlock) == 1, TimeSpan.FromSeconds(30)); return d.Processed(); }) .WithHandler((hub, request) => @@ -345,33 +355,42 @@ public async Task ManyDistinctMissingSubscribes_DoNotWedge() return request.Processed(); })); - // Observe the FIRST aggregate shed (fires synchronously on the flooding thread). - var firstShed = victim.StormBreaker!.AggregateSheds.Should().Within(10.Seconds()).Emit(); + try + { + // Observe the FIRST aggregate shed (fires synchronously on the flooding thread). + var firstShed = victim.StormBreaker!.AggregateSheds.Should().Within(10.Seconds()).Emit(); - // 1) Occupy the single action-block thread. - victim.Post(new BlockTurnRequest(), o => o.WithTarget(victim.Address)); - blockHandlerEntered.Wait(TimeSpan.FromSeconds(10)) - .Should().BeTrue("the block handler must occupy the turn thread before we flood"); + // 1) Occupy the single action-block thread. + victim.Post(new BlockTurnRequest(), o => o.WithTarget(victim.Address)); + await blockHandlerEntered.Should().Within(10.Seconds()) + .Emit("the block handler must occupy the turn thread before we flood"); - // 2) Flood sheddable traffic while the block is held → depth crosses the watermark. - for (var i = 0; i < flood; i++) - victim.Post(new MissingSubscribeEvent(i), o => o.WithTarget(victim.Address)); + // 2) Flood sheddable traffic while the block is held → depth crosses the watermark. + for (var i = 0; i < flood; i++) + victim.Post(new MissingSubscribeEvent(i), o => o.WithTarget(victim.Address)); - await firstShed; // RED without the aggregate breaker: this never emits + await firstShed; // RED without the aggregate breaker: this never emits - // 3) A user-facing probe posted DURING the overload must never be shed. - var probe = victim.Observe(new SayHelloRequest(), o => o.WithTarget(victim.Address)) - .Should().Within(10.Seconds()).Emit(); + // 3) A user-facing probe posted DURING the overload must never be shed. + var probe = victim.Observe(new SayHelloRequest(), o => o.WithTarget(victim.Address)) + .Should().Within(10.Seconds()).Emit(); - // 4) Release the block; the bounded queue drains and the probe is answered. - releaseBlock.Set(); - (await probe).Should().BeAssignableTo>( - "the action block stayed drainable — sheddable traffic was shed, user-facing work was not"); + // 4) Release the block; the bounded queue drains and the probe is answered. + Volatile.Write(ref releaseBlock, 1); + (await probe).Should().BeAssignableTo>( + "the action block stayed drainable — sheddable traffic was shed, user-facing work was not"); - victim.StormBreaker.AggregateShedCount.Should().BeGreaterThan(0, - "the aggregate watermark shed the excess sheddable traffic"); - victim.StormBreaker.TripCount.Should().Be(0, - "the per-key breaker must NOT be what saved the hub — this isolates the aggregate path"); + victim.StormBreaker.AggregateShedCount.Should().BeGreaterThan(0, + "the aggregate watermark shed the excess sheddable traffic"); + victim.StormBreaker.TripCount.Should().Be(0, + "the per-key breaker must NOT be what saved the hub — this isolates the aggregate path"); + } + finally + { + // 🚨 In a `finally`: an assertion that throws above must not leave the turn thread + // parked into the next test. + Volatile.Write(ref releaseBlock, 1); + } } /// @@ -391,48 +410,61 @@ public async Task ManyDistinctMissingSubscribes_DoNotWedge() [Fact] public async Task Dispose_WhenHandlerBlocksTheTurn_WatchdogTearsDownChildrenAndDisposables() { - using var blockHandlerEntered = new ManualResetEventSlim(false); - using var releaseBlock = new ManualResetEventSlim(false); - using var ownDisposed = new ManualResetEventSlim(false); - using var childDisposed = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate — see ManyDistinctMissingSubscribes_DoNotWedge above for why the + // two directions get different shapes: producer → test is an AsyncSubject, test → parked + // turn is a volatile flag polled under a bounded SpinUntil. + var blockHandlerEntered = new AsyncSubject(); + var ownDisposed = new AsyncSubject(); + var childDisposed = new AsyncSubject(); + var releaseBlock = 0; var victim = (MessageHub)Mesh.GetHostedHub(new Address("victim", "wedged-dispose"), c => c .WithPostingIdentity(PostingIdentity.System) .WithHandler((_, d) => { - blockHandlerEntered.Set(); - releaseBlock.Wait(TimeSpan.FromSeconds(30)); // hold the single turn thread → starve ShutdownRequest + blockHandlerEntered.OnNext(Unit.Default); + blockHandlerEntered.OnCompleted(); + // hold the single turn thread → starve ShutdownRequest + SpinWait.SpinUntil(() => Volatile.Read(ref releaseBlock) == 1, TimeSpan.FromSeconds(30)); return d.Processed(); })); // Stands in for a sync-stream subscription that DisposeImpl tears down. - victim.RegisterForDisposal(Disposable.Create(ownDisposed.Set)); + victim.RegisterForDisposal(Disposable.Create(() => + { + ownDisposed.OnNext(Unit.Default); + ownDisposed.OnCompleted(); + })); // Stands in for a hosted sync-stream hub whose keep-alive heartbeat is stopped by hostedHubs.Dispose(). var child = victim.GetHostedHub(new Address("child", "1"), cc => cc.WithPostingIdentity(PostingIdentity.System), HostedHubCreation.Always)!; - child.RegisterForDisposal(Disposable.Create(childDisposed.Set)); + child.RegisterForDisposal(Disposable.Create(() => + { + childDisposed.OnNext(Unit.Default); + childDisposed.OnCompleted(); + })); try { // 1) Wedge the victim's action block so its ShutdownRequest can never get a turn. victim.Post(new BlockTurnRequest(), o => o.WithTarget(victim.Address)); - blockHandlerEntered.Wait(TimeSpan.FromSeconds(10)) - .Should().BeTrue("the block handler must occupy the turn thread before we dispose"); + await blockHandlerEntered.Should().Within(10.Seconds()) + .Emit("the block handler must occupy the turn thread before we dispose"); // 2) Dispose — the state machine is starved; only the 8s watchdog can complete it. victim.Dispose(); // 3) The watchdog (8s) must force the real teardown, not just signal completion. - ownDisposed.Wait(TimeSpan.FromSeconds(20)) - .Should().BeTrue("the watchdog must run DisposeImpl so the hub's own subscriptions don't leak when disposal wedges"); - childDisposed.Wait(TimeSpan.FromSeconds(20)) - .Should().BeTrue("the watchdog must dispose hosted hubs so their keep-alive heartbeats don't leak forever"); + await ownDisposed.Should().Within(20.Seconds()) + .Emit("the watchdog must run DisposeImpl so the hub's own subscriptions don't leak when disposal wedges"); + await childDisposed.Should().Within(20.Seconds()) + .Emit("the watchdog must dispose hosted hubs so their keep-alive heartbeats don't leak forever"); } finally { // Always release the held turn — even if an assertion fails — so the blocking handler // can't pin a thread for 30s and bleed into other tests. - releaseBlock.Set(); + Volatile.Write(ref releaseBlock, 1); } await Task.Yield(); } diff --git a/test/MeshWeaver.Messaging.Hub.Test/TeardownHubCreationFreezeTest.cs b/test/MeshWeaver.Messaging.Hub.Test/TeardownHubCreationFreezeTest.cs index 5972dd5cdb..08f1fbcbe7 100644 --- a/test/MeshWeaver.Messaging.Hub.Test/TeardownHubCreationFreezeTest.cs +++ b/test/MeshWeaver.Messaging.Hub.Test/TeardownHubCreationFreezeTest.cs @@ -1,4 +1,7 @@ +using System.Reactive; +using System.Reactive.Subjects; using System.Threading; +using System.Threading.Tasks; using MeshWeaver.Fixture; using Xunit; @@ -32,10 +35,14 @@ public class TeardownHubCreationFreezeTest(ITestOutputHelper output) : HubTestBa private record Blocker; [Fact] - public void DisposingRoot_FreezesHubCreation_AcrossTheWholeSubtree_BeforeAsyncShutdownRuns() + public async Task DisposingRoot_FreezesHubCreation_AcrossTheWholeSubtree_BeforeAsyncShutdownRuns() { - using var handlerEntered = new ManualResetEventSlim(false); - using var releaseHandler = new ManualResetEventSlim(false); + // 🚨 No hand-woven gate. The handler → test signal is an AsyncSubject the producer + // completes and the test awaits through the assertion helpers; the test → handler release + // is a volatile flag the deliberately parked turn polls under a bounded SpinUntil, set in + // the `finally` below so a failing assertion cannot strand the action block. + var handlerEntered = new AsyncSubject(); + var releaseHandler = 0; var client = GetClient(); // Independent root so disposing it cannot interfere with the fixture's own hubs. @@ -46,11 +53,12 @@ public void DisposingRoot_FreezesHubCreation_AcrossTheWholeSubtree_BeforeAsyncSh .WithTypes(typeof(Blocker)) .WithHandler((_, request) => { - handlerEntered.Set(); + handlerEntered.OnNext(Unit.Default); + handlerEntered.OnCompleted(); // Deliberately ignores cancellation: keeps the root's action // block busy so the ShutdownRequest posted by Dispose() cannot - // be processed until the test releases the gate. - releaseHandler.Wait(TimeSpan.FromSeconds(30)); + // be processed until the test releases the flag. + SpinWait.SpinUntil(() => Volatile.Read(ref releaseHandler) == 1, TimeSpan.FromSeconds(30)); return request.Processed(); })); @@ -66,7 +74,7 @@ public void DisposingRoot_FreezesHubCreation_AcrossTheWholeSubtree_BeforeAsyncSh // any freeze observed on the descendants came from the synchronous // cascade inside Dispose(). root.Post(new Blocker(), o => o.WithTarget(root.Address)); - handlerEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue("the blocker handler must be running"); + await handlerEntered.Should().Within(10.Seconds()).Emit("the blocker handler must be running"); root.Dispose(); @@ -85,7 +93,7 @@ public void DisposingRoot_FreezesHubCreation_AcrossTheWholeSubtree_BeforeAsyncSh } finally { - releaseHandler.Set(); + Volatile.Write(ref releaseHandler, 1); } } } diff --git a/test/MeshWeaver.Persistence.Test/PartitionObjectsSubscriberIndependenceTest.cs b/test/MeshWeaver.Persistence.Test/PartitionObjectsSubscriberIndependenceTest.cs index cbc7dc1ef6..1732098e3e 100644 --- a/test/MeshWeaver.Persistence.Test/PartitionObjectsSubscriberIndependenceTest.cs +++ b/test/MeshWeaver.Persistence.Test/PartitionObjectsSubscriberIndependenceTest.cs @@ -65,17 +65,22 @@ public void GetPartitionObjects_CompletesWhileSubscriberContextIsBlocked() SynchronizationContext.SetSynchronizationContext(ctx); try { - using var done = new ManualResetEventSlim(false); + // 🚨 A volatile flag, not a hand-woven gate. Blocking this thread without + // pumping IS the subject, and SpinUntil expresses it with no kernel handle + // to dispose — and, like the event it replaces, it never runs a queued + // continuation, which is the property the assertion below rests on. + var done = 0; using var subscription = adapter .GetPartitionObjects("Test/Node", "Data", options) .Subscribe( _ => Interlocked.Increment(ref received), - ex => { error = ex; done.Set(); }, - () => done.Set()); + ex => { error = ex; Volatile.Write(ref done, 1); }, + () => Volatile.Write(ref done, 1)); // The blocked wait — the pump must complete WITHOUT this // thread's context ever executing a continuation. - pumpCompletedWhileBlocked = done.Wait(TimeSpan.FromSeconds(15)); + pumpCompletedWhileBlocked = SpinWait.SpinUntil( + () => Volatile.Read(ref done) == 1, TimeSpan.FromSeconds(15)); } finally {