From 5a781c706ca98b77bdf849b84e5773ec2a685627 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 12 Aug 2026 09:51:08 -0400 Subject: [PATCH] fix(scripts): build Roslyn ScriptOptions once per process, not per compile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScriptOptions.WithReferences(Assembly[]) resolves each assembly through MetadataReference.CreateFromFile, which does not cache: every call mints a fresh AssemblyMetadata -> PEReader -> NativeHeapMemoryBlock holding an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile therefore leaked native memory permanently — invisible to the GC, to gcdump and to the managed allocation counters, so the working set grew while the GC heap did not. Diagnosed from a live dump of a wonder-app-vd03 Site node: 2,885 MB working set 78 min after a cold start, only 150 MB live GC heap, ~2,469 MB on the default process heap across ~6,700 undisposed AssemblyMetadata instances against 473 DLLs on disk. Three sites, all hoisted to static readonly: - SiteRuntime ScriptCompilationService (the dumped one) - InboundAPI InboundScriptExecutor — same defect on the central node; method compiles recur on every re-registration and revision change - CentralUI ScriptAnalysisService — CreateFromFile per sandbox run ScriptAnalysis RoslynScriptCompiler also builds options per call but draws from the static ScriptTrustPolicy.DefaultReferences, so it mints no metadata and is left alone. Guarded by reference-equality on the artifact rather than by watching memory: a bytes-watching test would be flaky, and the leak is native so the managed counters cannot see it at all. The test is proven to fail before the fix. This does NOT close the AddTemplateScript OOM — that path was shown twice not to compile scripts. It explains how a long-running node reaches a native memory state where a large allocation fails with gigabytes free, which is a lead worth re-testing, not a closure. --- docs/requirements/Component-SiteRuntime.md | 2 + .../ScriptAnalysis/ScriptAnalysisService.cs | 25 ++++++++-- .../InboundScriptExecutor.cs | 49 +++++++++++++------ .../Scripts/ScriptCompilationService.cs | 27 +++++++++- .../Scripts/ScriptCompilationServiceTests.cs | 48 ++++++++++++++++++ 5 files changed, 130 insertions(+), 21 deletions(-) diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 436d07b7..125a24a9 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -98,6 +98,8 @@ flowchart TD > **Per-instance compilation during staggered startup (P6, follow-up)**: the Roslyn compile of each instance's scripts still runs inside Instance Actor start during staggered startup; moving it off-thread is a deferred optimization (it affects failover time-to-recover only, not correctness). As of the round-2 hardening, a process-wide compile cache dedupes identical script bodies within a node's process lifetime (the deploy gate's compile is reused by Instance Actor start), shrinking the recompile cost; the *first* compile after process start still runs inside Instance Actor start, so the deferral stands. +> **The Roslyn `ScriptOptions` are process-static, and must stay that way**: the reference set backing every site-side compile is built once per process, never per compile. `ScriptOptions.WithReferences(Assembly[])` resolves each assembly through `MetadataReference.CreateFromFile`, which does **not** cache — each call mints a fresh `MetadataReference` owning an `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`, an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile leaks native memory permanently: no GC reclaims it, and it is invisible to gcdump and to the managed allocation counters, so the node's working set grows without the GC heap growing. This was a live defect (a Site node at 2,885 MB working set with 150 MB of live GC heap, ~2,469 MB of it on the default process heap across ~6,700 undisposed `AssemblyMetadata` instances). Note this is orthogonal to the compile cache above — the cache dedupes *identical* script bodies, so it bounds nothing when the bodies differ, and it clears wholesale on overflow, after which every script recompiles. Pinned by a reference-equality regression test rather than by watching memory. + ### Deployment Handling - Receives flattened instance configurations from central via the Communication Layer. - Stores the new configuration in local SQLite. diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs index b5298b3a..dd1d3698 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -57,6 +57,26 @@ public class ScriptAnalysisService "System.Text", "System.Threading.Tasks"); + /// + /// Options for a sandbox run — plus the sandbox host assembly + /// resolved by file path. + /// + /// + /// Built ONCE, deliberately. This was previously rebuilt on every sandbox run, and + /// MetadataReference.CreateFromFile does not cache: each call mints an + /// AssemblyMetadata → PEReader → NativeHeapMemoryBlock holding an + /// unmanaged copy of the assembly metadata that nothing disposes, so every run leaked it + /// for the life of the process. Same defect as the Site-side one confirmed from a live dump + /// on 2026-08-12; smaller blast radius here only because sandbox runs are operator-driven + /// rather than continuous. + /// + /// + private static readonly ScriptOptions SandboxOptions = + DefaultOptions.WithReferences(DefaultOptions.MetadataReferences.Concat(new[] + { + Microsoft.CodeAnalysis.MetadataReference.CreateFromFile(typeof(SandboxScriptHost).Assembly.Location) + })); + private readonly ISharedScriptCatalog _sharedScripts; private readonly IMemoryCache _cache; private readonly IServiceProvider _services; @@ -191,10 +211,7 @@ public class ScriptAnalysisService request.TimeoutSeconds ?? SandboxDefaultTimeoutSeconds, 1, SandboxMaxTimeoutSeconds); - var options = DefaultOptions.WithReferences(DefaultOptions.MetadataReferences.Concat(new[] - { - Microsoft.CodeAnalysis.MetadataReference.CreateFromFile(typeof(SandboxScriptHost).Assembly.Location) - })); + var options = SandboxOptions; var globalsType = request.Kind == ScriptKind.InboundApi ? typeof(SandboxInboundScriptHost) diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs index e3fe8806..49106409 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs @@ -202,6 +202,39 @@ public class InboundScriptExecutor /// null when the script is missing, fails to compile, or violates the /// script trust model. Does not mutate the handler cache. /// + /// + /// Roslyn scripting options for every inbound-API method compile. + /// + /// + /// Built ONCE, deliberately. Do not inline this back into . + /// WithReferences(Assembly[]) resolves each assembly through + /// MetadataReference.CreateFromFile, which does not cache — every call mints a + /// fresh AssemblyMetadata → PEReader → NativeHeapMemoryBlock holding + /// an unmanaged copy of the assembly metadata that nothing disposes. Per-compile options + /// therefore leak native memory for the life of the process, invisibly to the GC and to + /// gcdump. Method compiles are not one-shot: every method re-registration and every + /// revision change recompiles, so the growth is unbounded on a long-lived central node. + /// + /// + /// + /// Same defect, same shape as the Site-side one confirmed from a live dump on 2026-08-12 + /// (SiteRuntime.Scripts.ScriptCompilationService.SharedScriptOptions). + /// + /// + private static readonly ScriptOptions SharedScriptOptions = ScriptOptions.Default + .WithReferences( + typeof(object).Assembly, + typeof(Enumerable).Assembly, + typeof(Dictionary<,>).Assembly, + typeof(RouteHelper).Assembly, + typeof(ScriptParameters).Assembly, + typeof(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo).Assembly) + .WithImports( + "System", + "System.Collections.Generic", + "System.Linq", + "System.Threading.Tasks"); + private (Func>? Handler, IReadOnlyList Errors) Compile(ApiMethod method) { if (string.IsNullOrWhiteSpace(method.Script)) @@ -224,23 +257,9 @@ public class InboundScriptExecutor try { - var scriptOptions = ScriptOptions.Default - .WithReferences( - typeof(object).Assembly, - typeof(Enumerable).Assembly, - typeof(Dictionary<,>).Assembly, - typeof(RouteHelper).Assembly, - typeof(ScriptParameters).Assembly, - typeof(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo).Assembly) - .WithImports( - "System", - "System.Collections.Generic", - "System.Linq", - "System.Threading.Tasks"); - var compiled = CSharpScript.Create( method.Script, - scriptOptions, + SharedScriptOptions, globalsType: typeof(InboundScriptContext)); var diagnostics = compiled.Compile(); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs index 78efb0f3..af159e77 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs @@ -66,8 +66,31 @@ public class ScriptCompilationService /// /// Shared Roslyn scripting options (references + imports) used by both full /// script compilation and trigger-expression compilation. + /// + /// + /// Built ONCE, deliberately. Do not turn this back into a method. + /// WithReferences(Assembly[]) resolves every assembly through + /// MetadataReference.CreateFromFile, which does not cache: each call + /// mints a fresh MetadataReference owning an AssemblyMetadata → + /// PEReader → NativeHeapMemoryBlock, an unmanaged copy of the + /// assembly metadata that nothing disposes. Building the options per compile + /// therefore leaks native memory permanently — no GC reclaims it, and neither + /// gcdump nor GC.GetTotalAllocatedBytes can see it, because it is not + /// on the managed heap. + /// + /// + /// + /// This was a live defect: a Site node reached a 2,885 MB working set with only + /// 150 MB of live GC heap, ~2,469 MB of it on the default process heap across + /// ~6,700 undisposed AssemblyMetadata instances (against 473 DLLs on + /// disk). The method form reads as harmless, which is exactly why it survived — + /// see ScriptCompilationServiceTests.Compile_DistinctScripts_ShareOneMetadataReferenceSet_SoNativeMemoryDoesNotGrow, + /// which pins reference-equality of the reference set across compiles. + /// ScriptAnalysisService.DefaultOptions and + /// ScriptTrustPolicy.DefaultReferences already follow this pattern. + /// /// - private static ScriptOptions BuildScriptOptions() => ScriptOptions.Default + private static readonly ScriptOptions SharedScriptOptions = ScriptOptions.Default .WithReferences(ScriptAssemblies) .WithImports( "System", @@ -141,7 +164,7 @@ public class ScriptCompilationService { var script = CSharpScript.Create( code, - BuildScriptOptions(), + SharedScriptOptions, globalsType: globalsType); var diagnostics = script.Compile(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs index 9d3b476c..782b03d4 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs @@ -203,4 +203,52 @@ public class ScriptCompilationServiceTests "var sw = System.Diagnostics.Stopwatch.StartNew(); return sw.ElapsedMilliseconds;"); Assert.Empty(violations); } + + /// + /// Native-memory leak guard. ScriptOptions.WithReferences(Assembly[]) resolves each + /// assembly through MetadataReference.CreateFromFile, and every such reference owns an + /// AssemblyMetadata → PEReader → NativeHeapMemoryBlock — an unmanaged copy + /// of the assembly metadata that nothing here ever disposes. Building the options per compile + /// therefore grows native memory permanently: no GC reclaims it, and it is invisible to + /// GC.GetTotalAllocatedBytes and to gcdump. + /// + /// + /// Diagnosed from a live dump of the wonder-app-vd03 Site node (2026-08-12): 2,885 MB working + /// set 78 min after a cold start, of which only 150 MB was live GC heap; VMMap attributed + /// 2,469 MB to the default process heap and dumpheap -stat found 6,740 each of + /// AssemblyMetadata / PEReader / MetadataImageReference against just 473 + /// DLLs on disk — i.e. ~1,348 undisposed copies of this service's 5-assembly reference set. + /// + /// + /// + /// Asserted on the artifact rather than on memory: a watch-the-bytes test would be flaky, and + /// the leak is native so the managed allocation counters cannot see it at all. Two DISTINCT + /// bodies are required — identical ones would be served from + /// without a second CompileUncached, and the test + /// would pass without proving anything. + /// + /// + [Fact] + public void Compile_DistinctScripts_ShareOneMetadataReferenceSet_SoNativeMemoryDoesNotGrow() + { + SiteScriptCompileCache.Clear(); + var first = _service.Compile("first", "return 1 + 1;"); + var second = _service.Compile("second", "return 2 + 2;"); + + Assert.True(first.IsSuccess); + Assert.True(second.IsSuccess); + Assert.NotSame(first.CompiledScript, second.CompiledScript); // two real compiles, not a cache hit + + var firstRefs = first.CompiledScript!.Options.MetadataReferences; + var secondRefs = second.CompiledScript!.Options.MetadataReferences; + + Assert.NotEmpty(firstRefs); // else the reference-equality checks below are vacuous + Assert.Equal(firstRefs.Length, secondRefs.Length); + + for (var i = 0; i < firstRefs.Length; i++) + Assert.Same(firstRefs[i], secondRefs[i]); + + // The options object itself is cached, so it must not be rebuilt either. + Assert.Same(first.CompiledScript.Options, second.CompiledScript.Options); + } }