From bc679799f2fcf63828a82449828483eadeb5f281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:18:21 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(#4654):=20stop=20exiting=20a=20thread?= =?UTF-8?q?=20per=20burst=20=E2=80=94=20IoPool=20lanes=20keep=20their=20th?= =?UTF-8?q?reads;=20ThreadPool=20workers=20kept=20alive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A thread EXIT triggers a CoreCLR defect: a thread that touched a collectible [ThreadStatic] frees, on exit, a loader handle in an unrelated LIVE collectible context, leaving its GC static dangling (Doc/Architecture/CollectibleThreadStaticHandleReuse). The runtime fix is upstream; this removes the gratuitous exits this process produces. - LimitedConcurrencyLevelTaskScheduler: a dedicated lane thread now parks when its queue is empty and is woken by the next leaf, instead of exiting per drain; IoPool's disposal releases the threads via Complete(). - Chart: DOTNET_ThreadPool_ThreadsToKeepAlive=-1 so pool workers stop retiring after 20 s idle (measured: 18 workers retire within 3 s at a 500 ms timeout without it, all 18 remain with it). - BlockingLaneThreadsAreKeptTest: 20 sequential bursts on one thread (negative control: the exit-per-drain shape uses 20), a warm lane still runs its full cap, disposal releases the kept threads. - Doc: the 2026-10-07 09:53Z memex-cloud SIGSEGV read from createdump output (crashing thread young, no managed frames, addr 0x8; the dump died with its emptyDir on the 13:09Z roll); pearl's restarts are managed OOM. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../templates/memex-portal/deployment.yaml | 10 ++ .../CollectibleThreadStaticHandleReuse.md | 70 ++++++-- .../Data/Architecture/ControlledIoPooling.md | 2 +- .../Threading/IoPool.cs | 20 ++- .../LimitedConcurrencyLevelTaskScheduler.cs | 162 ++++++++++++++---- .../HandWovenGateRatchetGuard.cs | 9 + .../BlockingLaneThreadsAreKeptTest.cs | 152 ++++++++++++++++ 7 files changed, 381 insertions(+), 44 deletions(-) create mode 100644 test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs diff --git a/deploy/helm/templates/memex-portal/deployment.yaml b/deploy/helm/templates/memex-portal/deployment.yaml index 85e2ad53f2..190b7b0dcb 100644 --- a/deploy/helm/templates/memex-portal/deployment.yaml +++ b/deploy/helm/templates/memex-portal/deployment.yaml @@ -301,6 +301,16 @@ spec: value: "/data/dumps/coredump.%p.%t" - name: "DOTNET_CreateDumpDiagnostics" value: "1" + # ๐Ÿงต ThreadPool workers are KEPT instead of retiring after ~20 s idle (#4654). A thread + # EXIT is the trigger of a CoreCLR defect: a thread that touched a collectible + # [ThreadStatic] (ArrayPool.Shared over a NodeType-compiled T) frees, on exit, a loader + # handle in an unrelated LIVE collectible context, and that context's static then dangles + # (Doc/Architecture/CollectibleThreadStaticHandleReuse). The runtime fix is upstream; + # until it ships, this removes the exits of the threads hub turns run on. Cost: the + # pool's peak thread count stays allocated (parked, no CPU). IoPool's blocking lanes keep + # their own threads for the same reason (LimitedConcurrencyLevelTaskScheduler). + - name: "DOTNET_ThreadPool_ThreadsToKeepAlive" + value: "-1" {{- with .Values.selfUpdate }} {{- if .azureClientId }} # Explicit AZURE_CLIENT_ID for the self-updater's ACR credential (the workload-identity diff --git a/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md b/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md index 3db9e2c7e0..2460aaa45e 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md @@ -242,7 +242,7 @@ The trigger is a **thread exit**. This process has two steady sources of them: - **ThreadPool workers retire** after about 20 s idle. Those are the threads hub turns and deserialisation run on, so they touch NodeType-typed `ArrayPool` constantly. -- **Every blocking lane of the pools `IoPoolRegistry` registers starts a fresh thread per burst, and +- **Until #4654, every blocking lane of the pools `IoPoolRegistry` registers started a fresh thread per burst (it now keeps them โ€” see Remedies), and that thread exits when its queue drains.** This covers the bounded production pools: the `mw-cpu-lane` compile lane and the `mw-io-lane` IO lanes. It does not cover `IoPool.Unbounded`, whose `InvokeBlocking` runs on the ThreadPool through `Task.Run` and is covered by the ThreadPool @@ -275,21 +275,65 @@ carry the identity of the allocator that minted it, with `FreeLoaderAllocatorHan skipping on a mismatch. The repro above is the report. Filing it on `dotnet/runtime` is a public act on the maintainer's behalf, and it has not been done. -**In this repository**, the defect fires only on a thread **exit**, so two changes remove it until a -runtime fix ships. Both are **stopgaps against a runtime defect**, and they are named as such: - -1. `System.Threading.ThreadPool.ThreadsToKeepAlive = -1` (runtimeconfig, or - `DOTNET_ThreadPool_ThreadsToKeepAlive=-1`). Pool workers stop retiring. The cost is that the - pool's peak thread count stays allocated. -2. `IIoPool` lanes keep their dedicated threads alive when idle instead of exiting on every drain. - That is a change to the sealed scheduler and needs its own review. - -Neither has been applied. Each changes thread lifetime for the whole process, so each is an owner's -decision. +**In this repository**, the defect fires only on a thread **exit**, so two changes remove the trigger +for the threads this process controls until a runtime fix ships. Both are **stopgaps against a +runtime defect**, named as such, and both are applied (#4654): + +1. **ThreadPool workers no longer retire.** The portal chart sets + `DOTNET_ThreadPool_ThreadsToKeepAlive=-1` (`deploy/helm/templates/memex-portal/deployment.yaml`). + Measured on .NET 10 with `DOTNET_ThreadPool_ThreadTimeoutMs=500`: a 40-item burst leaves 18 + workers, and all of them retire within 3 s; with the keep-alive set, all 18 remain. The cost is that + the pool's peak thread count stays allocated, parked. It takes effect on the next roll of a + deployment that renders this chart. +2. **`IIoPool` blocking lanes keep their threads.** `LimitedConcurrencyLevelTaskScheduler` starts a + lane thread on demand (never more than the cap), PARKS it when the queue is empty, wakes it for the + next leaf, and lets it exit only when the owning `IoPool`'s disposal completes (`Complete()`). + Before, every burst at an idle lane started a thread and every drain ended one. Pinned by + `BlockingLaneThreadsAreKeptTest`: 20 sequential bursts run on ONE thread (the exit-per-drain shape, + restored as a negative control, uses 20), a warm lane still runs its full cap at once, and disposal + releases every kept thread. + +Neither is the fix. Threads still exit at mesh teardown (every test host does that), and anything +outside these two pools that starts and ends threads is untouched. + +## Production sighting: memex-cloud, 2026-10-07 09:53:29Z + +Read from the `createdump` output in Loki through governed `Logs` actions +(`Ops/Actions/logs-memexcloud-20261007-q5kxg-*` on the control instance). The dump itself was not +read. + +- Pod `memex-portal-deployment-748f7f4577-q5kxg`: `Application started` at 08:59:29Z, so **54 minutes + of uptime**, not the ~21 h of the original #4654 crashes. Heap 2.69 GiB, `serverGC=True`, 22 + ThreadPool threads and `poolPending=0` at the last heartbeat (09:52:51Z). Nothing at `info`/`warn` + in the last 90 s beyond heartbeats and two GitHub-webhook warnings. +- `Crashing thread 45de signal 11`; `NT_SIGINFO โ€ฆ signo 11 code 0001 โ€ฆ addr 0x8`: `SEGV_MAPERR` at + offset 8 from a null base. **No `Unwind: exception type` line**, so this is not a managed exception + routed through `createdump`. +- **Thread `45de` had no managed frames.** Its `Unwind: thread 45de` line is followed directly by the + next thread's, where every application thread prints `Unwind: managed frames`. Its id is among the + newest of the 127 threads in the dump (ids run up to `47cb`; the long-lived runtime threads are + `0001`โ€“`0036`), so it was created shortly before the crash. A recently created thread with no + managed frame is a thread starting or ending, which is the window this defect's free runs in. The + logs cannot say which, and pids are shared with child processes (`git`), so the id gap is not a + thread count. +- The `RIP` printed for `45de` (`00007f8e66665913`, beside the other threads' libc wait addresses) is + the signal handler's, not the fault's, and its stack was "found in other mapping" of 3 pages, the + alternate signal stack. The fault registers exist only in the dump. +- ๐Ÿšจ **The dump is gone.** `DOTNET_DbgMiniDumpName` writes to the `memex-dumps` **emptyDir**, which dies + with the pod, and the 13:09Z roll replaced the pod. With rolls this frequent, a production dump + survives for hours at most; reading one needs a copy taken before the next roll. + +**Not established:** that this crash is this mechanism. It is compatible with it (a young thread with +no managed frame, a null-based read) and with others. + +The same day's restarts on the `pearl` instance are a **different** defect: all three of its dumps in +the 48 h to 16:08Z (2026-10-06 01:07Z and 12:42Z, 2026-10-07 12:49Z) are `signo 6` with +`Unwind: exception type System.OutOfMemoryException` โ€” a managed out-of-memory abort, not a native +fault (`Ops/Actions/logs-pearl-20261007-crash-4654`). ## Related - [Debugging Native Crashes](../DebuggingNativeCrashes): sighting #18, and the fact-6 recipe that found the dangling base -- [Controlled IO Pooling](../ControlledIoPooling): the `IIoPool` lanes whose threads exit per drain +- [Controlled IO Pooling](../ControlledIoPooling): the `IIoPool` lanes, whose threads are now kept between bursts - [Node Type Compilation](../NodeTypeCompilation): where collectible contexts are minted and retired diff --git a/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md b/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md index 59871c8873..3bce881bc1 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md @@ -185,7 +185,7 @@ The concurrency cap is enforced two ways, chosen per leaf kind: | Leaf kind | Mechanism | Why | |---|---|---| | **Genuinely-async** (`Invoke`, `InvokeStream`) | `SemaphoreSlim` async gate, then `.SubscribeOn(TaskPoolScheduler.Default)` | The gate caps **in-flight ops**; the ThreadPool thread is *released during the await*, so a cap of 32 network ops uses ~0 threads while waiting. `SubscribeOn` moves the whole subscribe โ€” gate wait and the function's synchronous prologue โ€” onto the ThreadPool, so it never runs on the calling hub scheduler. (`FromAsync`'s own scheduler argument only schedules *notification delivery*, not where the function is invoked โ€” hence `SubscribeOn`, exactly as `MeshQuery` does.) | -| **Sync-blocking / CPU** (`InvokeBlocking`) | Dedicated `LimitedConcurrencyLevelTaskScheduler` | Blocking work holds a real thread for its whole duration. The limited-concurrency scheduler runs at most *cap* leaves at a time, **on threads it starts itself** (`mw-io-lane`; the CPU lane's are `mw-cpu-lane`), never ThreadPool workers. It used to BORROW pool workers, and a cap never protected the pool: with caps up to 256 over a pool whose minimum is the core count, a burst of blocking reads held every worker the grain turns need โ€” see [Blocking Leaves Off the ThreadPool](../BlockingLeavesOffTheThreadPool). | +| **Sync-blocking / CPU** (`InvokeBlocking`) | Dedicated `LimitedConcurrencyLevelTaskScheduler` | Blocking work holds a real thread for its whole duration. The limited-concurrency scheduler runs at most *cap* leaves at a time, **on threads it starts itself** (`mw-io-lane`; the CPU lane's are `mw-cpu-lane`), never ThreadPool workers, and KEEPS them parked between bursts until the pool is disposed โ€” a thread exit per burst was the trigger of a CoreCLR defect, see [Collectible Thread-Static Handle Reuse](../CollectibleThreadStaticHandleReuse). It used to BORROW pool workers, and a cap never protected the pool: with caps up to 256 over a pool whose minimum is the core count, a burst of blocking reads held every worker the grain turns need โ€” see [Blocking Leaves Off the ThreadPool](../BlockingLeavesOffTheThreadPool). | > The two shapes use threads differently, on purpose. **Async leaves reuse the ThreadPool the framework already uses**, with a governor in front of it โ€” they hold a worker only between awaits, which is what the pool is built for. **Blocking leaves do not touch the ThreadPool at all**: they run on short-lived threads the pool's scheduler starts (one per concurrent drain loop, exiting when its queue is empty), because a blocked thread is exactly what Orleans cannot coordinate with when it is one of its own workers. The thread COUNT is the same either way โ€” a burst of N blocking leaves needs N threads โ€” but a borrowed worker is taken from the grain turns and replaced only by the pool's slow injection, while a started thread is taken from no one. diff --git a/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs b/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs index 40345bd452..456434113f 100644 --- a/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs +++ b/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs @@ -17,6 +17,10 @@ public sealed class IoPool : IIoPool, IDisposable { private readonly SemaphoreSlim _gate; private readonly TaskFactory _blockingFactory; + // The scheduler behind _blockingFactory. Its lane threads are KEPT between bursts (#4654 โ€” a + // thread exit is the trigger of a CoreCLR collectible-thread-static defect), so disposal must + // tell it to let them go: TryFinishDisposal calls Complete() once no blocking leaf can still run. + private readonly LimitedConcurrencyLevelTaskScheduler _blockingScheduler; private readonly int _maxConcurrency; // Pool-wide cancellation, linked into every leaf's token. Drain()/Dispose() cancel it so all // in-flight leaves unwind promptly โ€” the join then knows they will release their gate permits. @@ -210,11 +214,12 @@ internal IoPool(int maxConcurrency, TimeSpan drainTimeout, TimeSpan drainGrace, _drainGrace = drainGrace; _maxConcurrency = maxConcurrency; _gate = new SemaphoreSlim(maxConcurrency, maxConcurrency); + _blockingScheduler = new LimitedConcurrencyLevelTaskScheduler(maxConcurrency, blockingThreadName); _blockingFactory = new TaskFactory( CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskContinuationOptions.None, - new LimitedConcurrencyLevelTaskScheduler(maxConcurrency, blockingThreadName)); + _blockingScheduler); // ๐Ÿšจ STARTED HERE, NOT AT TEARDOWN โ€” see StartCanceller. Every field the thread touches is // assigned above; it parks on _cancelRequestedLatch and does nothing until Drain()/Dispose() // raises it. @@ -1608,6 +1613,15 @@ private void RequestCancelOffCallerThread() /// internal bool CancellerIsAlive => _canceller.IsAlive; + /// + /// The blocking lane's live threads, parked ones included (dedicated mode). Kept between bursts + /// and released by disposal (#4654); pinned by BlockingLaneThreadsAreKeptTest. + /// + internal int LiveBlockingThreads => _blockingScheduler.LiveDrainLoops; + + /// The blocking lane's threads currently parked waiting for work. + internal int ParkedBlockingThreads => _blockingScheduler.ParkedLaneThreads; + /// /// Drains in-flight work (see ) then disposes the gate and cancellation /// source. Synchronous by design: when it returns, no pool thread is running, so the caller may @@ -1702,6 +1716,10 @@ private void TryFinishDisposal() // the canceller is never still parked here. _cancelRequestedLatch.Dispose(); _cancelCompleted.Dispose(); + // The lane threads were kept parked between bursts; no blocking leaf can run any more + // (_gateUsers, _inFlight and _blockingInFlight are all zero), so let them exit. Non-blocking: + // it pulses and returns, and is safe even when this runs ON a lane thread. + _blockingScheduler.Complete(); _disposedSubject.OnNext(0); _disposedSubject.OnCompleted(); } diff --git a/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs b/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs index 7d40790bc1..43325f51ba 100644 --- a/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs +++ b/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs @@ -14,18 +14,31 @@ namespace MeshWeaver.Mesh.Threading; /// leaves (e.g. File.ReadAllBytes, Roslyn compile, Process). It caps how many run at /// once, so a burst of blocking work QUEUES behind the cap instead of fanning out. /// -/// ๐Ÿšจ The drain loops run on threads this scheduler starts itself โ€” one per concurrent -/// drain loop, each exiting when the queue is empty, so an idle pool holds no thread. The Docs sample -/// this was adapted from BORROWS ThreadPool workers, and for a blocking leaf that is the defect: the -/// leaf holds its worker for as long as it blocks, the pool's minimum is ProcessorCount (6 on a -/// portal pod), and most caps are far above that (FileSystem 256, Http 16). A burst of -/// network-volume reads, bundle reads or git waits could therefore hold every worker the grain -/// turns and routing legs need, and the silo then waited on the ThreadPool's slow thread injection โ€” -/// Orleans' ".NET Thread Pool execution stalled". The CPU lane went first -/// (IoPoolNames.CompileCpu, Doc/Architecture/CompileOffTheThreadPool); the same argument -/// holds for anything that holds a thread without yielding it -/// (Doc/Architecture/BlockingLeavesOffTheThreadPool). A null thread name keeps the -/// borrowing behaviour; it exists so a test can show the difference. +/// ๐Ÿšจ The drain loops run on threads this scheduler starts itself โ€” at most one per slot, +/// started on demand and then KEPT: an idle lane thread parks on the queue's monitor until the next +/// wakes it, and exits only once has been called (the +/// owning calls it when its disposal completes). The Docs sample this was adapted +/// from BORROWS ThreadPool workers, and for a blocking leaf that is the defect: the leaf holds its +/// worker for as long as it blocks, the pool's minimum is ProcessorCount (6 on a portal pod), +/// and most caps are far above that (FileSystem 256, Http 16). A burst of network-volume +/// reads, bundle reads or subprocess waits could therefore hold every worker the grain turns and +/// routing legs need, and the silo then waited on the ThreadPool's slow thread injection โ€” Orleans' +/// ".NET Thread Pool execution stalled". The CPU lane went first (IoPoolNames.CompileCpu, +/// Doc/Architecture/CompileOffTheThreadPool); the same argument holds for anything that holds a +/// thread without yielding it (Doc/Architecture/BlockingLeavesOffTheThreadPool). A null +/// thread name keeps the borrowing behaviour; it exists so a test can show the difference. +/// +/// ๐Ÿšจ Why a lane thread is KEPT rather than started per burst (#4654). This scheduler used +/// to start a fresh thread whenever a leaf reached an idle lane and let it EXIT the moment the queue +/// drained โ€” one OS thread created and destroyed per burst, thousands an hour on a portal. A thread +/// EXIT is the trigger of a CoreCLR defect: a thread that once touched a collectible +/// [ThreadStatic] (any ArrayPool<T>.Shared over a NodeType-compiled T +/// creates one) frees, on exit, a loader handle in whatever LIVE collectible context has since been +/// handed that thread-static index โ€” releasing an unrelated GC-statics box, whose static then dangles +/// (Doc/Architecture/CollectibleThreadStaticHandleReuse). The runtime fix is upstream; what this +/// repository controls is how often its own threads exit, and a lane thread now exits once per pool +/// lifetime instead of once per burst. The cost is that a lane keeps the threads its widest burst +/// needed (never more than the cap) parked until the pool is disposed. /// /// The _tasks queue is an INSTANCE field guarded by lock(_tasks) /// โ€” not static, so it dies with the owning and never bleeds @@ -41,9 +54,18 @@ internal sealed class LimitedConcurrencyLevelTaskScheduler : TaskScheduler private readonly int _maxDegreeOfParallelism; - // Number of drain loops currently dispatched (running, or started and about to run). + // Number of drain loops currently dispatched (running, or started and about to run). In the + // dedicated-thread mode this is the number of LIVE lane threads, parked ones included. private int _delegatesQueuedOrRunning; + // Dedicated mode only: lane threads parked in the monitor wait and not yet woken. QueueTask wakes + // one per queued task before it considers starting a new thread. Protected by lock(_tasks). + private int _parked; + + // Dedicated mode only: set by Complete(); a lane thread that finds the queue empty then exits + // instead of parking. Protected by lock(_tasks). + private bool _completed; + // Non-null: each drain loop runs on a thread started here, under this name, never a ThreadPool // worker. Null: the drain loops borrow ThreadPool workers (kept for the contrast tests only). private readonly string? _dedicatedThreadName; @@ -69,11 +91,19 @@ public LimitedConcurrencyLevelTaskScheduler( protected override void QueueTask(Task task) { - // Add the task to the queue and, if we're below the cap, dispatch a - // ThreadPool work item to drain it. + // Add the task to the queue; wake a parked lane thread if there is one, otherwise โ€” below the + // cap โ€” dispatch a new drain loop. lock (_tasks) { var node = _tasks.AddLast(task); + if (_parked > 0) + { + // Counted down HERE, by the waker, so two tasks queued back to back wake two parked + // threads instead of pulsing the same one twice and running both serially. + --_parked; + Monitor.Pulse(_tasks); + return; + } if (_delegatesQueuedOrRunning < _maxDegreeOfParallelism) { ++_delegatesQueuedOrRunning; @@ -96,14 +126,55 @@ protected override void QueueTask(Task task) } } + /// + /// Ends the lane threads: each finishes whatever is still queued, then exits instead of parking. + /// Returns at once โ€” it joins nothing, so it is safe on 's non-blocking + /// disposal path, including from a lane thread itself. Idempotent. A task queued AFTER this still + /// runs (a drain loop is started for it, and exits when the queue drains), so nothing queued is + /// ever stranded. + /// + public void Complete() + { + lock (_tasks) + { + _completed = true; + _parked = 0; + Monitor.PulseAll(_tasks); + } + } + + /// + /// Live drain loops: in the dedicated mode the lane threads alive (parked ones included); in the + /// borrowing mode the loops in flight. + /// + internal int LiveDrainLoops + { + get + { + lock (_tasks) + return _delegatesQueuedOrRunning; + } + } + + /// Lane threads currently parked waiting for work (dedicated mode; always 0 otherwise). + internal int ParkedLaneThreads + { + get + { + lock (_tasks) + return _parked; + } + } + private void NotifyThreadPoolOfPendingWork() { if (_dedicatedThreadName is { } name) { - // A drain loop of its own: the same loop, on a thread the pool never lends out. Background - // so an idle-but-draining loop can never hold the process open; UnsafeStart because each - // task carries its own captured ExecutionContext into TryExecuteTask. - new Thread(_ => DrainQueue(), maxStackSize: 0) + // A lane thread of its own, on a thread the pool never lends out. Background so a parked + // lane can never hold the process open; UnsafeStart because each task carries its own + // captured ExecutionContext into TryExecuteTask (and a kept thread must not pin whichever + // caller's context happened to start it). + new Thread(_ => LaneLoop(), maxStackSize: 0) { IsBackground = true, Name = name, @@ -114,27 +185,60 @@ private void NotifyThreadPoolOfPendingWork() ThreadPool.UnsafeQueueUserWorkItem(_ => DrainQueue(), null); } - private void DrainQueue() + /// + /// A dedicated lane thread's whole life: run queued tasks; when the queue is empty, PARK until + /// wakes it; exit only once has been called. The + /// park is a wait for WORK on a thread this scheduler owns โ€” never a hub turn, a grain turn or a + /// ThreadPool worker โ€” so it holds up no scheduler the mesh runs on. + /// + private void LaneLoop() { + while (true) { - // Process tasks until the queue drains, then relinquish this slot. - while (true) + Task item; + lock (_tasks) { - Task item; - lock (_tasks) + while (_tasks.Count == 0) { - if (_tasks.Count == 0) + if (_completed) { --_delegatesQueuedOrRunning; - break; + return; } + ++_parked; + Monitor.Wait(_tasks); + } - item = _tasks.First!.Value; - _tasks.RemoveFirst(); + item = _tasks.First!.Value; + _tasks.RemoveFirst(); + } + + TryExecuteTask(item); + } + } + + /// + /// The borrowing mode's drain loop (a ThreadPool worker must never be parked): run until the + /// queue is empty, then give the worker back. + /// + private void DrainQueue() + { + while (true) + { + Task item; + lock (_tasks) + { + if (_tasks.Count == 0) + { + --_delegatesQueuedOrRunning; + break; } - TryExecuteTask(item); + item = _tasks.First!.Value; + _tasks.RemoveFirst(); } + + TryExecuteTask(item); } } diff --git a/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs b/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs index e983132add..2261c0f416 100644 --- a/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs +++ b/test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs @@ -75,6 +75,15 @@ private static readonly (string File, string Why)[] SanctionedGates = + "AGENTS.md names it as the one sanctioned SemaphoreSlim in the repo. Everything else " + "channels through it precisely so no other file needs one."), + ("src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs", + "The INTERNAL scheduler behind IoPool's blocking lanes โ€” constructed only by IoPool, " + + "so it is the same sealed boundary split into a second file. Its one Monitor.Wait is " + + "where an idle lane thread (a thread this scheduler started itself, never a hub turn, " + + "a grain turn or a ThreadPool worker) waits for its next blocking leaf. The thread is " + + "kept rather than started per burst because a thread EXIT triggers the CoreCLR " + + "collectible thread-static defect (#4654, Doc/Architecture/CollectibleThreadStatic" + + "HandleReuse); IoPool's disposal releases it through Complete()."), + ("tools/MeshWeaver.ThumbnailGenerator/ThumbnailGenerator.cs", "A STANDALONE Playwright CLI (OutputType Exe) whose csproj references Microsoft." + "Playwright, SixLabors.ImageSharp and System.CommandLine and NOT ONE MeshWeaver " diff --git a/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs b/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs new file mode 100644 index 0000000000..15a801ecc0 --- /dev/null +++ b/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs @@ -0,0 +1,152 @@ +using System.Collections.Concurrent; +using System.Reactive.Linq; +using MeshWeaver.Mesh.Threading; +using MeshWeaver.Messaging; +using Xunit; + +namespace MeshWeaver.Hosting.Test; + +/// +/// A blocking lane KEEPS its threads between bursts and lets them go only when its pool is disposed +/// (#4654, Doc/Architecture/CollectibleThreadStaticHandleReuse). +/// +/// The defect this pins: the lane used to start a fresh OS thread whenever a leaf reached an +/// idle lane and let it EXIT the moment its queue drained โ€” one thread created and destroyed per +/// burst. A thread exit is the trigger of a CoreCLR defect that frees an unrelated GC-statics box in +/// a LIVE collectible context, and the createdump of the 2026-10-07 09:53Z memex-cloud SIGSEGV +/// named a crashing thread with no managed frames whose id sat near the newest ~18,000 tids the +/// process had consumed in 54 minutes. Whatever else exits, this pool's lanes should not, once per +/// burst. +/// +/// Deterministic, not a timing: each burst waits for the lane to go IDLE before the next one +/// starts โ€” "idle" is either the lane's thread parked (kept) or no live lane thread at all (the old +/// exit-per-burst shape) โ€” so the old code cannot reuse a thread by catching it mid-drain, and the +/// distinct-thread count is exact in both shapes. Negative control: revert +/// LimitedConcurrencyLevelTaskScheduler.LaneLoop to the exit-on-empty drain and the first fact +/// sees one thread per burst. +/// +public class BlockingLaneThreadsAreKeptTest +{ + private const int Bursts = 20; + private static readonly TimeSpan Bound = TimeSpan.FromSeconds(10); + + private static bool LaneIsIdle(IoPool pool) => + pool.CurrentInFlight == 0 + && (pool.LiveBlockingThreads == 0 || pool.ParkedBlockingThreads == pool.LiveBlockingThreads); + + [Fact(Timeout = 60_000)] + public async Task SequentialBursts_RunOnOneKeptThread_NotOneThreadPerBurst() + { + using var pool = new IoPool(2); + var threads = new ConcurrentDictionary(); + var ct = TestContext.Current.CancellationToken; + + for (var i = 0; i < Bursts; i++) + { + await pool.InvokeBlocking(_ => + { + threads.TryAdd(Environment.CurrentManagedThreadId, Thread.CurrentThread); + return 0; + }).Await(ct); + + SpinWait.SpinUntil(() => LaneIsIdle(pool), Bound) + .Should().BeTrue("the lane must go idle after each single-leaf burst"); + } + + threads.Should().HaveCount(1, + "a burst arriving at an idle lane must wake the lane's kept thread โ€” a thread created and " + + "destroyed per burst is the exit storm that triggers the collectible thread-static defect"); + pool.LiveBlockingThreads.Should().Be(1, "the kept thread is still alive, parked"); + threads.Values.Single().IsAlive.Should().BeTrue(); + } + + [Fact(Timeout = 60_000)] + public async Task AWarmLane_StillRunsUpToItsCapConcurrently() + { + const int cap = 3; + using var pool = new IoPool(cap); + var ct = TestContext.Current.CancellationToken; + + // Warm: park `cap` threads. + await RunConcurrent(pool, cap, ct); + SpinWait.SpinUntil(() => LaneIsIdle(pool) && pool.ParkedBlockingThreads == cap, Bound) + .Should().BeTrue($"after a {cap}-wide burst the lane keeps {cap} threads parked"); + + // A second burst of `cap` parked leaves must wake ALL of them โ€” not pulse one thread `cap` + // times and run the leaves serially. + var maxRunning = await RunConcurrent(pool, cap, ct); + maxRunning.Should().Be(cap); + pool.LiveBlockingThreads.Should().Be(cap, "waking parked threads starts no new ones"); + } + + [Fact(Timeout = 60_000)] + public async Task Disposal_ReleasesTheKeptThreads() + { + var pool = new IoPool(2); + var ct = TestContext.Current.CancellationToken; + var threads = new ConcurrentDictionary(); + var release = new Probe(); + try + { + var both = Enumerable.Range(0, 2).Select(_ => pool.InvokeBlocking(_ => + { + threads.TryAdd(Environment.CurrentManagedThreadId, Thread.CurrentThread); + Interlocked.Increment(ref release.Running); + SpinWait.SpinUntil(() => release.Go != 0, Bound); + return 0; + }).Await(ct)).ToArray(); + SpinWait.SpinUntil(() => Volatile.Read(ref release.Running) == 2, Bound).Should().BeTrue(); + release.Go = 1; + await Task.WhenAll(both); + } + finally + { + release.Go = 1; + } + + SpinWait.SpinUntil(() => LaneIsIdle(pool) && pool.ParkedBlockingThreads == 2, Bound) + .Should().BeTrue("both lane threads are kept, parked, before disposal"); + + pool.Dispose(); + await pool.Disposed.Timeout(Bound).FirstAsync().Await(ct); + + SpinWait.SpinUntil(() => threads.Values.All(t => !t.IsAlive), Bound) + .Should().BeTrue("disposal must let the kept lane threads exit โ€” a pool must not leak threads past its mesh"); + pool.LiveBlockingThreads.Should().Be(0); + } + + private sealed class Probe + { + public volatile int Go; + public int Running; + } + + private static async Task RunConcurrent(IoPool pool, int width, CancellationToken ct) + { + var probe = new Probe(); + var max = 0; + try + { + var all = Enumerable.Range(0, width).Select(_ => pool.InvokeBlocking(_ => + { + var now = Interlocked.Increment(ref probe.Running); + int seen; + while ((seen = Volatile.Read(ref max)) < now + && Interlocked.CompareExchange(ref max, now, seen) != seen) { } + SpinWait.SpinUntil(() => probe.Go != 0, Bound); + Interlocked.Decrement(ref probe.Running); + return 0; + }).Await(ct)).ToArray(); + + SpinWait.SpinUntil(() => Volatile.Read(ref probe.Running) == width, Bound) + .Should().BeTrue($"all {width} leaves must run at once under a cap of {width}"); + probe.Go = 1; + await Task.WhenAll(all); + } + finally + { + probe.Go = 1; + } + return max; + } +} From 23fd3284b7e4047998a1b35c240759bd6ac4da61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:41:18 +0200 Subject: [PATCH 2/2] fix(iopool): publish Disposed from the scheduler's drained callback; review follow-ups on docs and test Co-Authored-By: Claude Sonnet 5.5 --- .../CollectibleThreadStaticHandleReuse.md | 20 +++--- .../Data/Architecture/ControlledIoPooling.md | 2 +- .../Threading/IoPool.cs | 13 ++-- .../LimitedConcurrencyLevelTaskScheduler.cs | 64 ++++++++++++++++--- .../BlockingLaneThreadsAreKeptTest.cs | 2 +- 5 files changed, 76 insertions(+), 25 deletions(-) diff --git a/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md b/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md index 2460aaa45e..355782f496 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/CollectibleThreadStaticHandleReuse.md @@ -242,16 +242,16 @@ The trigger is a **thread exit**. This process has two steady sources of them: - **ThreadPool workers retire** after about 20 s idle. Those are the threads hub turns and deserialisation run on, so they touch NodeType-typed `ArrayPool` constantly. -- **Until #4654, every blocking lane of the pools `IoPoolRegistry` registers started a fresh thread per burst (it now keeps them โ€” see Remedies), and - that thread exits when its queue drains.** This covers the bounded production pools: the - `mw-cpu-lane` compile lane and the `mw-io-lane` IO lanes. It does not cover `IoPool.Unbounded`, - whose `InvokeBlocking` runs on the ThreadPool through `Task.Run` and is covered by the ThreadPool - bullet above. `LimitedConcurrencyLevelTaskScheduler.NotifyThreadPoolOfPendingWork` runs - `new Thread(_ => DrainQueue())`, and `DrainQueue` returns when `_tasks` is empty. The CPU lane has - worked this way since the dedicated-thread compile lane, and every IO lane since #5678 (blocking - leaves off the ThreadPool, merged 2026-09-25 06:43Z; the images running on - the control and public instances on 2026-09-26, core `4c8530d7dd`, contain it). Each - blocking-leaf burst is therefore one thread exit, and one more chance to free a handle belonging to +- **Until #4654, every blocking lane of the pools `IoPoolRegistry` registers started a fresh thread per + burst, and that thread exited when its queue drained** (it now keeps them parked โ€” see Remedies). This + covered the bounded production pools: the `mw-cpu-lane` compile lane and the `mw-io-lane` IO lanes. It + never covered `IoPool.Unbounded`, whose `InvokeBlocking` runs on the ThreadPool through `Task.Run` and + is covered by the ThreadPool bullet above. `LimitedConcurrencyLevelTaskScheduler` then ran + `new Thread(_ => DrainQueue())` from `NotifyThreadPoolOfPendingWork`, and `DrainQueue` returned when + `_tasks` was empty. The CPU lane worked this way since the dedicated-thread compile lane, and every IO + lane since #5678 (blocking leaves off the ThreadPool, merged 2026-09-25 06:43Z; the images running on + the control and public instances on 2026-09-26, core `4c8530d7dd`, contained it). Each + blocking-leaf burst was therefore one thread exit, and one more chance to free a handle belonging to a context unloaded since that thread first ran. Combined with the recompile cadence measured on the control instance (one new compiled assembly every ~8 s for diff --git a/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md b/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md index 3bce881bc1..112721f56e 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md @@ -187,7 +187,7 @@ The concurrency cap is enforced two ways, chosen per leaf kind: | **Genuinely-async** (`Invoke`, `InvokeStream`) | `SemaphoreSlim` async gate, then `.SubscribeOn(TaskPoolScheduler.Default)` | The gate caps **in-flight ops**; the ThreadPool thread is *released during the await*, so a cap of 32 network ops uses ~0 threads while waiting. `SubscribeOn` moves the whole subscribe โ€” gate wait and the function's synchronous prologue โ€” onto the ThreadPool, so it never runs on the calling hub scheduler. (`FromAsync`'s own scheduler argument only schedules *notification delivery*, not where the function is invoked โ€” hence `SubscribeOn`, exactly as `MeshQuery` does.) | | **Sync-blocking / CPU** (`InvokeBlocking`) | Dedicated `LimitedConcurrencyLevelTaskScheduler` | Blocking work holds a real thread for its whole duration. The limited-concurrency scheduler runs at most *cap* leaves at a time, **on threads it starts itself** (`mw-io-lane`; the CPU lane's are `mw-cpu-lane`), never ThreadPool workers, and KEEPS them parked between bursts until the pool is disposed โ€” a thread exit per burst was the trigger of a CoreCLR defect, see [Collectible Thread-Static Handle Reuse](../CollectibleThreadStaticHandleReuse). It used to BORROW pool workers, and a cap never protected the pool: with caps up to 256 over a pool whose minimum is the core count, a burst of blocking reads held every worker the grain turns need โ€” see [Blocking Leaves Off the ThreadPool](../BlockingLeavesOffTheThreadPool). | -> The two shapes use threads differently, on purpose. **Async leaves reuse the ThreadPool the framework already uses**, with a governor in front of it โ€” they hold a worker only between awaits, which is what the pool is built for. **Blocking leaves do not touch the ThreadPool at all**: they run on short-lived threads the pool's scheduler starts (one per concurrent drain loop, exiting when its queue is empty), because a blocked thread is exactly what Orleans cannot coordinate with when it is one of its own workers. The thread COUNT is the same either way โ€” a burst of N blocking leaves needs N threads โ€” but a borrowed worker is taken from the grain turns and replaced only by the pool's slow injection, while a started thread is taken from no one. +> The two shapes use threads differently, on purpose. **Async leaves reuse the ThreadPool the framework already uses**, with a governor in front of it โ€” they hold a worker only between awaits, which is what the pool is built for. **Blocking leaves do not touch the ThreadPool at all**: they run on dedicated threads the pool's scheduler starts (one per concurrent drain loop, kept parked between bursts and exiting only when the pool is disposed), because a blocked thread is exactly what Orleans cannot coordinate with when it is one of its own workers. The thread COUNT is the same either way โ€” a burst of N blocking leaves needs N threads โ€” but a borrowed worker is taken from the grain turns and replaced only by the pool's slow injection, while a started thread is taken from no one. --- diff --git a/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs b/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs index 456434113f..cb0fe2ecfb 100644 --- a/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs +++ b/src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs @@ -1718,10 +1718,15 @@ private void TryFinishDisposal() _cancelCompleted.Dispose(); // The lane threads were kept parked between bursts; no blocking leaf can run any more // (_gateUsers, _inFlight and _blockingInFlight are all zero), so let them exit. Non-blocking: - // it pulses and returns, and is safe even when this runs ON a lane thread. - _blockingScheduler.Complete(); - _disposedSubject.OnNext(0); - _disposedSubject.OnCompleted(); + // it pulses and returns, and is safe even when this runs ON a lane thread. Disposed is + // published from the scheduler's drained callback โ€” after the LAST lane thread has left its + // loop's accounting (or at once when none is alive) โ€” so the signal keeps meaning "no pool + // thread remains" without anything here waiting for it. + _blockingScheduler.Complete(() => + { + _disposedSubject.OnNext(0); + _disposedSubject.OnCompleted(); + }); } /// diff --git a/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs b/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs index 43325f51ba..c11e799b1c 100644 --- a/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs +++ b/src/MeshWeaver.Mesh.Contract/Threading/LimitedConcurrencyLevelTaskScheduler.cs @@ -66,6 +66,11 @@ internal sealed class LimitedConcurrencyLevelTaskScheduler : TaskScheduler // instead of parking. Protected by lock(_tasks). private bool _completed; + // Set by Complete(); invoked exactly once, OUTSIDE the lock, by whichever thread takes the live + // drain-loop count to zero after completion (or by Complete() itself when none is alive). + // Protected by lock(_tasks). + private Action? _onDrained; + // Non-null: each drain loop runs on a thread started here, under this name, never a ThreadPool // worker. Null: the drain loops borrow ThreadPool workers (kept for the contrast tests only). private readonly string? _dedicatedThreadName; @@ -132,15 +137,32 @@ protected override void QueueTask(Task task) /// disposal path, including from a lane thread itself. Idempotent. A task queued AFTER this still /// runs (a drain loop is started for it, and exits when the queue drains), so nothing queued is /// ever stranded. + /// + /// is the non-blocking completion signal: it runs once, on the + /// last lane thread right after it has taken the live count to zero (or inline here when no lane + /// thread is alive), so a caller can publish "no lane thread remains" without joining anything. + /// That thread is then only returning from its loop. /// - public void Complete() + public void Complete(Action? onDrained = null) { + Action? fire; lock (_tasks) { _completed = true; _parked = 0; + _onDrained ??= onDrained; Monitor.PulseAll(_tasks); + fire = TakeDrainedLocked(); } + fire?.Invoke(); + } + + private Action? TakeDrainedLocked() + { + if (!_completed || _delegatesQueuedOrRunning != 0 || _onDrained is not { } action) + return null; + _onDrained = null; + return action; } /// @@ -195,7 +217,9 @@ private void LaneLoop() { while (true) { - Task item; + Task? item = null; + Action? drained = null; + var exit = false; lock (_tasks) { while (_tasks.Count == 0) @@ -203,17 +227,28 @@ private void LaneLoop() if (_completed) { --_delegatesQueuedOrRunning; - return; + drained = TakeDrainedLocked(); + exit = true; + break; } ++_parked; Monitor.Wait(_tasks); } - item = _tasks.First!.Value; - _tasks.RemoveFirst(); + if (!exit) + { + item = _tasks.First!.Value; + _tasks.RemoveFirst(); + } } - TryExecuteTask(item); + if (exit) + { + drained?.Invoke(); + return; + } + + TryExecuteTask(item!); } } @@ -226,16 +261,27 @@ private void DrainQueue() while (true) { Task item; + Action? drained; lock (_tasks) { if (_tasks.Count == 0) { --_delegatesQueuedOrRunning; - break; + drained = TakeDrainedLocked(); + item = null!; + } + else + { + drained = null; + item = _tasks.First!.Value; + _tasks.RemoveFirst(); } + } - item = _tasks.First!.Value; - _tasks.RemoveFirst(); + if (item is null) + { + drained?.Invoke(); + return; } TryExecuteTask(item); diff --git a/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs b/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs index 15a801ecc0..a30fdf66b7 100644 --- a/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs +++ b/test/MeshWeaver.Hosting.Test/BlockingLaneThreadsAreKeptTest.cs @@ -82,7 +82,7 @@ public async Task AWarmLane_StillRunsUpToItsCapConcurrently() [Fact(Timeout = 60_000)] public async Task Disposal_ReleasesTheKeptThreads() { - var pool = new IoPool(2); + using var pool = new IoPool(2); var ct = TestContext.Current.CancellationToken; var threads = new ConcurrentDictionary(); var release = new Probe();