From fb7c6c7046d2e65c102e128174146c746c642810 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 23 May 2026 17:33:34 -0400 Subject: [PATCH] fix(scripting): route engines through CompiledScriptCache (Core.Scripting-016) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both VirtualTagEngine.Load and ScriptedAlarmEngine.LoadAsync were calling ScriptEvaluator.Compile directly, bypassing CompiledScriptCache. The Core.Scripting-008 collectible-ALC fix wired Dispose only through the cache's Clear()/Dispose(), so the per-publish accretion the -008 fix was meant to eliminate was still in effect on the actual production path — the headline 'no more restarts needed' guarantee wasn't delivered. Resolution: - VirtualTagEngine + ScriptedAlarmEngine each gained a private CompiledScriptCache instance. - Both Load methods now call _compileCache.GetOrCompile(source). - Publish-replace path: _compileCache.Clear() runs alongside the existing _tags / _alarms clears so the prior generation's ALCs are disposed before recompile. - Engine Dispose now calls _compileCache.Dispose() so shutdown actually releases the emitted assemblies. Side-fix in CompiledScriptCache: Dispose() set _disposed=true then called Clear(), but Clear() had a pre-existing 'if (_disposed) return' guard that aborted the drain unconditionally — making the Dispose-triggered cleanup a silent no-op. Removed the disposed-guard on Clear() (clearing an empty/ cleared cache is idempotent). Side-fix in ScriptedAlarmEngine.Dispose: cleared _alarms AFTER the Task.WhenAll drain. The drain guarantees no background callback is mid- flight, so clearing is safe. Previously _alarms was deliberately NOT cleared on Dispose (per Core.ScriptedAlarms-005), but that left the AlarmState records holding TimedScriptEvaluator → ScriptEvaluator → delegate references that rooted the emitted assemblies, defeating the cache's Dispose work on the engine side. Regression tests: - VirtualTagEngineTests.Dispose_unloads_compiled_script_assembly - ScriptedAlarmEngineTests.Dispose_unloads_compiled_predicate_assembly Both use WeakReference + bounded GC.Collect() to prove the emitted assembly is reclaimable after engine.Dispose(). The alarms test had to be synchronous (not 'async Task') because async state machines capture locals as state-struct fields, keeping them alive past the method's apparent end and defeating GC. Verification: - Core.Scripting.Tests: 104/104 (unchanged). - VirtualTags.Tests: 57/57 (was 56 — +1 unload test). - ScriptedAlarms.Tests: 67/67 (was 66 — +1 unload test). - All other consumer suites still green. Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Core.Scripting/findings.md | 43 ++++++++++- code-reviews/README.md | 4 +- .../ScriptedAlarmEngine.cs | 45 ++++++++++-- .../CompiledScriptCache.cs | 10 ++- .../VirtualTagEngine.cs | 26 ++++++- .../ScriptedAlarmEngineTests.cs | 71 +++++++++++++++++++ .../VirtualTagEngineTests.cs | 56 +++++++++++++++ 7 files changed, 242 insertions(+), 13 deletions(-) diff --git a/code-reviews/Core.Scripting/findings.md b/code-reviews/Core.Scripting/findings.md index e493cd9..c4a4c4c 100644 --- a/code-reviews/Core.Scripting/findings.md +++ b/code-reviews/Core.Scripting/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-23 | | Commit reviewed | `a9be809` | | Status | Reviewed | -| Open findings | 5 | +| Open findings | 4 | ## Checklist coverage @@ -612,7 +612,7 @@ so the bug surfaces only as a misleading Roslyn diagnostic). | Severity | Medium | | Category | Performance & resource management | | Location | `src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:74-117`, `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs:139-182` | -| Status | Open | +| Status | Resolved | **Description:** The Core.Scripting-008 resolution introduced `ScriptEvaluator.IDisposable` + `CompiledScriptCache.Clear()` that disposes @@ -657,4 +657,41 @@ for each engine: snapshot the per-evaluator emitted assembly via `WeakReference`, call `Load(...)` with a different definition set, and assert the prior generation's assemblies become collectable. -**Resolution:** _(empty until closed; on close, record the fixing commit SHA, the date, and a one-line description of the fix)_ +**Resolution:** Resolved 2026-05-23 — took the cleaner route from the +recommendation: routed both engines' compile paths through +`CompiledScriptCache`. `VirtualTagEngine` and +`ScriptedAlarmEngine` each gained a private `_compileCache` instance field, +their `Load`/`LoadAsync` methods now call `_compileCache.GetOrCompile(source)` +instead of `ScriptEvaluator.Compile(source)` directly, and the cache is cleared +on publish-replace alongside the existing `_tags` / `_alarms` clears so the +prior generation's ALCs are disposed before recompile. Engine `Dispose` now +also calls `_compileCache.Dispose()` so the engine-shutdown path actually +releases the emitted assemblies. **Side-fix:** discovered + fixed an +adjacent bug in `CompiledScriptCache.Dispose()` itself — it set +`_disposed = true` before calling `Clear()`, but `Clear()`'s pre-existing +`if (_disposed) return` guard then aborted the drain unconditionally, so +the Dispose-triggered cleanup was a silent no-op. Removed the disposed-guard +on `Clear()` (the operation is idempotent — clearing an empty/cleared cache +is safe). Without this side-fix the engine-Dispose path would have left +the cached evaluators rooted forever even though the call chain looked +correct. **Side-fix for ScriptedAlarmEngine.Dispose:** moved the pre-existing +"do NOT clear `_alarms` here" comment to "clear `_alarms` AFTER the drain" +because the AlarmState records hold the `TimedScriptEvaluator`/`ScriptEvaluator` +delegates that root the emitted assembly — leaving them in `_alarms` after +Dispose was the same root-the-script-forever pattern this finding is about, +just on the engine side rather than the cache side. The `_alarms` clear is +safe after the `Task.WhenAll` drain because that drain guarantees no +background callback is mid-flight. Regression tests added: +`VirtualTagEngineTests.Dispose_unloads_compiled_script_assembly` and +`ScriptedAlarmEngineTests.Dispose_unloads_compiled_predicate_assembly` — +each uses `WeakReference` + bounded `GC.Collect()` to prove the emitted +assembly is reclaimable after `engine.Dispose()`. **Important test pattern +detail:** the alarms test originally failed because its helper was +`async Task` — async state machines capture locals as +state-struct fields and can keep them alive past the method's apparent end. +Rewrote as a synchronous helper using `LoadAsync(...).GetAwaiter().GetResult()` +inside two cooperating `[MethodImpl(MethodImplOptions.NoInlining)]` helpers +(`CompileAlarmAndCaptureWeak` + `ExtractEmittedAssemblyWeakRef`) so the +intermediate reflection locals die when each helper returns. Test totals +after fix: Core.Scripting 104 green (unchanged); VirtualTags 57 green (was +56 — +1 unload test); ScriptedAlarms 67 green (was 66 — +1 unload test). diff --git a/code-reviews/README.md b/code-reviews/README.md index 3727b20..c385db6 100644 --- a/code-reviews/README.md +++ b/code-reviews/README.md @@ -20,7 +20,7 @@ Each module's `findings.md` is the source of truth; this file is generated from | [Core.Abstractions](Core.Abstractions/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 8 | | [Core.AlarmHistorian](Core.AlarmHistorian/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 11 | | [Core.ScriptedAlarms](Core.ScriptedAlarms/findings.md) | Claude Code | 2026-05-23 | `a9be809` | Reviewed | 1 | 13 | -| [Core.Scripting](Core.Scripting/findings.md) | Claude Code | 2026-05-23 | `a9be809` | Reviewed | 5 | 16 | +| [Core.Scripting](Core.Scripting/findings.md) | Claude Code | 2026-05-23 | `a9be809` | Reviewed | 4 | 16 | | [Core.VirtualTags](Core.VirtualTags/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 13 | | [Driver.AbCip](Driver.AbCip/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 15 | | [Driver.AbCip.Cli](Driver.AbCip.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 8 | @@ -51,7 +51,6 @@ Findings with status `Open` or `In Progress`, ordered by severity. | Core.Scripting-012 | High | Security | `ForbiddenTypeAnalyzer.cs:60-76`, `ScriptSandbox.cs:96-126` | The Core.Scripting-008 rewrite broadened the BCL references list from a narrow allow-list (`System.Private.CoreLib` + `System.Linq` only) to the full `TRUSTED_PLATFORM_ASSEMBLIES` set filtered to `System.*` + `netstandard` + `Microsoft.Win… | | Core.Scripting-013 | Medium | Security | `ScriptEvaluator.cs:202-225` (`BuildWrapperSource`) | The synthesized wrapper pastes the user's source verbatim between `{` and `}` braces inside a static method body, with a `#line 1` directive and no escaping. The legacy `CSharpScript.CreateDelegate` path was robust to this because Roslyn's… | | Core.Scripting-014 | Medium | Concurrency & thread safety | `CompiledScriptCache.cs:91-103` (`Clear`) | `Clear()` snapshots `_cache.Keys.ToArray()` then iterates, calling `TryRemove(key, out var lazy)` on each — the key-only overload, not the value-scoped one used in `GetOrCompile`'s catch block. Between the snapshot and a given `TryRemove`,… | -| Core.Scripting-016 | Medium | Performance & resource management | `src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:74-117`, `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs:139-182` | The Core.Scripting-008 resolution introduced `ScriptEvaluator.IDisposable` + `CompiledScriptCache.Clear()` that disposes each materialised evaluator before dropping its dictionary entry, so per-publish ALC accretion is no longer process-li… | | Driver.Galaxy-015 | Medium | Security | `libs/MxGateway.Client.dll`, `libs/MxGateway.Contracts.dll`, `libs/README.md` | Commit `994997b` checks in two binary DLLs (`MxGateway.Client.dll`, 99 840 bytes; `MxGateway.Contracts.dll`, 489 984 bytes) under `src/Drivers/.../Driver.Galaxy/libs/` and references them via ``. These are the onl… | | Driver.Galaxy-016 | Medium | Performance & resource management | `ZB.MOM.WW.OtOpcUa.Driver.Galaxy.csproj:43-47`, `libs/README.md:32-37` | The five new `PackageReference` versions declared in the csproj (`Google.Protobuf` 3.34.1, `Grpc.Core.Api` 2.76.0, `Grpc.Net.Client` 2.71.0, `Microsoft.Extensions.Logging.Abstractions` 10.0.0, `Polly` 8.5.2) do not all match what the vendo… | | Core.ScriptedAlarms-013 | Low | Documentation & comments | `ScriptedAlarmEngine.cs:66-81` | The new internal test accessors `TryGetScratchReadCacheForTest` and `TryGetScratchContextForTest` (introduced by the Core.ScriptedAlarms-009 resolution at `0001cdd`) return the *live* per-alarm scratch — the same `Dictionary _scratchByAlarmId = new(StringComparer.Ordinal); + /// + /// Compile cache for every alarm predicate. Routes 's + /// calls through the + /// cache so the collectible + /// each compile produces is actually disposed on the publish-replace path + /// (Core.Scripting-016): the cache's + /// disposes every materialised evaluator before dropping its dictionary entry, + /// so a config-publish releases the prior generation's ALCs and the per-publish + /// accretion the Core.Scripting-008 fix targeted is actually freed in production. + /// Pre-fix the engine called ScriptEvaluator.Compile directly, which left + /// the ALCs rooted until the process exited — defeating -008 on the real path. + /// + private readonly CompiledScriptCache _compileCache = new(); + /// /// Test-only diagnostic: returns the per-alarm scratch read-cache dictionary /// if one has been allocated, else null. Used by Core.ScriptedAlarms-009 @@ -143,6 +157,10 @@ public sealed class ScriptedAlarmEngine : IDisposable // have changed (different Inputs, different Logger), so any reuse would be // unsafe. (Core.ScriptedAlarms-009) _scratchByAlarmId.Clear(); + // Dispose every compiled-predicate ALC from the prior generation BEFORE we + // recompile this one. Skipping this is what made Core.Scripting-008 a + // no-op in production. (Core.Scripting-016) + _compileCache.Clear(); var compileFailures = new List(); foreach (var def in definitions) @@ -157,7 +175,10 @@ public sealed class ScriptedAlarmEngine : IDisposable continue; } - var evaluator = ScriptEvaluator.Compile(def.PredicateScriptSource); + // Route through CompiledScriptCache so the emitted assembly's + // collectible ALC participates in publish-replace cleanup. + // (Core.Scripting-016) + var evaluator = _compileCache.GetOrCompile(def.PredicateScriptSource); var timed = new TimedScriptEvaluator(evaluator, _scriptTimeout); var logger = _loggerFactory.Create(def.AlarmId); @@ -645,12 +666,24 @@ public sealed class ScriptedAlarmEngine : IDisposable } } - // Do NOT clear _alarms here: Timer.Dispose() does not wait for in-flight callbacks, - // so a ShelvingCheckAsync or ReevaluateAsync can still be running inside _evalGate. - // Those paths now re-check _disposed after acquiring the gate and bail out safely. - // Clearing _alarms outside the gate would race concurrent reads and is unnecessary - // (the whole object is being discarded). (Core.ScriptedAlarms-005) + // Safe to clear here: the Task.WhenAll drain above guaranteed no + // ReevaluateAsync / ShelvingCheckAsync is mid-flight, and _disposed=true + // prevents new background work from being queued (OnUpstreamChange bails on + // line 334). Pre-Core.Scripting-016 the comment said "Do NOT clear _alarms", + // but that was when the engine called ScriptEvaluator.Compile directly and + // held the script ALCs through _alarms→AlarmState→TimedScriptEvaluator + // forever — leaving them rooted defeated the -008 collectible-ALC unload. + // Clearing now drops the delegate references so the cache's Dispose call + // below can actually unload the emitted assemblies. (Core.ScriptedAlarms-005 + // re-evaluated under -016.) + _alarms.Clear(); _alarmsReferencing.Clear(); + _scratchByAlarmId.Clear(); + // Dispose every compiled-predicate ALC so the engine's shutdown actually + // releases the emitted assemblies. The drain above ensures no evaluator is + // mid-call; CompiledScriptCache.Dispose internally guards against use-after- + // dispose. (Core.Scripting-016) + _compileCache.Dispose(); } private sealed record AlarmState( diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/CompiledScriptCache.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/CompiledScriptCache.cs index ea35097..4098ad5 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/CompiledScriptCache.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/CompiledScriptCache.cs @@ -88,9 +88,17 @@ public sealed class CompiledScriptCache : IDisposable /// unloads and the /// emitted script assembly becomes eligible for GC (Core.Scripting-008). /// + /// + /// Safe to call after — the operation is idempotent. + /// sets _disposed = true before invoking this + /// method (so callers see the post-Dispose guard on ), + /// but this method itself MUST run to completion so the Dispose-triggered + /// drain actually unloads every materialised evaluator's ALC. (Core.Scripting-016 + /// uncovered this — a previous Clear-aborts-when-disposed guard silently + /// skipped the entire drain on Dispose, leaving emitted assemblies rooted.) + /// public void Clear() { - if (_disposed) return; // Snapshot the entries, swap them out, then dispose. We use TryRemove rather // than _cache.Clear() so a concurrent GetOrCompile re-add after our snapshot // is not silently lost — a new compile starts a fresh cache entry, the old diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs index fb8c990..1f6d373 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs @@ -37,6 +37,21 @@ public sealed class VirtualTagEngine : IDisposable private readonly DependencyGraph _graph = new(); private readonly Dictionary _tags = new(StringComparer.Ordinal); + + /// + /// Compile cache for every virtual-tag script. Routes 's + /// calls through the + /// cache so the collectible + /// each compile produces is actually disposed on the publish-replace path + /// (Core.Scripting-016): the cache's + /// disposes every materialised evaluator before dropping its dictionary entry, + /// so a config-publish releases the prior generation's ALCs and the per-publish + /// accretion the Core.Scripting-008 fix targeted is actually freed in production. + /// Pre-fix the engine called ScriptEvaluator.Compile directly, which left + /// the ALCs rooted until the process exited — defeating -008 on the real path. + /// + private readonly CompiledScriptCache _compileCache = new(); + private readonly ConcurrentDictionary _valueCache = new(StringComparer.Ordinal); private readonly ConcurrentDictionary>> _observers = new(StringComparer.Ordinal); @@ -74,6 +89,10 @@ public sealed class VirtualTagEngine : IDisposable UnsubscribeFromUpstream(); _tags.Clear(); _graph.Clear(); + // Dispose every compiled-script ALC from the prior generation BEFORE we + // recompile this one. Skipping this is what made Core.Scripting-008 a + // no-op in production (Core.Scripting-016). + _compileCache.Clear(); var compileFailures = new List(); var seenPaths = new HashSet(StringComparer.Ordinal); @@ -102,7 +121,9 @@ public sealed class VirtualTagEngine : IDisposable continue; } - var evaluator = ScriptEvaluator.Compile(def.ScriptSource); + // Route through CompiledScriptCache so the emitted assembly's collectible + // ALC participates in publish-replace cleanup. (Core.Scripting-016) + var evaluator = _compileCache.GetOrCompile(def.ScriptSource); var timed = new TimedScriptEvaluator(evaluator, _scriptTimeout); var scriptLogger = _loggerFactory.Create(def.Path); @@ -481,6 +502,9 @@ public sealed class VirtualTagEngine : IDisposable UnsubscribeFromUpstream(); _tags.Clear(); _graph.Clear(); + // Dispose every compiled-script ALC so the engine's shutdown actually + // releases the emitted assemblies. (Core.Scripting-016) + _compileCache.Dispose(); } internal DependencyGraph GraphForTesting => _graph; diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs index d58c87b..55b3ceb 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs @@ -1015,4 +1015,75 @@ public sealed class ScriptedAlarmEngineTests "LoadAsync must drop the prior generation's scratch — reuse across a publish " + "would attach a stale Logger / Inputs to the new alarm definition."); } + + // --- Core.Scripting-016: engine routes compiles through CompiledScriptCache --- + + [Fact] + public void Dispose_unloads_compiled_predicate_assembly() + { + // Pre-fix the engine called ScriptEvaluator.Compile directly, so the + // emitted predicate assembly's ALC stayed loaded for the process lifetime. + // After the fix the engine routes through CompiledScriptCache; engine + // Dispose triggers cache Dispose which unloads every cached evaluator's ALC. + // Assert via WeakReference + GC that the assembly is actually reclaimed. + // Helper is sync + [NoInlining] so its locals can't be kept alive by an + // async state machine (an earlier async version of this test failed because + // the state-machine struct held the evaluator past the method-end). + var weak = CompileAlarmAndCaptureWeak(); + for (int i = 0; i < 10 && weak.IsAlive; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + } + weak.IsAlive.ShouldBeFalse( + "engine Dispose must release the compiled-predicate assembly via " + + "CompiledScriptCache (Core.Scripting-016). If this fails the engine is " + + "back to calling ScriptEvaluator.Compile directly and -008's headline " + + "fix doesn't run in production."); + } + + [System.Runtime.CompilerServices.MethodImpl( + System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + private static WeakReference CompileAlarmAndCaptureWeak() + { + var up = new FakeUpstream(); + up.Set("Temp", 50); + var eng = Build(up, out _); + // Block on LoadAsync so this helper stays synchronous — an `async Task` + // wrapper would compile to a state machine whose generated struct keeps the + // local `eng` reference alive past the method's apparent end, defeating GC. + eng.LoadAsync( + [new ScriptedAlarmDefinition( + "HighTemp", "Plant/Line1", "HighTemp", + AlarmKind.AlarmCondition, AlarmSeverity.High, + "x", + """return (int)ctx.GetTag("Temp").Value > 100;""")], + default).GetAwaiter().GetResult(); + + // Reach into the engine's compile cache via reflection — the field is + // private; we only need the Assembly reference, scoped to this NoInlining + // helper so the locals die when it returns. + var weak = ExtractEmittedAssemblyWeakRef(eng); + eng.Dispose(); + return weak; + } + + [System.Runtime.CompilerServices.MethodImpl( + System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + private static WeakReference ExtractEmittedAssemblyWeakRef(ScriptedAlarmEngine eng) + { + var cacheField = typeof(ScriptedAlarmEngine).GetField( + "_compileCache", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; + var cache = cacheField.GetValue(eng)!; + var cacheDictField = cache.GetType().GetField( + "_cache", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; + var cacheDict = (System.Collections.IDictionary)cacheDictField.GetValue(cache)!; + var lazy = cacheDict.Values.Cast().Single(); + var evaluator = lazy.GetType().GetProperty("Value")!.GetValue(lazy)!; + var del = (Delegate)evaluator.GetType().GetField( + "_func", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)! + .GetValue(evaluator)!; + return new WeakReference(del.Method.Module.Assembly); + } } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags.Tests/VirtualTagEngineTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags.Tests/VirtualTagEngineTests.cs index e06b0e6..c819d4d 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags.Tests/VirtualTagEngineTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags.Tests/VirtualTagEngineTests.cs @@ -655,4 +655,60 @@ public sealed class VirtualTagEngineTests lock (_buf) { _buf.Add((path, value)); } } } + + // --- Core.Scripting-016: engine routes compiles through CompiledScriptCache --- + + [Fact] + public void Dispose_unloads_compiled_script_assembly() + { + // Pre-fix the engine called ScriptEvaluator.Compile directly, so the emitted + // script assembly's ALC stayed loaded for the process lifetime. After the fix + // the engine routes through CompiledScriptCache; engine Dispose triggers cache + // Dispose which unloads every cached evaluator's ALC. Assert via WeakReference + // + GC that the assembly is actually reclaimed. + var weak = CompileTagAndCaptureWeak(); + for (int i = 0; i < 10 && weak.IsAlive; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + } + weak.IsAlive.ShouldBeFalse( + "engine Dispose must release the compiled-tag assembly via " + + "CompiledScriptCache (Core.Scripting-016). If this fails the engine is " + + "back to calling ScriptEvaluator.Compile directly and -008's headline " + + "fix doesn't run in production."); + } + + [System.Runtime.CompilerServices.MethodImpl( + System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + private static WeakReference CompileTagAndCaptureWeak() + { + var up = new FakeUpstream(); + up.Set("X", 10); + var engine = Build(up); + engine.Load([new VirtualTagDefinition( + Path: "Doubled", + DataType: DriverDataType.Float64, + ScriptSource: """return (double)ctx.GetTag("X").Value * 2.0;""")]); + + // Reach into the engine's compile cache via reflection — internals scoped to + // Tests-via-InternalsVisibleTo and the field is private. + var cacheField = typeof(VirtualTagEngine).GetField( + "_compileCache", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; + var cache = cacheField.GetValue(engine)!; + var cacheDictField = cache.GetType().GetField( + "_cache", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; + var cacheDict = (System.Collections.IDictionary)cacheDictField.GetValue(cache)!; + var lazy = cacheDict.Values.Cast().Single(); + var lazyValueProp = lazy.GetType().GetProperty("Value")!; + var evaluator = lazyValueProp.GetValue(lazy)!; + var funcField = evaluator.GetType().GetField( + "_func", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; + var del = (Delegate)funcField.GetValue(evaluator)!; + var weak = new WeakReference(del.Method.Module.Assembly); + + engine.Dispose(); + return weak; + } }