Repository navigation
Pooled I/O leaf now observes cancellation: script-completion's first-in-process reference build (#2480, #2578) - #2598
Conversation
…observes cancellation (#2480, #2578) KernelScriptReferences.CreateSharedSnapshot() materialized ~350 assemblies' Roslyn metadata inline inside a per-request pooled leaf (MeshNodeLanguageService's Compile-pool leaf, and KernelExecutor's un-pooled EnsureInitialized), with no CancellationToken participation at all. The FIRST caller in a mesh/process paid that build's full — occasionally multi-second, under memory pressure — cost while holding the leaf's IIoPool gate permit, so a silo/mesh teardown racing that first caller could never reclaim the permit within IoPool's 30s drain budget: exactly the "1 pooled I/O leaf(s) still running" residual both issues report (CI trace from run 33152742916 attempt 1 pins ScriptCompletions_FilterByTypedPrefix_NotJustTheAlphabet as the first script-completion call in its process, dispose consuming the full 30006ms with zero log output in between). The shared snapshot now runs as a detached Task; every caller races its OWN cancellation token against that Task via Task.WaitAsync instead of blocking inline — the per-request leaf settles (and releases its gate permit) promptly on cancellation, while the shared build keeps running to completion for whichever mesh/session needs it next (safe: it never touches a collectible NodeType ALC or a disposed mesh's DI scope). KernelExecutor.EnsureInitializedAsync is additionally routed through the Compile IoPool it previously bypassed entirely, so it is now tracked and drainable like every other pooled leaf. IoPoolRegistry now logs the offending pool by name on a nonzero residual, on both drain paths (DrainAll and Dispose/Disposed) — the teardown report used to carry no pool name, exception, or stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0cbcc83 to
102275d
Compare
There was a problem hiding this comment.
Pull request overview
This PR primarily targets teardown-safety for pooled I/O by making the “first reference-materialization in process” path cancellable-at-the-wait boundary, and by improving IoPool leak attribution in logs. The change set also includes several unrelated additions (notably a new Whisper Helm chart and various repo/test scaffolding updates), which increases review/merge risk relative to the PR description’s stated scope.
Changes:
- Reworks kernel/script reference snapshot initialization and script workspace initialization to race caller cancellation against shared, long-running initialization work.
- Routes kernel initialization work through the bounded Compile
IIoPooland expands IoPool teardown logging to include the pool name. - Introduces/extends additional infrastructure and scaffolding (e.g., embeddings provider API, Helm chart content, and misc. repo/test assets).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/MeshWeaver.Kernel.Hub/KernelScriptReferences.cs | Introduces shared snapshot initialization via Task/Task.Run + WaitAsync(ct); central to the cancellation-at-wait approach. |
| src/MeshWeaver.Kernel.Hub/KernelExecutor.cs | Routes initialization through Compile IoPool and adds async initialization; contains a cancellation-related initialization state bug. |
| src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs | Adds script-workspace caching via Lazy<Task<…>> and awaits it for script completions/diagnostics. |
| src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs | Adds per-pool residual logging; message prefix currently references silo teardown even for non-silo callers. |
| src/MeshWeaver.Hosting.Embeddings/IEmbeddingProvider.cs | Introduces a Task-returning async API for embeddings (new async boundary). |
| src/MeshWeaver.Hosting.Embeddings/OllamaEmbeddingProvider.cs | Implements embeddings via async/await HTTP calls (intended as an I/O leaf). |
| src/MeshWeaver.Hosting.Embeddings/NullEmbeddingProvider.cs | Provides a no-op embeddings provider implementation. |
| src/MeshWeaver.Layout/DataBinding/Binding.cs | Currently a comment-only placeholder; should be deleted or made into a real type. |
| src/MeshWeaver.Utils/README.md | Adds README; contains a spelling typo (“utilities”). |
| deploy/whisper/helm/Chart.yaml | Adds a Whisper Helm chart (scope unrelated to the stated pooled I/O teardown fix). |
Suppressed comments (5)
src/MeshWeaver.Kernel.Hub/KernelExecutor.cs:432
compilePool.Invoke(async t => ...)introduces anasynclambda inside src code. The project convention is to keep non-test code reactive and route genuine async work exclusively through the IoPool boundary (withoutasync/awaitat call sites) to avoid scheduler parking and AccessContext loss.
scope => compilePool.Invoke(async t =>
{
using (accessContext is null
? null
: publicHub.ServiceProvider.GetService<AccessService>()
src/MeshWeaver.Kernel.Hub/KernelExecutor.cs:438
- This
awaitinside the pooled compile leaf introduces an async boundary in hub-adjacent code. Prefer composing the script execution as an observable (or pushing the Task entirely behind the IoPool implementation) so the outer layer remains fully reactive.
return await (session ??= new ScriptSession(
scriptGlobals!,
name => cellSurfaceBindings?.GetValueOrDefault(name)))
.RunAsync(cleaned, scriptOptions, typeof(MeshScriptGlobals), t)
.ConfigureAwait(false);
src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs:488
- This second
awaitcontinues the Task-based flow; consider returning an observable from the public API and composing completions withoutawaitso cancellation and scheduler behavior are controlled consistently via IoPool.
if (document is null) return Array.Empty<CompletionEntry>();
return await CompleteAsync(document, position, maxResults, usage, memory, ct).ConfigureAwait(false);
}
src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs:499
- This async diagnostics path awaits the shared workspace. For consistency with the rest of the mesh, prefer a reactive composition that routes async Roslyn work through
IIoPooland avoidsawaitat this layer.
private async Task<IReadOnlyList<DiagnosticInfo>> GetScriptDiagnosticsAsync(
string proposedCode, CancellationToken ct)
{
var script = await _scriptWorkspace.Value.WaitAsync(ct).ConfigureAwait(false);
var document = script.Workspace.CurrentSolution
.WithDocumentText(script.DocumentId, SourceText.From(StripKernelDirectives(proposedCode)))
src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs:504
- This await on Roslyn compilation continues the Task-based flow in src. Consider pushing this Task behind IoPool (
Invoke/Run) and returning an observable outcome so callers don’t await in hub-adjacent code.
var compilation = document is null
? null
: await document.Project.GetCompilationAsync(ct).ConfigureAwait(false);
if (compilation is null) return Array.Empty<DiagnosticInfo>();
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… prefix (PR #2598) - KernelExecutor.EnsureInitializedAsync: initialized now flips to true only after the reference build succeeds, so a cancellation or transient fault mid-build no longer permanently wedges the session (previously the flag was set first, harmless when this was synchronous code but a real bug once it became genuinely cancellable). - IoPoolRegistry.DrainAll(): residual log line now says IoPoolDrain, not IoPoolSiloTeardown — DrainAll is the generic mesh-teardown phase used by both MeshTeardownExtensions and MonolithMeshTestBase, not only the Orleans silo path. The Dispose()/Disposed path IoPoolSiloTeardown itself uses keeps that prefix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results (shard 3)474 tests 470 ✅ 5m 3s ⏱️ Results for commit eb2c019. |
Test Results (shard 2)946 tests 946 ✅ 5m 18s ⏱️ Results for commit eb2c019. |
Test Results (shard 0)894 tests 894 ✅ 5m 24s ⏱️ Results for commit eb2c019. |
Test Results (shard 4) 10 files 10 suites 5m 53s ⏱️ Results for commit eb2c019. |
Test Results (shard 1)983 tests 983 ✅ 6m 20s ⏱️ Results for commit eb2c019. |
Test Results (shard 5)1 573 tests 1 572 ✅ 6m 54s ⏱️ Results for commit eb2c019. |
Test Results 42 files 42 suites 34m 54s ⏱️ Results for commit eb2c019. |
Summary
Fixes #2480 and #2578 — both describe the same defect class: a pooled I/O leaf that ignores its cancellation token, letting silo/mesh teardown dispose over live work.
Root cause, confirmed against the real CI evidence (attempt 1 of run 33152742916, the actual failing run behind #2578 — not the passing re-run):
KernelScriptReferences.CreateSharedSnapshot()materializes ~350 assemblies' Roslyn metadata (MetadataReference.CreateFromFilehas no cancellable overload) — a genuinely slow, uncancellable, first-ever-per-process cost.CancellationTokenparticipation anywhere in the call chain (KernelScriptReferences.GetReferences→MeshScriptEnvironment.References→MeshNodeLanguageService.BuildScriptWorkspace/KernelExecutor.EnsureInitialized).IIoPoolgate permit. A silo/mesh teardown racing that first caller has no way to reclaim the permit insideIoPool's 30 s drain budget — exactly the"1 pooled I/O leaf(s) still running"/leakedIoLeaves=1residual both issues report._meshweaver-test-trace.log, attempt 1) pinsMeshNodeLanguageServiceTest.ScriptCompletions_FilterByTypedPrefix_NotJustTheAlphabetas the first script-completion call in its process (checked: no earlier test in that shard's process touches the script/kernel reference path — the many earlierCompile*/NodeType*tests use a separate, unrelated Roslyn reference path for NodeType compilation). Its dispose consumed the entire 30006 ms drain budget with zero log output in between — consistent only with a silent, uncancellable, still-running synchronous computation, not a hang that would leave a trace.MeshNodeLanguageService(used by the code editor's LSP-backed completions) andKernelExecutor(real script/kernel sessions) both resolve the same shared, process-wideKernelScriptReferences.SharedSnapshot, off the same per-meshCompileIIoPool— so the same defect is reachable from a live prod portal's real AI/kernel usage, which is the structural link to Pooled I/O leaf ignores its cancellation token, letting silo teardown dispose the mesh over live work #2480 (amemex-cloudpod,IoPoolSiloTeardown, no pool/leaf name in the log to confirm the exact call site — but the same shared, unguarded leaf is the only concrete match found).Fix (no band-aids — the leaf now genuinely observes its token, the drain budget is untouched)
KernelScriptReferences.SharedSnapshotis now a detachedTask(Task.Run(CreateSharedSnapshot)) instead of a bare synchronousLazy<T>value. Every caller races its own cancellation token against that shared Task viaTask.WaitAsync(ct)— never against the underlying work, which must keep running to completion for whichever mesh/session needs it next (it's a process-wide, cache-forever memo). This is the same "settle the bridge without aborting the shared work" shape the docs already prescribe fordelegate_to_agent's prior instance of this exact bug class.MeshNodeLanguageService._scriptWorkspacefollows the same pattern (Lazy<Task<ScriptWorkspace>>);GetScriptCompletionsAsync/GetScriptDiagnosticsAsyncnowawait _scriptWorkspace.Value.WaitAsync(ct).KernelExecutor.EnsureInitializedAsync(renamed fromEnsureInitialized) is additionally routed through thecompilePoolit previously bypassed entirely — it used to run as a bare synchronous call, untracked by anyIIoPool, so it could be neither cancelled nor drained by teardown at all. It is now the same tracked, drainable leaf shape as the Roslyn compile inRunOnePass.IoPoolRegistrynow logs the offending pool by name on a nonzero residual, on both drain paths (DrainAll— the test-base path;Dispose/Disposed— the actualIoPoolSiloTeardownprod path). Issue Pooled I/O leaf ignores its cancellation token, letting silo teardown dispose the mesh over live work #2480 explicitly asked for this: "The teardown report does not name the offending pool or leaf... worth including in the fix."Verification
dotnet build -c Release -warnaserror --no-incrementalclean on every touched project (MeshWeaver.Mesh.Contract,MeshWeaver.Kernel.Hub,MeshWeaver.Graph) and their test-project dependent (MeshWeaver.Hosting.Monolith.Test,Memex.Portal.Shared).IoPoolTest(23/23),IoPoolSiloTeardownTest(3/3),NoStaticCollectionsTest(2/2 — the field-type change toSharedSnapshot/Materializeddoesn't trip the mutable-collection guard), fullMeshNodeLanguageServiceTestclass (24/24, including the originally-failing test),MonolithKernelTest(8/8 — real script execution through the now-pooledEnsureInitializedAsync),WhatsNewEntryIntegrityTest(1/1).DOTNET_PROCESSOR_COUNT=4) never reproduced the race — consistent with both issues being single, CI-runner-specific occurrences. Rather than fabricate a synthetic repro, I pulled the actual failing CI run's attempt (not the passing re-run) as the authoritative red-proof and correlated it line-by-line against the code path above.Not fixed here
If any part of #2480's prod occurrence turns out to be a different leaf (the prod log carries no pool/leaf name), this fix still stands on its own evidence for #2578 and closes a real, provable defect reachable from prod's
MeshNodeLanguageService/KernelExecutorpaths — worth watching for recurrence given the new per-pool logging this PR adds.🤖 Generated with Claude Code