diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-28-script-completions-no-longer-stall-portal-shutdown.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-28-script-completions-no-longer-stall-portal-shutdown.md new file mode 100644 index 0000000000..6b4c44e8ad --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-28-script-completions-no-longer-stall-portal-shutdown.md @@ -0,0 +1,20 @@ +--- +Name: Script completions no longer stall portal shutdown +Category: Fix +Description: The first code-completion or script run in a portal's lifetime could hold up a restart for up to 30 seconds; it no longer does. +Icon: Sparkle +Order: -20260828 +--- + +# Script completions no longer stall portal shutdown + +The very first time a portal process ran a script cell or offered code completions in a +script-flavored editor, it had to build a shared list of reference assemblies — a one-time cost +that is normally quick, but occasionally ran long under memory pressure. That build used to run +inline while holding a slot in the pool a shutdown needs to hand back cleanly, so on the rare +occasion it was slow, a restart or redeploy could sit waiting on it for up to 30 seconds instead of +completing promptly. + +The build now runs independently of any one request: a request waits for it but no longer blocks a +shutdown from proceeding, even on that first, slower call. Restarts and redeploys are consistently +quick regardless of when they land relative to the first script or completion request. diff --git a/src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs b/src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs index b9031c2fc8..5bc90bcf84 100644 --- a/src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs +++ b/src/MeshWeaver.Graph/Configuration/MeshNodeLanguageService.cs @@ -55,8 +55,18 @@ internal sealed class MeshNodeLanguageService( // lifetime, so a single lazily-built project serves every request: each suggest forks the // immutable solution with the in-flight text, nothing is applied back, and concurrent // requests never contend. Bounded: one AdhocWorkspace total, not one per cell. - private readonly Lazy _scriptWorkspace = new( - () => BuildScriptWorkspace(hub.ServiceProvider), + // + // 🚨 Task, not a bare value (issues #2480/#2578). Building it touches + // MeshScriptEnvironment.ReferencesAsync, whose FIRST-ever call in the process can be + // genuinely slow (materializing ~350 assemblies' metadata) — it used to run synchronously, + // inline, inside this leaf's Compile-pool gate, with no cancellation participation at all, + // so a silo/mesh teardown racing the first caller could not reclaim that gate permit within + // the drain budget. Every caller now races its OWN `ct` against this shared Task via + // Task.WaitAsync (see GetScriptCompletionsAsync/GetScriptDiagnosticsAsync) — the FIRST + // caller's own token never cancels the build (it is shared by every request against this + // mesh instance), it only governs how long THAT caller waits. + private readonly Lazy> _scriptWorkspace = new( + () => BuildScriptWorkspaceAsync(hub.ServiceProvider), LazyThreadSafetyMode.ExecutionAndPublication); // Path used as the script document's FilePath — never a real MeshNode path. @@ -469,7 +479,7 @@ private async Task> GetScriptCompletionsAsync( { var usage = usageIndex; var memory = CurrentMemory(); - var script = _scriptWorkspace.Value; + var script = await _scriptWorkspace.Value.WaitAsync(ct).ConfigureAwait(false); var document = script.Workspace.CurrentSolution .WithDocumentText(script.DocumentId, SourceText.From(StripKernelDirectives(proposedCode))) .GetDocument(script.DocumentId); @@ -484,7 +494,7 @@ private async Task> GetScriptCompletionsAsync( private async Task> GetScriptDiagnosticsAsync( string proposedCode, CancellationToken ct) { - var script = _scriptWorkspace.Value; + var script = await _scriptWorkspace.Value.WaitAsync(ct).ConfigureAwait(false); var document = script.Workspace.CurrentSolution .WithDocumentText(script.DocumentId, SourceText.From(StripKernelDirectives(proposedCode))) .GetDocument(script.DocumentId); @@ -781,12 +791,20 @@ private static CompletionKind MapTagsToKind(ImmutableArray tags) /// compilation's usings, the process-shared reference set, the shared metadata resolver, /// and the script globals as the submission's host object — so Mesh, Log, /// Ct and Inputs complete as bare identifiers exactly as they run. + /// + /// 🚨 Awaits the shared reference snapshot with — + /// this build is cached (once) for every future request against THIS mesh instance's + /// _scriptWorkspace, so it must never be torn down by whichever caller happens to be + /// first; each caller's own cancellation is applied at the `_scriptWorkspace.Value.WaitAsync` + /// call site instead (see the field's doc comment). /// - private static ScriptWorkspace BuildScriptWorkspace(IServiceProvider serviceProvider) + private static async Task BuildScriptWorkspaceAsync(IServiceProvider serviceProvider) { var workspace = new AdhocWorkspace(); var projectId = ProjectId.CreateNewId(); var docId = DocumentId.CreateNewId(projectId); + var references = await Kernel.Hub.MeshScriptEnvironment + .ReferencesAsync(serviceProvider, CancellationToken.None).ConfigureAwait(false); var projectInfo = ProjectInfo .Create( projectId, @@ -799,7 +817,7 @@ private static ScriptWorkspace BuildScriptWorkspace(IServiceProvider serviceProv metadataReferenceResolver: Kernel.Hub.MeshScriptEnvironment.MetadataResolver) .WithUsings(Kernel.Hub.MeshScriptEnvironment.Imports), parseOptions: new CSharpParseOptions(kind: SourceCodeKind.Script), - metadataReferences: Kernel.Hub.MeshScriptEnvironment.References(serviceProvider), + metadataReferences: references, isSubmission: true, hostObjectType: Kernel.Hub.MeshScriptEnvironment.GlobalsType) .WithDocuments( diff --git a/src/MeshWeaver.Kernel.Hub/KernelExecutor.cs b/src/MeshWeaver.Kernel.Hub/KernelExecutor.cs index 04c3217912..e2e14bf9f3 100644 --- a/src/MeshWeaver.Kernel.Hub/KernelExecutor.cs +++ b/src/MeshWeaver.Kernel.Hub/KernelExecutor.cs @@ -349,17 +349,17 @@ private void ClearCancellationIf(CancellationTokenSource cts) // No lock and no gate here: REPL ordering across submissions is owned by the serial // Concat pump (see `submissions`), which subscribes this observable only after the // previous submission completed. So Execute is just: init → resolve NuGet refs → run - // the Roslyn pass. SubscribeOn moves the WHOLE subscribe — including - // EnsureInitialized's AppDomain scan — onto the ThreadPool, so the executor action - // block is never blocked by a compile. (The Roslyn compile itself runs on the bounded - // Compile IoPool inside RunOnePass; NuGet restore on the Http pool.) + // the Roslyn pass. SubscribeOn moves the WHOLE subscribe onto the ThreadPool, so the + // executor action block is never blocked by a compile. EnsureInitializedAsync's + // AppDomain scan is ADDITIONALLY routed through the bounded Compile IoPool (issues + // #2480/#2578) — it used to run as a bare synchronous call here, off any pool, which + // meant it could neither be tracked nor cancelled by silo/mesh teardown; now it is the + // same tracked, drainable leaf shape as the Roslyn compile itself in RunOnePass. return Observable.Defer(() => - { - EnsureInitialized(); - return EnsureCellSurfaceReferences() + compilePool.Invoke(t => EnsureInitializedAsync(t)) + .SelectMany(_ => EnsureCellSurfaceReferences()) .SelectMany(_ => nugetPool.Invoke(t => ResolveNuGetReferencesAsync(code, t))) - .SelectMany(cleaned => RunOnePass(cleaned, viewId, scriptOutputLogger, inputs, accessContext, ct)); - }) + .SelectMany(cleaned => RunOnePass(cleaned, viewId, scriptOutputLogger, inputs, accessContext, ct))) .Catch(ex => { if (ex is OperationCanceledException) return Observable.Throw(ex); @@ -479,16 +479,24 @@ public void Dispose() /// Build script options once on first submission. Pulls the IMessageHub for /// the globals from the public hub (the parent), so scripts see the SAME hub /// external callers do — they can post messages, subscribe streams, etc. + /// + /// 🚨 Runs INSIDE the Compile IoPool leaf wraps it in — never bare + /// on the ThreadPool as before (issues #2480/#2578). The reference materialization behind + /// can be genuinely slow the first time + /// any mesh in the process touches it, and used to hold no pool gate at all while doing so — + /// invisible to IoPoolRegistry.DrainAll and therefore never cancelled or joined by + /// teardown. Now the wait is a real, trackable, cancellable leaf; + /// governs only how long THIS session waits, never the shared snapshot build itself (see + /// ). /// - private void EnsureInitialized() + private async Task EnsureInitializedAsync(CancellationToken ct) { - if (initialized) return; - initialized = true; + if (initialized) return Unit.Default; var defaultLogger = publicHub.ServiceProvider.GetRequiredService() .CreateLogger("MeshWeaver.Kernel.Script"); - scriptLogger = new ScriptLogger(defaultLogger); - scriptGlobals = new MeshScriptGlobals { Mesh = publicHub, Log = scriptLogger }; + var localScriptLogger = new ScriptLogger(defaultLogger); + var localScriptGlobals = new MeshScriptGlobals { Mesh = publicHub, Log = localScriptLogger }; // Curated anchors + DI-contributed module assemblies + default imports all come // from MeshScriptEnvironment — the ONE description of the script environment, @@ -498,8 +506,20 @@ private void EnsureInitialized() // metadata block per reference PER SESSION (~350 refs ≈ 150-200 MiB of native // memory per kernel session, never reclaimed — the CI memory-pressure leak; see // KernelScriptReferences docs). - scriptOptions = ScriptOptions.Default - .WithReferences(MeshScriptEnvironment.References(publicHub.ServiceProvider)) + // + // 🚨 `initialized` flips to true ONLY after every step below has succeeded (Copilot + // review, PR #2598). It used to be set FIRST — harmless when the reference build was a + // synchronous call that either finished or threw synchronously right there, but this + // method is now `await`-based and genuinely cancellable, so a cancellation or a + // transient fault partway through used to leave `initialized == true` with + // `scriptOptions` still null FOREVER: the guard at the top would then skip every future + // submission's attempt to build it, permanently wedging the session. Local variables + // here (rather than writing straight into the instance fields) keep a failed attempt + // from leaving scriptLogger/scriptGlobals half-set either. + var references = await MeshScriptEnvironment.ReferencesAsync(publicHub.ServiceProvider, ct) + .ConfigureAwait(false); + var options = ScriptOptions.Default + .WithReferences(references) // 🚨 Required for the sharing to be complete: the compilation resolves // the globals type's transitive closure via ResolveMissingAssembly — // with the default resolver that re-materializes every assembly's @@ -507,6 +527,12 @@ private void EnsureInitialized() .WithMetadataResolver(MeshScriptEnvironment.MetadataResolver) .WithImports(MeshScriptEnvironment.Imports) .WithEmitDebugInformation(true); + + scriptLogger = localScriptLogger; + scriptGlobals = localScriptGlobals; + scriptOptions = options; + initialized = true; + return Unit.Default; } /// diff --git a/src/MeshWeaver.Kernel.Hub/KernelScriptAssembly.cs b/src/MeshWeaver.Kernel.Hub/KernelScriptAssembly.cs index 6c84783dc6..c709f768b8 100644 --- a/src/MeshWeaver.Kernel.Hub/KernelScriptAssembly.cs +++ b/src/MeshWeaver.Kernel.Hub/KernelScriptAssembly.cs @@ -8,7 +8,7 @@ namespace MeshWeaver.Kernel.Hub; /// MeshNodes (export, import, NodeType compile, …) registers one /// per assembly that scripts need to reference. /// -/// The kernel's resolves all +/// The kernel's resolves all /// instances via IServiceProvider.GetServices<KernelScriptAssembly> /// and adds each to the Roslyn script options. That way /// the script's using MyNamespace; directive resolves even if Roslyn's diff --git a/src/MeshWeaver.Kernel.Hub/KernelScriptReferences.cs b/src/MeshWeaver.Kernel.Hub/KernelScriptReferences.cs index 764031380b..2391681bf8 100644 --- a/src/MeshWeaver.Kernel.Hub/KernelScriptReferences.cs +++ b/src/MeshWeaver.Kernel.Hub/KernelScriptReferences.cs @@ -31,7 +31,7 @@ namespace MeshWeaver.Kernel.Hub; /// /// The fix: one per assembly /// file, once per process, shared by every session — both for the declared -/// reference set () and for resolver-driven resolution +/// reference set () and for resolver-driven resolution /// ( wraps /// and memoizes per path). Roslyn /// shares the underlying AssemblyMetadata across compilations that use the @@ -54,8 +54,15 @@ internal static class KernelScriptReferences private static readonly ConcurrentDictionary Materialized = new(StringComparer.OrdinalIgnoreCase); - private static readonly Lazy> SharedSnapshot = - new(CreateSharedSnapshot, LazyThreadSafetyMode.ExecutionAndPublication); + // 🚨 Task, not a bare value — see GetReferencesAsync. The materialization below is + // synchronous, uncancellable (no CancellationToken overload exists for + // MetadataReference.CreateFromFile) CPU/disk work over ~350 assemblies, run via Task.Run so + // it executes on its OWN detached ThreadPool work item, decoupled from whichever caller's + // pooled leaf happens to trigger the FIRST-ever build. A caller's own cancellation must never + // abort this — it is shared by every mesh in the process — so GetReferencesAsync races the + // CALLER's token against this shared Task with Task.WaitAsync, never against the work itself. + private static readonly Lazy>> SharedSnapshot = + new(() => Task.Run(CreateSharedSnapshot), LazyThreadSafetyMode.ExecutionAndPublication); private static ImmutableArray CreateSharedSnapshot() => AppDomain.CurrentDomain.GetAssemblies() @@ -165,12 +172,27 @@ private static bool IsTestScaffolding(string assemblyName) /// References for one kernel session: the process-shared snapshot plus shared /// references for any assembly in (curated /// core set + DI-contributed module assemblies) that was loaded after the - /// snapshot was taken. The per-session cost is ~zero — everything resolves to - /// the same process-wide instances. + /// snapshot was taken. The per-session cost is ~zero once the snapshot is warm — + /// everything resolves to the same process-wide instances. + /// + /// 🚨 governs only how long THIS caller waits — it is raced + /// against the shared snapshot Task via Task.WaitAsync, never used to cancel the + /// snapshot build itself. The build is a process-wide, cache-forever memo shared by every + /// mesh in the process (issues #2480 / + /// #2578: it used to run inline inside a per-request pooled leaf with no cancellation + /// participation at all — the FIRST caller in the process paid its full, sometimes + /// multi-second cold cost while holding that leaf's IIoPool gate permit, and a silo/mesh + /// teardown racing that first caller had no way to reclaim the permit within the drain + /// budget). Racing the WAIT (not the work) means: this caller's own pooled leaf settles + /// promptly on cancellation and releases its gate permit, while the shared build keeps + /// running to completion in the background for whichever mesh/request needs it next — safe, + /// because it never touches a collectible NodeType ALC or any one mesh's disposed DI scope + /// ( explicitly excludes collectible-ALC assemblies). /// - public static ImmutableArray GetReferences(IEnumerable sessionAssemblies) + public static async Task> GetReferencesAsync( + IEnumerable sessionAssemblies, CancellationToken ct) { - var snapshot = SharedSnapshot.Value; + var snapshot = await SharedSnapshot.Value.WaitAsync(ct).ConfigureAwait(false); var result = ImmutableArray.CreateBuilder(snapshot.Length + 4); var seen = new HashSet(); foreach (var reference in snapshot) diff --git a/src/MeshWeaver.Kernel.Hub/MeshScriptEnvironment.cs b/src/MeshWeaver.Kernel.Hub/MeshScriptEnvironment.cs index 95f0648edb..0976bc42f6 100644 --- a/src/MeshWeaver.Kernel.Hub/MeshScriptEnvironment.cs +++ b/src/MeshWeaver.Kernel.Hub/MeshScriptEnvironment.cs @@ -68,7 +68,7 @@ public static class MeshScriptEnvironment /// free — only its metadata reference has to be guaranteed here, immune to the process-wide /// snapshot freeze in . De-duplication against the /// snapshot and the anchors happens by file path in - /// . + /// . /// public static IReadOnlyCollection SessionAssemblies(IServiceProvider serviceProvider) { @@ -98,9 +98,15 @@ public static IReadOnlyCollection SessionAssemblies(IServiceProvider s /// The full, process-shared metadata reference set for a script session — the shared /// snapshot of every referenceable loaded assembly plus the session anchors. Safe to /// hand to a Roslyn workspace: every instance is the one shared materialization. + /// + /// 🚨 governs only how long the CALLER waits for the shared, + /// process-wide snapshot — never used to abort the snapshot build itself, which keeps + /// running for the next caller regardless of this one's cancellation. See + /// . /// - public static ImmutableArray References(IServiceProvider serviceProvider) - => KernelScriptReferences.GetReferences(SessionAssemblies(serviceProvider)); + public static Task> ReferencesAsync( + IServiceProvider serviceProvider, CancellationToken ct) + => KernelScriptReferences.GetReferencesAsync(SessionAssemblies(serviceProvider), ct); /// /// Whether an assembly is TEST SCAFFOLDING and must therefore be kept out of the script diff --git a/src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs b/src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs index 888473ab15..967b0e1db1 100644 --- a/src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs +++ b/src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs @@ -1,6 +1,7 @@ using System.Collections.Concurrent; using System.Reactive; using System.Reactive.Linq; +using Microsoft.Extensions.Logging; namespace MeshWeaver.Mesh.Threading; @@ -18,6 +19,7 @@ public sealed class IoPoolRegistry : IDisposable { private readonly ConcurrentDictionary _pools = new(StringComparer.Ordinal); private readonly IoPoolOptions _options; + private readonly ILogger? _logger; private int _disposing; private readonly System.Reactive.Subjects.AsyncSubject _disposed = new(); @@ -37,9 +39,15 @@ public sealed class IoPoolRegistry : IDisposable /// Creates the registry with the given per-resource-class concurrency options. /// /// Concurrency caps per pool name; defaults are used when null. - public IoPoolRegistry(IoPoolOptions? options = null) + /// + /// Optional — resolved from DI when the mesh registers logging. Names the offending pool at + /// time (issue #2480: the teardown report carried no pool name, exception, + /// or stack, so a fingerprint could never be turned into a direct pointer to the leaf). + /// + public IoPoolRegistry(IoPoolOptions? options = null, ILogger? logger = null) { _options = options ?? new IoPoolOptions(); + _logger = logger; } /// @@ -127,8 +135,22 @@ public IObservable WhenDrained(TimeSpan timeout) => public int DrainAll() { var leaked = 0; - foreach (var pool in _pools.Values) - leaked += pool.Drain(); + foreach (var kvp in _pools) + { + var residual = kvp.Value.Drain(); + if (residual != 0) + // 🚨 "IoPoolDrain", not "IoPoolSiloTeardown" (Copilot review, PR #2598): DrainAll() + // is the generic mesh-teardown phase 2 (MeshTeardownExtensions AND + // MonolithMeshTestBase.DisposeAsync both call it), not just the Orleans silo path — + // the earlier prefix mislabeled every non-silo residual as if it came from + // IoPoolSiloTeardown specifically. That class's own Dispose()/Disposed path below + // keeps the "IoPoolSiloTeardown" prefix; it is where that name is accurate. + _logger?.LogWarning( + "IoPoolDrain: pool '{PoolName}' did not finish {Residual} leaf(es) within " + + "the drain budget — a leaf ignored its cancellation token; fix the leaf, do not " + + "widen the budget.", kvp.Key, residual); + leaked += residual; + } return leaked; } @@ -143,7 +165,7 @@ public void Dispose() // Dispose(): `using var pool = …` in an async method runs it on a ThreadPool thread, and // parking one there while the leaves it waits for need pool threads starves into a // deadlock on a small runner. The WAIT lives on Disposed, awaited asynchronously. - var pools = _pools.Values.ToArray(); + var pools = _pools.ToArray(); _pools.Clear(); if (pools.Length == 0) { @@ -152,17 +174,35 @@ public void Dispose() return; } + // Per-pool attribution (issue #2480: the silo-teardown report named neither pool nor leaf). + // Logged the moment EACH pool's own Disposed fires — independent of the Zip below, and + // independent of whether the caller's own bounded wait (IoPoolSiloTeardown's 30 s Timeout + // on the aggregate) has already given up. A pool that unwinds AFTER that timeout still gets + // attributed here, which is strictly more than the aggregate -1 residual ever names. + foreach (var kvp in pools) + { + var name = kvp.Key; + kvp.Value.Disposed.Subscribe(residual => + { + if (residual != 0) + _logger?.LogWarning( + "IoPoolSiloTeardown: pool '{PoolName}' left {Residual} leaf(es) still " + + "running at dispose — a leaf ignored its cancellation token; fix the " + + "leaf, do not widen the budget.", name, residual); + }); + } + // Zip: one emission once EVERY pool has reported, carrying the total residual. A pool whose // leaf never unwinds never reports, so this never fires — and the caller's bounded wait // surfaces that as the timeout it is, rather than a false all-clear. - Observable.Zip(pools.Select(pool => pool.Disposed)) + Observable.Zip(pools.Select(kvp => kvp.Value.Disposed)) .Select(residuals => residuals.Sum()) .Take(1) .Subscribe( total => { _disposed.OnNext(total); _disposed.OnCompleted(); }, _ => { _disposed.OnNext(0); _disposed.OnCompleted(); }); - foreach (var pool in pools) - pool.Dispose(); + foreach (var kvp in pools) + kvp.Value.Dispose(); } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/KernelScriptMemoryLeakTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/KernelScriptMemoryLeakTest.cs index d47b009899..619eb4ef73 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/KernelScriptMemoryLeakTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/KernelScriptMemoryLeakTest.cs @@ -31,7 +31,7 @@ namespace MeshWeaver.Hosting.Monolith.Test; /// with only a small managed delta. That signature = a small managed pin holding /// the script's ScriptState → Compilation → the ~300 /// AssemblyMetadata native metadata blocks built by -/// KernelExecutor.EnsureInitialized (one per loaded assembly). +/// KernelExecutor.EnsureInitializedAsync (one per loaded assembly). /// /// This probe runs one kernel script the same way MonolithKernelTest does, /// disposes the mesh AND its ServiceProvider, forces GC, and then BFS-walks the