diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IScriptCacheOwner.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IScriptCacheOwner.cs new file mode 100644 index 00000000..7cfeb40e --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IScriptCacheOwner.cs @@ -0,0 +1,20 @@ +namespace ZB.MOM.WW.OtOpcUa.Commons.Engines; + +/// +/// Optional capability an may implement when it caches +/// compiled scripts and needs an apply-boundary drop. The production +/// RoslynVirtualTagEvaluator caches one collectible +/// per compiled expression; without a +/// drop on each config-apply generation those ALCs accrete forever across script edits (every +/// republish roots a new one). VirtualTagHostActor casts its injected evaluator to this +/// interface and calls at each apply so stale ALCs are +/// reclaimed — mirroring how ScriptedAlarmEngine clears its compile cache per generation. +/// Evaluators with no cache (e.g. ) simply don't implement +/// it, and the host actor's null-conditional cast makes the call a no-op. +/// +public interface IScriptCacheOwner +{ + /// Drops every cached compiled script, disposing each so its collectible + /// AssemblyLoadContext unloads. Called at each config-apply generation boundary. + void ClearCompiledScripts(); +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs index d9e70a8c..3aa47d1b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs @@ -1,4 +1,3 @@ -using System.Collections.Concurrent; using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Commons.Engines; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; @@ -20,10 +19,13 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Engines; /// Cycle detection + cascade ordering live in ; this adapter /// stays single-tag scoped to keep 's message loop simple. /// -public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IDisposable +public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCacheOwner, IDisposable { - private readonly ConcurrentDictionary> _cache - = new(StringComparer.Ordinal); + // Purpose-built source-hash-keyed compile cache: Lazy single-compile (no double-compile race) and + // Clear()/Dispose() that dispose each evaluator's collectible AssemblyLoadContext. The apply-boundary + // Clear() (driven by VirtualTagHostActor via IScriptCacheOwner) is what stops ALCs accreting across + // script edits — the raw ConcurrentDictionary this replaced never released them. + private readonly CompiledScriptCache _cache = new(); private readonly ILogger _logger; private readonly SerilogLogger _scriptRoot; private readonly TimeSpan _runTimeout; @@ -64,7 +66,7 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IDisposabl ScriptEvaluator evaluator; try { - evaluator = _cache.GetOrAdd(expression, ScriptEvaluator.Compile); + evaluator = _cache.GetOrCompile(expression); } catch (CompilationErrorException ex) { @@ -98,11 +100,16 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IDisposabl try { - using var cts = new CancellationTokenSource(_runTimeout); - var raw = evaluator.RunAsync(context, cts.Token).GetAwaiter().GetResult(); + // Route through TimedScriptEvaluator (Task.Run + WaitAsync): a raw CancellationToken can't + // interrupt a CPU-bound/infinite-loop script (Roslyn scripts don't poll it), so the previous + // CTS-only path let one runaway script hang this actor forever. WaitAsync returns control when + // the wall-clock budget fires regardless of whether the inner task completes (the orphaned + // thread is the documented, accepted trade-off — TimedScriptEvaluator remarks). + var timed = new TimedScriptEvaluator(evaluator, _runTimeout); + var raw = timed.RunAsync(context).GetAwaiter().GetResult(); return VirtualTagEvalResult.Ok(raw); } - catch (OperationCanceledException) + catch (ScriptTimeoutException) { return VirtualTagEvalResult.Failure($"script timed out after {_runTimeout.TotalSeconds:F1}s"); } @@ -113,6 +120,9 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IDisposabl } } + /// + public void ClearCompiledScripts() => _cache.Clear(); + private static IReadOnlyDictionary BuildReadCache( IReadOnlyDictionary deps) { @@ -133,10 +143,7 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IDisposabl { if (_disposed) return; _disposed = true; - foreach (var ev in _cache.Values) - { - try { ev.Dispose(); } catch { /* best-effort */ } - } - _cache.Clear(); + // CompiledScriptCache.Dispose drains + disposes every materialised evaluator's ALC. + _cache.Dispose(); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs index 10785fd9..002f1870 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs @@ -85,6 +85,12 @@ public sealed class VirtualTagHostActor : ReceiveActor private void OnApply(ApplyVirtualTags msg) { + // Apply-boundary compile-cache drop: each deploy generation may carry edited script sources, and + // the production evaluator roots one collectible AssemblyLoadContext per compiled expression. Drop + // them here (mirroring ScriptedAlarmEngine's per-generation clear) so stale ALCs are reclaimed + // instead of accreting across edits. No-op for evaluators without a cache (NullVirtualTagEvaluator). + (_evaluator as IScriptCacheOwner)?.ClearCompiledScripts(); + var desired = new HashSet(msg.Plans.Select(p => p.VirtualTagId), StringComparer.Ordinal); // Stop + forget children whose vtagId is no longer desired. Stopping the child triggers its diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs index 064c0fa9..03587563 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs @@ -251,7 +251,7 @@ public sealed class RoslynVirtualTagEvaluatorTests } /// Decisive proof the fast-path skips Roslyn: the compiled-script cache (which every - /// Roslyn evaluation populates via GetOrAdd) stays EMPTY after a mirror evaluation, then + /// Roslyn evaluation populates via GetOrCompile) stays EMPTY after a mirror evaluation, then /// grows to one entry once a genuine (non-mirror) expression forces compilation. [Fact] public void Passthrough_does_not_populate_the_compiled_script_cache() @@ -299,13 +299,74 @@ public sealed class RoslynVirtualTagEvaluatorTests entry.VirtualTagId.ShouldBe("vt-log"); } - /// Reads the count of the private compiled-script cache via reflection. + // ── U2 — production timeout must be REAL: a CPU-bound/infinite-loop script must fail within the + // wall-clock budget, never hang the owning actor. The pre-fix CTS-only path could not interrupt + // a synchronous Roslyn script, so this class of script wedged the VirtualTagActor forever. ── + + /// An infinite-loop script returns Failure ("timed out") within the timeout budget instead + /// of hanging. The whole call is bounded by WaitAsync so a regression FAILS the test (with a + /// TimeoutException) rather than wedging the suite. + [Fact] + public async Task Evaluate_infinite_loop_script_fails_within_timeout_and_does_not_hang() + { + using var sut = new RoslynVirtualTagEvaluator( + NullLogger.Instance, NoOpScriptRoot(), TimeSpan.FromMilliseconds(200)); + + var evalTask = Task.Run( + () => sut.Evaluate("vt-loop", "while(true){}return 0;", new Dictionary()), + TestContext.Current.CancellationToken); + + var result = await evalTask.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + result.Success.ShouldBeFalse(); + result.Reason.ShouldNotBeNull(); + result.Reason.ShouldContain("timed out"); + } + + /// Regression: routing through did not + /// break the happy path — a fast compiled script still returns its value well within the budget. + [Fact] + public void Evaluate_fast_script_still_returns_value_after_timeout_wrapping() + { + using var sut = new RoslynVirtualTagEvaluator( + NullLogger.Instance, NoOpScriptRoot(), TimeSpan.FromMilliseconds(200)); + + var result = sut.Evaluate("vt-fast", "return (int)ctx.GetTag(\"a\").Value + 1;", + new Dictionary { ["a"] = 41 }); + + result.Success.ShouldBeTrue(result.Reason); + result.Value.ShouldBe(42); + } + + // ── U3 — the apply-boundary clear (IScriptCacheOwner) drops compiled entries so their collectible + // AssemblyLoadContexts are reclaimed. (The ALC-unload-on-Clear proof lives in CompiledScriptCacheTests; + // here we assert the adapter's ClearCompiledScripts actually empties its cache.) ── + + /// Verifies drops every cached + /// compile — the hook the host actor calls per deploy generation to release stale ALCs. + [Fact] + public void ClearCompiledScripts_empties_the_compile_cache() + { + using var sut = new RoslynVirtualTagEvaluator(NullLogger.Instance, NoOpScriptRoot()); + + sut.Evaluate("vt-a", "return (int)ctx.GetTag(\"a\").Value + 1;", new Dictionary { ["a"] = 1 }); + CompiledCacheCount(sut).ShouldBe(1); + + sut.ClearCompiledScripts(); + + CompiledCacheCount(sut).ShouldBe(0); + } + + /// Reads the count of the private compiled-script cache via reflection. The cache is a + /// CompiledScriptCache<,> which exposes a Count property (not ICollection). private static int CompiledCacheCount(RoslynVirtualTagEvaluator sut) { var field = typeof(RoslynVirtualTagEvaluator) .GetField("_cache", BindingFlags.Instance | BindingFlags.NonPublic); field.ShouldNotBeNull(); - var cache = (ICollection)field.GetValue(sut)!; - return cache.Count; + var cache = field.GetValue(sut)!; + var countProp = cache.GetType().GetProperty("Count"); + countProp.ShouldNotBeNull(); + return (int)countProp.GetValue(cache)!; } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs index 9d6838a2..9b1590e8 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs @@ -345,6 +345,26 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase mux.LastSender.ShouldNotBe(firstChild); } + /// Wiring guard (arch-review 02/U3, theme #1): every ApplyVirtualTags calls + /// on the evaluator at the apply boundary. + /// This is the load-bearing wire — the production evaluator roots one collectible ALC per compiled + /// script, and without this per-generation clear they accrete across deploys. A pure unit test of + /// the evaluator can't catch a missing call here; this asserts the host actor actually makes it. + [Fact] + public void ApplyVirtualTags_clears_the_evaluator_compile_cache_each_generation() + { + var publish = CreateTestProbe(); + var evaluator = new CacheOwningStubEvaluator(); + var host = Sys.ActorOf(VirtualTagHostActor.Props(publish.Ref, mux: null, evaluator)); + + host.Tell(new VirtualTagHostActor.ApplyVirtualTags(new[] { Plan("vt-1", "eq-1", "speed-rpm") })); + AwaitAssert(() => evaluator.ClearCount.ShouldBe(1)); + + // A second apply generation clears again — proves it's per-generation, not one-shot. + host.Tell(new VirtualTagHostActor.ApplyVirtualTags(new[] { Plan("vt-1", "eq-1", "speed-rpm") })); + AwaitAssert(() => evaluator.ClearCount.ShouldBe(2)); + } + /// Capturing : records every Record call so H5c tests can /// assert the host historizes (or does not) and with what path + snapshot. private sealed class CapturingHistoryWriter : IHistoryWriter @@ -371,4 +391,22 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase public VirtualTagEvalResult Evaluate(string id, string expr, IReadOnlyDictionary deps) => VirtualTagEvalResult.NoChange; } + + /// Spy evaluator that also owns a script cache: records how many times the host actor + /// calls so the apply-boundary wiring can be + /// asserted. Counter is interlocked/volatile — written on the actor thread, read on the test thread. + private sealed class CacheOwningStubEvaluator : IVirtualTagEvaluator, IScriptCacheOwner + { + private int _clearCount; + + /// Number of ClearCompiledScripts calls observed. + public int ClearCount => Volatile.Read(ref _clearCount); + + /// + public VirtualTagEvalResult Evaluate(string id, string expr, IReadOnlyDictionary deps) + => VirtualTagEvalResult.NoChange; + + /// + public void ClearCompiledScripts() => Interlocked.Increment(ref _clearCount); + } }