From ef2eebe6b6c46dcdc5bde3afad20cb44beeed08a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 13:41:15 -0400 Subject: [PATCH] fix(scripting): review minors: configured limit in summaries, dispose gate/CTS, validator, docs (fix round 1) - ScriptLogRateLimiter.LimitPerWindow. The ScriptLogTopicSink and VirtualTagActor summaries now print the configured limit, not DefaultLimitPerWindow. Tests assert "limit N/window" for a non-default N. - CalculationDriver.Dispose cancels, then takes _evalGate (bounded 2 s) and disposes the gate and _evalCts. The pump and timer ticks enter the gate through TryEnterEvalGateAsync, which stops cleanly once the driver is disposed. Test: Dispose with a 6 s runaway run in flight returns promptly, disposes both, and a late trigger is harmless. - ScriptedAlarmEngine captures _shutdownToken once and disposes _shutdownCts in Dispose when every drained task has completed. A task stuck past the grace leaves the CTS to the GC. Lifecycle tests assert the disposal. - DraftValidator: CalculationNoEffectiveTrigger is skipped for a change-triggered tag whose scriptId does not resolve. That case is already CalculationScriptMissing or CalculationScriptNotFound, and there are no reads to count. Both existing tests now assert the absence (red before the change). - docs/VirtualTags.md: System.Threading is denied as a whole namespace, with an allow-list; the type list is now accurate (Unsafe, RuntimeHelpers, and the ForbiddenMembers on System.Type); the resource limits section covers the run guard and the allocation budget. docs/ScriptEditor.md: accurate deny-list and a new "Runtime safety" section covering timeout, refusal, quarantine after 3 timeouts, allocation budget, lifting, and rate-limit summaries that appear only on the source's next entry after a burst then quiet. Refs #734 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- docs/ScriptEditor.md | 42 +++++++++++++++- docs/VirtualTags.md | 8 +-- .../Validation/DraftValidator.cs | 4 ++ .../ScriptedAlarmEngine.cs | 28 +++++++++-- .../ScriptLogRateLimiter.cs | 3 ++ .../ScriptLogTopicSink.cs | 2 +- .../CalculationDriver.cs | 49 +++++++++++++++++-- .../VirtualTags/VirtualTagActor.cs | 2 +- .../DraftValidatorCalculationTests.cs | 10 +++- .../ScriptedAlarmEngineLifecycleTests.cs | 3 ++ .../ScriptLogRateLimiterTests.cs | 4 ++ .../CalculationDriverRemediationTests.cs | 23 +++++++++ ...VirtualTagActorFailureLogRateLimitTests.cs | 1 + 13 files changed, 161 insertions(+), 18 deletions(-) diff --git a/docs/ScriptEditor.md b/docs/ScriptEditor.md index 5321aca8..41421d34 100644 --- a/docs/ScriptEditor.md +++ b/docs/ScriptEditor.md @@ -162,11 +162,49 @@ user's fault. `ForbiddenTypeAnalyzer.Analyze(compilation)` walks the wrapped syntax tree and resolves every referenced symbol against the sandbox deny-list (namespace prefixes `System.IO`, `System.Net`, `System.Diagnostics`, `System.Reflection`, -`System.Threading.Tasks`, etc., plus type-granular denials like -`System.Environment` and `System.Threading.Thread`). Each violation is mapped +`System.Threading` — the whole namespace, with only `CancellationToken`, +`Interlocked` and `Volatile` allowed back in — `System.Threading.Tasks`, +`System.Runtime.InteropServices`, `System.Runtime.Loader`, `Microsoft.Win32`; +type-granular denials `System.Environment`, `System.AppDomain`, `System.GC`, +`System.Activator`, `System.Runtime.CompilerServices.Unsafe` / +`RuntimeHelpers`; and member-granular denials on `System.Type`'s by-name +reflection entry points — `GetType(string)`, `GetMethod(s)`, `InvokeMember`, +and the `GetProperty`/`GetField`/`GetMember`/`GetConstructor`/`GetEvent` +families). The runtime publish gate uses the same analyzer, so a deny-list +change needs a `DiagnoseTests` case to keep editor ⇔ publish parity. Each violation is mapped via `tree.GetMappedLineSpan(span)` to user-source coordinates and emitted as an error marker with code `OTSCRIPT_FORBIDDEN`. +### Runtime safety (what happens after publish) + +The editor cannot catch a script that compiles cleanly but never returns or +allocates without bound, so every runtime seam (virtual tags, Calculation tags, +scripted-alarm predicates) runs scripts through the shared `ScriptRunGuard`: + +- **Timeout + one orphan.** A run that exceeds its budget is abandoned (it cannot + be pre-empted in-process) and the tag/alarm reads Bad. While that abandoned run + is still executing, every further trigger is **refused** — Bad + `"previous evaluation still running"`, counted on `otopcua.script.run_refused` — + so a `while(true)` costs one pool thread, not one per trigger. +- **Quarantine after 3 consecutive timeouts** (`Scripting:QuarantineAfterConsecutiveTimeouts`). + Only a timeout is a strike: a refusal neither adds to nor resets the streak, and + a run that returns (value or throw) resets it. A quarantined script is never + dispatched again; it reads Bad `"script quarantined: …"`, logs one Error and + increments `otopcua_script_quarantined`. An infinite loop therefore usually shows + as *refused* (one timeout, then refusals for as long as the orphan spins) rather + than *quarantined*. +- **Allocation budget.** A single run allocating more than + `Scripting:MaxAllocBytesPerRun` (64 MB) is quarantined. This is sampled after the + run returns, so a run that OOMs the process first is not stopped — that needs an + out-of-process runner. +- **Lifting.** A quarantine lifts when the script source changes or on redeploy, + never by itself. +- **Script-log rate limit.** The cluster-wide script-log fan-out admits 20 entries + per source per minute; the rest are counted and replaced by one "N suppressed" + summary. The summary is emitted on that source's **next** entry after the window + rolls, so a source that bursts and then goes quiet shows its summary only when + it logs again (the full stream is always in `scripts-*.log`). + ### 3. DependencyExtractor `DependencyExtractor.Extract(code).Rejections` enumerates every `ctx.GetTag` / diff --git a/docs/VirtualTags.md b/docs/VirtualTags.md index d58013ea..50ef578f 100644 --- a/docs/VirtualTags.md +++ b/docs/VirtualTags.md @@ -28,15 +28,15 @@ User scripts are compiled via `Microsoft.CodeAnalysis.CSharp` (regular compiler, `ScriptSandbox.Build` constructs the compile reference set in two parts. First, four pinned OtOpcUa assemblies are always included: `Core.Abstractions` (for `DataValueSnapshot` / `DriverDataType`), `Core.Scripting` (for `ScriptContext` + `Deadband`), `Serilog` (for `ILogger`), and the concrete context type's assembly. Second, the BCL subset is enumerated from the runtime's `TRUSTED_PLATFORM_ASSEMBLIES` list, restricted to filenames starting with `System.*` plus `netstandard.dll`, `mscorlib.dll`, and `Microsoft.Win32.Registry.dll` (the last needed so `ForbiddenTypeAnalyzer` can resolve and reject registry types). Pre-imported namespaces: `System`, `System.Linq`, `ZB.MOM.WW.OtOpcUa.Core.Abstractions`, `ZB.MOM.WW.OtOpcUa.Core.Scripting`. -`ForbiddenTypeAnalyzer.ForbiddenNamespacePrefixes` denies `System.IO`, `System.Net`, `System.Diagnostics`, `System.Reflection`, `System.Threading.Tasks`, `System.Runtime.InteropServices`, `System.Runtime.Loader`, and `Microsoft.Win32`. Matching is by prefix against the resolved symbol's containing namespace, so `System.Net` catches `System.Net.Http.HttpClient` and every subnamespace. `System.Threading.Tasks` is denied because scripts are synchronous predicates with no legitimate need to start background tasks — a `Task.Run` fan-out would outlive the per-evaluation timeout entirely (Core.Scripting-003). `System.Runtime.Loader` is denied to block `AssemblyLoadContext` / `AssemblyDependencyResolver` — arbitrary DLL loads into the host process (Core.Scripting-012). +`ForbiddenTypeAnalyzer.ForbiddenNamespacePrefixes` denies `System.IO`, `System.Net`, `System.Diagnostics`, `System.Reflection`, `System.Threading` (the whole namespace — `ForbiddenTypeAnalyzer.AllowedTypesInForbiddenNamespaces` lets only `CancellationToken`, `Interlocked` and `Volatile` back in), `System.Threading.Tasks`, `System.Runtime.InteropServices`, `System.Runtime.Loader`, and `Microsoft.Win32`. Matching is by prefix against the resolved symbol's containing namespace, so `System.Net` catches `System.Net.Http.HttpClient` and every subnamespace. `System.Threading.Tasks` is denied because scripts are synchronous predicates with no legitimate need to start background tasks — a `Task.Run` fan-out would outlive the per-evaluation timeout entirely (Core.Scripting-003). `System.Runtime.Loader` is denied to block `AssemblyLoadContext` / `AssemblyDependencyResolver` — arbitrary DLL loads into the host process (Core.Scripting-012). -`ForbiddenTypeAnalyzer.ForbiddenFullTypeNames` denies type-granularly: `System.Environment`, `System.AppDomain`, `System.GC`, `System.Activator`, `System.Threading.Thread`, `System.Threading.ThreadPool`, and `System.Threading.Timer`. These types require granular denial rather than namespace-prefix denial for different reasons: `Environment` / `AppDomain` / `GC` / `Activator` live directly in the `System` namespace (which is otherwise allowed for primitives), so a namespace-prefix rule cannot reach them without blocking `int` / `string` / `Math`; `Thread` / `ThreadPool` / `Timer` live in `System.Threading` (shared with allowed types like `CancellationToken` and `SemaphoreSlim`), so a prefix on `System.Threading` would block those too. `Environment.Exit` / `FailFast` terminate the host process outright (Core.Scripting-001); `Thread` and `ThreadPool` reintroduce background-fanout vectors that `System.Threading.Tasks` denial closed (Core.Scripting-010 / -012). +`ForbiddenTypeAnalyzer.ForbiddenFullTypeNames` denies type-granularly: `System.Environment`, `System.AppDomain`, `System.GC`, `System.Activator`, `System.Runtime.CompilerServices.Unsafe`, and `System.Runtime.CompilerServices.RuntimeHelpers`. These live in namespaces that are otherwise allowed — `System` (primitives, `Math`) and `System.Runtime.CompilerServices` (compile-time attributes such as `CallerMemberNameAttribute`) — so a namespace-prefix rule cannot reach them. `Environment.Exit` / `FailFast` terminate the host process outright (Core.Scripting-001); `Unsafe.As` bypasses the type system; `RuntimeHelpers.GetUninitializedObject` builds an instance of any type without a constructor. `Thread` / `ThreadPool` / `Timer` / `SemaphoreSlim` and the rest of `System.Threading` are now caught by the namespace prefix above instead of being listed one by one — a script is a synchronous expression with no use for background work or blocking primitives (Core.Scripting-010 / -012, review R05:10). `ForbiddenTypeAnalyzer.ForbiddenMembers` additionally denies `System.Type`'s by-name reflection entry points (`GetType(string)`, `GetMethod(s)`, `InvokeMember`, and the `GetProperty`/`GetField`/`GetMember`/`GetConstructor`/`GetEvent` families), which would otherwise reach a forbidden type without naming it syntactically. #### Known resource limits (accepted trade-offs) -The sandbox cannot prevent a script from **allocating unbounded memory**. A script calling `new byte[int.MaxValue]` repeatedly, or accumulating a large LINQ enumeration, can drive the server process to `OutOfMemoryException` before the 250 ms timeout fires. Script authoring is gated behind the Admin permission as the primary control; the test-harness preview (Stream F.4) allows operators to exercise a script before publishing. Out-of-process script execution is a v3 concern. +The sandbox cannot pre-empt a script. **Memory:** each run's own-thread allocation is sampled (`GC.GetAllocatedBytesForCurrentThread`) and a run over `Scripting:MaxAllocBytesPerRun` (default 64 MB) quarantines the script — but the sample is taken after the run returns, so a script that allocates fast enough (`new byte[int.MaxValue]` in a loop) can still OOM the process first. **CPU:** a tight loop keeps its pool thread after the timeout fires; `ScriptRunGuard` bounds that to one orphan per script by refusing further runs while it spins (Bad `"previous evaluation still running"`) and quarantines after three consecutive timeouts (a refusal is not a strike). Both are mitigations; pre-emptive CPU/memory limits need an out-of-process runner. Script authoring is gated behind the Admin permission as the primary control. See `docs/ScriptEditor.md` §"Runtime safety". -Similarly, **`System.Threading.Tasks` is now denied** (Core.Scripting-003), which prevents `Task.Run` / `Parallel` fan-out that would spawn background work outliving the timeout. However, a tight CPU-bound loop still runs on its thread-pool thread after `WaitAsync` returns — see the `TimedScriptEvaluator` remarks for detail. The orphaned thread is reclaimed when the Roslyn runtime eventually returns; in practice the operator fixes the script once the structured timeout warning appears in `scripts-*.log`. +`System.Threading.Tasks` is denied (Core.Scripting-003), which prevents `Task.Run` / `Parallel` fan-out that would spawn background work outliving the timeout. ### Compile cache (`CompiledScriptCache`) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs index 3b8d0bb1..96a25da5 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs @@ -463,6 +463,10 @@ public static class DraftValidator var (changeTriggered, timerIntervalMs) = ParseTriggers(t.TagConfig); if (timerIntervalMs is > 0) continue; var source = scriptId is not null && scriptSourceById.TryGetValue(scriptId, out var s) ? s : null; + // An unresolved scriptId is already CalculationScriptMissing / CalculationScriptNotFound; with no + // source there are no reads to count, so a change-triggered tag would get a second, misleading + // "no literal ctx.GetTag read" error. (changeTriggered=false + no timer is source-independent.) + if (changeTriggered && source is null) continue; var depCount = EquipmentScriptPaths.ExtractDependencyRefs(source).Count; if (changeTriggered && depCount > 0) continue; errors.Add(new("CalculationNoEffectiveTrigger", diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs index ae6352f5..c96c8c87 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs @@ -188,6 +188,15 @@ public sealed class ScriptedAlarmEngine : IDisposable /// (DrainDirtyAsync, ShelvingCheckAsync) runs on its token, so a store call that honours /// cancellation returns instead of outliving the engine. private readonly CancellationTokenSource _shutdownCts = new(); + + /// 's token, captured once: background work reads this, never the CTS, + /// so can dispose the CTS without racing a late reader. + private readonly CancellationToken _shutdownToken; + + private volatile bool _shutdownCtsDisposed; + + /// Test hook — whether disposed . + internal bool ShutdownCtsDisposedForTest => _shutdownCtsDisposed; private readonly Dictionary> _alarmsReferencing = new(StringComparer.Ordinal); // tag path -> alarm ids @@ -247,6 +256,7 @@ public sealed class ScriptedAlarmEngine : IDisposable _clock = clock ?? (() => DateTime.UtcNow); _scriptTimeout = scriptTimeout ?? TimedScriptEvaluator.DefaultTimeout; _guard = new ScriptRunGuard(scripting, _engineLogger); + _shutdownToken = _shutdownCts.Token; } /// The run guard every predicate evaluation goes through (diagnostics / tests). @@ -611,7 +621,7 @@ public sealed class ScriptedAlarmEngine : IDisposable // Started outside the lock: DrainDirtyAsync runs synchronously up to its // first real await, and nothing it does should happen under _dirtyLock. - if (startDrain) TrackBackgroundTask(() => DrainDirtyAsync(_shutdownCts.Token)); + if (startDrain) TrackBackgroundTask(() => DrainDirtyAsync(_shutdownToken)); } /// @@ -754,7 +764,7 @@ public sealed class ScriptedAlarmEngine : IDisposable } finally { _evalGate.Release(); } } - catch (OperationCanceledException) when (_shutdownCts.IsCancellationRequested) + catch (OperationCanceledException) when (_shutdownToken.IsCancellationRequested) { // Dispose cut the drain short after its bound — expected, not a failure. return; @@ -1080,7 +1090,7 @@ public sealed class ScriptedAlarmEngine : IDisposable { if (_disposed) return; var ids = _alarms.Keys.ToArray(); - TrackBackgroundTask(() => ShelvingCheckAsync(ids, _shutdownCts.Token)); + TrackBackgroundTask(() => ShelvingCheckAsync(ids, _shutdownToken)); } /// @@ -1219,7 +1229,7 @@ public sealed class ScriptedAlarmEngine : IDisposable } finally { _evalGate.Release(); } } - catch (OperationCanceledException) when (_shutdownCts.IsCancellationRequested) + catch (OperationCanceledException) when (_shutdownToken.IsCancellationRequested) { return; // Dispose cut the check short after its bound } @@ -1304,6 +1314,16 @@ public sealed class ScriptedAlarmEngine : IDisposable _alarmsReferencing.Clear(); _scratchByAlarmId.Clear(); _lastQualityBucketByAlarmId.Clear(); + + // Release the shutdown CTS once nothing can still be observing it through a registration: every + // drained task has completed. A task still stuck past the grace keeps its registration, so the CTS is + // left to the GC in that case (logged above). + if (toAwait.All(t => t.IsCompleted)) + { + _shutdownCts.Dispose(); + _shutdownCtsDisposed = true; + } + // 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- diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogRateLimiter.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogRateLimiter.cs index 17a6f2f6..5cce5fcd 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogRateLimiter.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogRateLimiter.cs @@ -43,6 +43,9 @@ public sealed class ScriptLogRateLimiter _clock = clock ?? (static () => DateTime.UtcNow); } + /// Entries admitted per source per window, as configured (after clamping to at least 1). + public int LimitPerWindow => _limit; + /// The window length. public TimeSpan Window => _window; diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogTopicSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogTopicSink.cs index a7b8f03e..88bdd973 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogTopicSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptLogTopicSink.cs @@ -83,7 +83,7 @@ public sealed class ScriptLogTopicSink : ILogEventSink Level: nameof(LogEventLevel.Warning), Message: $"{admission.SuppressedInPreviousWindow} script-log entr{(admission.SuppressedInPreviousWindow == 1 ? "y" : "ies")} " + $"from this source were suppressed in the previous {_limiter.Window.TotalSeconds:F0} s " + - $"(limit {ScriptLogRateLimiter.DefaultLimitPerWindow}/window); see scripts-*.log for the full stream", + $"(limit {_limiter.LimitPerWindow}/window); see scripts-*.log for the full stream", TimestampUtc: logEvent.Timestamp.UtcDateTime, VirtualTagId: virtualTagId, AlarmId: alarmId, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs index 905fff13..9f8ef41b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs @@ -111,7 +111,7 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba private CalculationTimerScheduler? _timers; private volatile bool _running; private DateTime? _lastComputeUtc; - private bool _disposed; + private volatile bool _disposed; /// Occurs when a calc tag's computed value changes (or transitions Good↔Bad). public event EventHandler? OnDataChange; @@ -538,7 +538,7 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba // _evalGate (never _lock) is held across the run, so the pump and the timer ticks still // evaluate one at a time while no host thread can block behind a script. - await _evalGate.WaitAsync().ConfigureAwait(false); + if (!await TryEnterEvalGateAsync().ConfigureAwait(false)) return; // disposed try { await RunEvaluationAsync(next).ConfigureAwait(false); @@ -566,7 +566,7 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba PendingEvaluation? prepared; lock (_lock) prepared = PrepareEvaluation(rawPath, DateTime.UtcNow); if (prepared is null) return; - await _evalGate.WaitAsync().ConfigureAwait(false); + if (!await TryEnterEvalGateAsync().ConfigureAwait(false)) return; // disposed try { await RunEvaluationAsync(prepared).ConfigureAwait(false); @@ -577,6 +577,25 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba } } + /// Acquire ; false once has disposed it (a pump + /// iteration or timer tick that raced the teardown simply stops). + private async Task TryEnterEvalGateAsync() + { + if (_disposed) return false; + try + { + await _evalGate.WaitAsync().ConfigureAwait(false); + } + catch (ObjectDisposedException) + { + return false; + } + if (!_disposed) return true; + // Dispose landed while we waited (it gave up the gate without disposing it): back out. + _evalGate.Release(); + return false; + } + /// Run one prepared evaluation and publish the outcome. MUST be called under /// and NOT under : the script run is the whole point of the /// split, and the script log + fan-out both invoke subscriber code. @@ -784,11 +803,33 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba StopTimers(); lock (_lock) _pending.Clear(); // Cancel an awaited run first (the guard lets the caller's cancel win over the timeout), then drop - // the compiled scripts. The CTS is not disposed: a pump mid-await may still read its token. + // the compiled scripts. try { _evalCts.Cancel(); } catch (ObjectDisposedException) { } _evaluator.Dispose(); + + // Then take the gate: once Dispose holds it no evaluation is mid-run (the token — read only under + // the gate — has been observed), so the gate and the CTS can be disposed. A late pump iteration or + // timer tick sees _disposed / an ObjectDisposedException in TryEnterEvalGateAsync and stops. Bounded: + // a run that ignores the cancel cannot hold Dispose; in that case both are left to the GC. + if (_evalGate.Wait(DisposeGateTimeout)) + { + _evalCts.Dispose(); + _evalGate.Dispose(); + _evalResourcesDisposed = true; + } + else + { + _logger.LogWarning("Calculation {Driver}: an evaluation did not release the gate within {Timeout} of Dispose; leaving it to the GC", + _driverInstanceId, DisposeGateTimeout); + } } + private static readonly TimeSpan DisposeGateTimeout = TimeSpan.FromSeconds(2); + private volatile bool _evalResourcesDisposed; + + /// Test hook — whether disposed the evaluation gate and its CTS. + internal bool EvalResourcesDisposedForTest => _evalResourcesDisposed; + /// Per-tag published state (guarded by ). private sealed class TagState { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs index 63ea8736..8c7b4f41 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs @@ -377,7 +377,7 @@ public sealed class VirtualTagActor : ReceiveActor { PublishLog("Warning", $"{admission.SuppressedInPreviousWindow} further evaluation failure(s) were suppressed from the script log " + - $"in the previous {_failureLogLimiter.Window.TotalSeconds:F0} s (limit {ScriptLogRateLimiter.DefaultLimitPerWindow}/window); last: {reason}"); + $"in the previous {_failureLogLimiter.Window.TotalSeconds:F0} s (limit {_failureLogLimiter.LimitPerWindow}/window); last: {reason}"); } if (admission.Admitted) PublishLog(level, reason); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCalculationTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCalculationTests.cs index b542871f..b68fb1cd 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCalculationTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCalculationTests.cs @@ -55,7 +55,10 @@ public sealed class DraftValidatorCalculationTests Tags = [CalcTagRow("t-1", "dev-1", "a", "{\"changeTriggered\":true}")], }; - DraftValidator.Validate(draft).ShouldContain(e => e.Code == "CalculationScriptMissing" && e.Context == "t-1"); + var errors = DraftValidator.Validate(draft); + errors.ShouldContain(e => e.Code == "CalculationScriptMissing" && e.Context == "t-1"); + errors.ShouldNotContain(e => e.Code == "CalculationNoEffectiveTrigger", + "an unresolved script has no reads to count — the missing script is the one error to report"); } [Fact] @@ -70,7 +73,10 @@ public sealed class DraftValidatorCalculationTests Scripts = [], }; - DraftValidator.Validate(draft).ShouldContain(e => e.Code == "CalculationScriptNotFound" && e.Context == "t-1"); + var errors = DraftValidator.Validate(draft); + errors.ShouldContain(e => e.Code == "CalculationScriptNotFound" && e.Context == "t-1"); + errors.ShouldNotContain(e => e.Code == "CalculationNoEffectiveTrigger", + "an unresolved script has no reads to count — the missing script is the one error to report"); } [Fact] diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineLifecycleTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineLifecycleTests.cs index 7afea07a..2e9b7df6 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineLifecycleTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineLifecycleTests.cs @@ -82,6 +82,7 @@ public sealed class ScriptedAlarmEngineLifecycleTests sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(5), "the drain is bounded; a hung store cannot hang shutdown"); await WaitForAsync(() => store.ObservedCancellation, timeoutMs: 2000); store.ObservedCancellation.ShouldBeTrue("the in-flight save was cancelled through the engine's shutdown token"); + eng.ShutdownCtsDisposedForTest.ShouldBeTrue("the cancelled save completed inside the grace, so the CTS is released"); } [Fact] @@ -105,6 +106,8 @@ public sealed class ScriptedAlarmEngineLifecycleTests store.SaveInProgress.ShouldBeFalse(); store.ObservedCancellation.ShouldBeFalse("a save that completes within the bound is never cancelled"); store.Saved.ShouldContain("A"); + eng.ShutdownCtsDisposedForTest.ShouldBeTrue("every background task completed, so the shutdown CTS is disposed"); + Should.NotThrow(() => eng.OnUpstreamChange("Temp", new DataValueSnapshot(1, 0u, DateTime.UtcNow, DateTime.UtcNow))); } private static async Task WaitForAsync(Func cond, int timeoutMs = 5000) diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptLogRateLimiterTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptLogRateLimiterTests.cs index 3f560e40..70674942 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptLogRateLimiterTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptLogRateLimiterTests.cs @@ -59,6 +59,9 @@ public sealed class ScriptLogRateLimiterTests ScriptLogRateLimiter.DefaultLimitPerWindow.ShouldBe(20); ScriptLogRateLimiter.DefaultWindow.ShouldBe(TimeSpan.FromMinutes(1)); var limiter = new ScriptLogRateLimiter(); + limiter.LimitPerWindow.ShouldBe(20); + new ScriptLogRateLimiter(limitPerWindow: 3).LimitPerWindow.ShouldBe(3); + new ScriptLogRateLimiter(limitPerWindow: 0).LimitPerWindow.ShouldBe(1, "values below 1 are treated as 1"); for (var i = 0; i < 20; i++) limiter.Admit("s").Admitted.ShouldBeTrue(); limiter.Admit("s").Admitted.ShouldBeFalse(); } @@ -101,6 +104,7 @@ public sealed class ScriptLogRateLimiterTests var summary = s1[2]; summary.Message.ShouldContain("3"); summary.Message.ShouldContain("suppressed"); + summary.Message.ShouldContain("limit 2/window", Case.Sensitive, "the summary names the CONFIGURED limit, not the default"); summary.VirtualTagId.ShouldBe("V1", "the summary carries the source's identity so the page attributes it"); summary.Level.ShouldBe("Warning"); s1[3].Message.ShouldBe("chatty again"); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverRemediationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverRemediationTests.cs index 88015ffc..0372ea15 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverRemediationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverRemediationTests.cs @@ -50,6 +50,29 @@ public sealed class CalculationDriverRemediationTests driver.IsEvaluationIdle.ShouldBeTrue("the calc driver's evaluation pump should have drained"); } + /// Fix round 1 minor: Dispose releases the evaluation gate and its cancellation source — even + /// with a runaway run in flight (the guard lets the caller's cancel win, so the awaited run returns + /// promptly) — and a trigger arriving after Dispose is harmless. + [Fact] + public async Task Dispose_with_a_run_in_flight_disposes_the_eval_gate_and_a_late_trigger_is_harmless() + { + var (driver, _, _, _) = Build( + CalcTag("calc/e/spin", "var v = ctx.GetTag(\"src/a\").Value; var end = System.DateTime.UtcNow.AddSeconds(6); while (System.DateTime.UtcNow < end) { } return 1;")); + await driver.InitializeAsync("{}", TestContext.Current.CancellationToken); + driver.OnDependencyValue("src/a", 1, StatusGood, DateTime.UtcNow); + var deadline = DateTime.UtcNow.AddSeconds(10); + while (driver.IsEvaluationIdle && DateTime.UtcNow < deadline) await Task.Delay(5); + driver.IsEvaluationIdle.ShouldBeFalse("positive control: a run is in flight when Dispose lands"); + + var sw = System.Diagnostics.Stopwatch.StartNew(); + driver.Dispose(); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(5), "the caller's cancel releases the awaited run; Dispose is not held for the 6 s spin"); + driver.EvalResourcesDisposedForTest.ShouldBeTrue(); + + Should.NotThrow(() => driver.OnDependencyValue("src/a", 2, StatusGood, DateTime.UtcNow)); + Should.NotThrow(() => driver.Dispose()); + } + [Fact] public async Task A_compile_failure_is_remembered_and_logged_once_across_repeated_triggers() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorFailureLogRateLimitTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorFailureLogRateLimitTests.cs index 74ee2e32..c1a6253e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorFailureLogRateLimitTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorFailureLogRateLimitTests.cs @@ -43,6 +43,7 @@ public sealed class VirtualTagActorFailureLogRateLimitTests : RuntimeActorTestBa var entries = capture.ToArray(); entries[3].Message.ShouldContain("7"); entries[3].Message.ShouldContain("suppressed"); + entries[3].Message.ShouldContain("limit 3/window", Case.Sensitive, "the summary names the CONFIGURED limit, not the default"); entries[3].VirtualTagId.ShouldBe("vt-chatty"); entries[4].Message.ShouldBe("boom"); }