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); + } }