diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 9bfd569f..4aae2261 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -96,7 +96,7 @@ flowchart TD > **Startup config load is retried (S5)**: step 1's deployed-config read is best-effort against local SQLite, which can transiently fail (locked/busy). A failed load must **not** leave the site as a silent zero-instance node — it is re-attempted on a fixed interval until it succeeds, then the retry self-cancels. -> **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). +> **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. ### Deployment Handling - Receives flattened instance configurations from central via the Communication Layer. @@ -238,6 +238,7 @@ When the Instance Actor is stopped (due to disable, delete, or redeployment), Ak - **Range Violation**: Value is outside the allowed min/max range. - **Rate of Change**: Value change rate exceeds the defined threshold over a configurable time window. Direction filter (rising / falling / either) restricts which side of the rate triggers. - **HiLo**: Multi-setpoint level alarm with up to four configurable setpoints (LoLo, Lo, Hi, HiHi). Any subset may be configured. Each setpoint may carry its own priority that overrides the alarm-level priority for that band. +- **Expression** trigger evaluation runs on the bounded script-execution thread pool shared with script bodies (and Expression *script* triggers): scheduler saturation — e.g. stuck scripts occupying all threads — delays Expression alarm transitions site-wide until a thread frees. This is a deliberate trade-off (evaluation no longer blocks dispatcher threads; per-actor coalescing bounds queue growth) and the scheduler gauges + stuck-script watchdog make the saturated state visible. - For binary trigger types (ValueMatch / RangeViolation / RateOfChange), when the condition is met and the alarm is currently in **normal** state, the alarm transitions to **active**: - Updates the alarm state on the parent Instance Actor (which publishes to the Akka stream). - If an on-trigger script is defined, spawns an Alarm Execution Actor to execute it. @@ -486,7 +487,7 @@ Per Akka.NET best practices, internal actor communication uses **Tell** (fire-an ### Script Compilation Errors - If script compilation fails when a deployment is received, the entire deployment for that instance is **rejected**. No partial state is applied. - The failure is reported back to central as a failed deployment. -- **Enforced by a site-side pre-compile validation gate (S3)**: before the Instance Actor is created or the config persisted, the Deployment Manager compiles **all** of the instance's scripts, alarm on-trigger scripts, and trigger expressions **off-thread**; any compile failure rejects the deployment with the collected compile errors. This makes the deployment result honest even if central's pre-deployment validation was bypassed or skew let a bad artifact through. +- **Enforced by a site-side pre-compile validation gate (S3)**: before the Instance Actor is created or the config persisted, the Deployment Manager compiles **all** of the instance's scripts, alarm on-trigger scripts, and trigger expressions **synchronously, as a pure prefix step of the deploy handler on the singleton's actor thread** — deliberately not off-thread, because piping the verdict back to self would reorder concurrent deploys against delete/disable and break the redeploy-supersede mailbox-FIFO ordering (see the in-code rationale in `DeploymentManagerActor.HandleDeploy`); unchanged script bodies hit the process-wide compile cache, so the gate recompiles only genuinely new code. Any compile failure rejects the deployment with the collected compile errors. This makes the deployment result honest even if central's pre-deployment validation was bypassed or skew let a bad artifact through. - Note: Pre-deployment validation at central should still catch compilation errors before they reach the site; the site gate is the authoritative backstop. --- diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs index 42c32655..e22e4769 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs @@ -105,14 +105,16 @@ public class MxGatewayDataConnection : IDataConnection, IBrowsableDataConnection // Background event loop: route each value change to the matching subscription callback. _eventLoopCts = new CancellationTokenSource(); - _ = Task.Run(() => RunEventLoopAsync(_eventLoopCts.Token)); + var loopToken = _eventLoopCts.Token; // capture NOW: the field may be replaced by a + var loopClient = _client; // subsequent reconnect before Task.Run executes (N1) + _ = Task.Run(() => RunEventLoopAsync(loopClient, loopToken)); } - private async Task RunEventLoopAsync(CancellationToken ct) + private async Task RunEventLoopAsync(IMxGatewayClient client, CancellationToken ct) { try { - await _client!.RunEventLoopAsync(update => + await client.RunEventLoopAsync(update => { if (_tagToSub.TryGetValue(update.TagPath, out var subId) && _subs.TryGetValue(subId, out var s)) s.Callback(update.TagPath, new TagValue(update.Value, update.Quality, update.Timestamp)); @@ -122,6 +124,18 @@ public class MxGatewayDataConnection : IDataConnection, IBrowsableDataConnection { // Normal shutdown (DisconnectAsync / DisposeAsync cancelled the loop). } + catch (Exception ex) when (ct.IsCancellationRequested) + { + // Teardown fault of a superseded loop (N1): Cancel() is not synchronous with + // the loop's exit — a cancelled/disposed gRPC stream commonly surfaces as + // RpcException(StatusCode.Cancelled) (Grpc.Net default without + // ThrowOperationCanceledOnCancellation) or ObjectDisposedException from the + // concurrent DisposeAsync, milliseconds AFTER ConnectAsync has already reset + // _disconnectFired. Any fault on a cancelled token is normal shutdown; + // raising Disconnected here would flap the NEW healthy session. + _logger.LogDebug(ex, + "MxGateway event loop faulted after cancellation — superseded loop teardown; not signalling disconnect"); + } catch (Exception ex) { _logger.LogWarning(ex, "MxGateway event stream faulted; signalling disconnect"); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs index 019e39f3..477c1fb1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs @@ -186,6 +186,10 @@ public class AlarmActor : ReceiveActor // Handle the off-dispatcher trigger-expression evaluation result (P1). Receive(HandleExpressionEvalResult); + + // Handle a faulted off-dispatcher evaluation task (N2) — clears in-flight + // instead of stranding the alarm trigger with an unhandled Status.Failure. + Receive(HandleExpressionEvalFailed); } /// @@ -551,7 +555,8 @@ public class AlarmActor : ReceiveActor } }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, - success: r => new ExpressionEvalResult(r)); + success: r => new ExpressionEvalResult(r), + failure: ex => new ExpressionEvalFailed(ex)); } /// @@ -576,6 +581,22 @@ public class AlarmActor : ReceiveActor if (_evalPending) { _evalPending = false; StartExpressionEvaluation(); } } + /// + /// Applies a faulted off-dispatcher evaluation task (N2): logs the fault, then + /// delegates to the false result (per the class contract — a throwing expression + /// is treated as false so the state machine still runs, an Active alarm clears) + /// which clears in-flight and drains any pending evaluation. Without this the + /// alarm trigger would be permanently parked with _evalInFlight stuck true. + /// + private void HandleExpressionEvalFailed(ExpressionEvalFailed msg) + { + _healthCollector?.IncrementAlarmError(); + _logger.LogError(msg.Cause, + "Alarm {Alarm} trigger-expression evaluation task faulted on {Instance}; treated as false", + _alarmName, _instanceName); + HandleExpressionEvalResult(new ExpressionEvalResult(false)); + } + /// /// HiLo level evaluator: returns the most-severe matching band for the /// given value. Severity order checked from highest to lowest so that a @@ -795,6 +816,15 @@ public class AlarmActor : ReceiveActor /// actor thread. /// private sealed record ExpressionEvalResult(bool Result); + + /// + /// Piped back to self when the off-dispatcher evaluation TASK itself faults + /// (e.g. ObjectDisposedException from a disposed ScriptExecutionScheduler + /// during shutdown) — the inner body's catch never sees that. Without this + /// mapping the actor receives an unhandled Status.Failure and _evalInFlight + /// stays true forever, permanently parking the expression trigger (N2). + /// + internal sealed record ExpressionEvalFailed(Exception Cause); } internal enum RateOfChangeDirection { Either, Rising, Falling } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index d5aa014c..e360dffc 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -518,7 +518,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers // delete/disable commands, breaking that ordering. A deploy is an // infrequent admin command, so briefly holding the singleton for a pure // Roslyn compile is acceptable; the central deployer already Asks and - // waits for the DeploymentStatusResponse. + // waits for the DeploymentStatusResponse. Redeploys and multi-instance + // deploys of unchanged scripts hit the process-wide compile cache + // (SiteScriptCompileCache), so the synchronous gate recompiles only + // genuinely new code and the Instance Actor's PreStart reuses the gate's + // compile (N4). var errors = _deployCompileValidator.Validate(command.FlattenedConfigurationJson); if (errors.Count > 0) { diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs index 68c8460a..65448f1f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs @@ -159,6 +159,10 @@ public class ScriptActor : ReceiveActor, IWithTimers // Handle the off-dispatcher trigger-expression evaluation result (P1). Receive(HandleExpressionEvalResult); + + // Handle a faulted off-dispatcher evaluation task (N2) — clears in-flight + // instead of stranding the trigger with an unhandled Status.Failure. + Receive(HandleExpressionEvalFailed); } /// @@ -310,7 +314,8 @@ public class ScriptActor : ReceiveActor, IWithTimers } }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, - success: r => new ExpressionEvalResult(r)); + success: r => new ExpressionEvalResult(r), + failure: ex => new ExpressionEvalFailed(ex)); } /// @@ -333,6 +338,18 @@ public class ScriptActor : ReceiveActor, IWithTimers if (_evalPending) { _evalPending = false; StartExpressionEvaluation(); } } + /// + /// Applies a faulted off-dispatcher evaluation task (N2): logs the fault via the + /// existing error path (which also increments the health counter), then reuses the + /// false-result path — clearing in-flight, applying the false edge, and draining any + /// pending evaluation — so a transient scheduler fault cannot permanently park the trigger. + /// + private void HandleExpressionEvalFailed(ExpressionEvalFailed msg) + { + LogExpressionError(msg.Cause); + HandleExpressionEvalResult(new ExpressionEvalResult(false)); + } + /// /// Applies a WhileTrue trigger's condition-state transition: on the /// false→true edge, fire once and start the re-fire timer; on the @@ -621,6 +638,15 @@ public class ScriptActor : ReceiveActor, IWithTimers /// the actor thread. /// private sealed record ExpressionEvalResult(bool Result); + + /// + /// Piped back to self when the off-dispatcher evaluation TASK itself faults + /// (e.g. ObjectDisposedException from a disposed ScriptExecutionScheduler + /// during shutdown) — the inner body's catch never sees that. Without this + /// mapping the actor receives an unhandled Status.Failure and _evalInFlight + /// stays true forever, permanently parking the expression trigger (N2). + /// + internal sealed record ExpressionEvalFailed(Exception Cause); } // ── Trigger config types ── diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Deployment/DeployCompileValidator.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Deployment/DeployCompileValidator.cs index d2241395..196a1eec 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Deployment/DeployCompileValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Deployment/DeployCompileValidator.cs @@ -14,8 +14,11 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Deployment; /// deployment; no partial state is applied). /// /// Because it holds no actor state and only calls the (stateless) -/// , it is safe to run off the singleton -/// mailbox on a Task.Run thread — the deploy path never blocks on Roslyn. +/// , it is pure and thread-safe; the Deployment +/// Manager nevertheless invokes it synchronously on the singleton's actor thread +/// as a pure prefix step, preserving mailbox-FIFO deploy ordering (see +/// ). Compiles of unchanged +/// script bodies are deduped by the process-wide compile cache. /// public sealed class DeployCompileValidator { diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs index d08692d9..485323f3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs @@ -13,6 +13,13 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; /// ; this service keeps the real /// execution-path compile of the script against / /// . +/// +/// Compile results — success AND failure — are memoised process-wide in +/// (keyed on code hash + globals type), so an +/// unchanged script body compiles once per process lifetime and the deploy gate, +/// the Instance Actor's PreStart recompile, and SharedScriptLibrary all reuse one +/// compiled Script<object?>. Error text is name-free by construction, so a +/// cached failure renders correctly for every caller. /// public class ScriptCompilationService { @@ -92,10 +99,33 @@ public class ScriptCompilationService => CompileCore(name, expression, typeof(TriggerExpressionGlobals)); /// - /// Shared compilation path: validates the trust model, builds the script - /// against the given globals type, and returns the compiled result. + /// Shared compilation funnel for every site-side compile (the deploy gate, the + /// Instance Actor's PreStart recompile, and SharedScriptLibrary). Results — success + /// AND failure — are memoised process-wide in , + /// keyed on code hash + globals type: an unchanged script body compiles once per + /// process lifetime and the compiled Script<object?> is shared across all + /// call sites (N4). Cached error text is name-free by construction (Roslyn diagnostics / + /// the compile-exception message never embed the caller's script name), so a cached + /// failure renders correctly for every caller. /// private ScriptCompilationResult CompileCore(string name, string code, Type globalsType) + { + var result = SiteScriptCompileCache.GetOrAdd(code, globalsType, + () => CompileUncached(name, code, globalsType)); + if (!result.IsSuccess) + _logger.LogDebug( + "Script {Script}: returning cached compile failure ({ErrorCount} error(s))", + name, result.Errors.Count); // miss-path logs name the FIRST caller; keep this caller visible + return result; + } + + /// + /// The uncached compile: validates the trust model, builds the script against the + /// given globals type, and returns the compiled result. The verdict is deterministic + /// on the code (plus the compile-time-static trust policy and globals surface), so it + /// is safe to cache alongside the compiled script. + /// + private ScriptCompilationResult CompileUncached(string name, string code, Type globalsType) { // Validate trust model var violations = ValidateTrustModel(code); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs new file mode 100644 index 00000000..0b025af8 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs @@ -0,0 +1,84 @@ +using System.Collections.Concurrent; +using System.Security.Cryptography; +using System.Text; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +/// +/// Process-wide cache of site-side script s, keyed on +/// the globals type plus a SHA-256 hash of the script code. Unlike the TemplateEngine's +/// ScriptCompileVerdictCache (which caches central-validation verdicts), this +/// caches the full compiled result — the immutable Roslyn Script<object?> itself — +/// so the synchronous deploy gate and the InstanceActor.PreStart recompile share one +/// Roslyn compile per script body per process lifetime (N4). +/// +/// Soundness rests on three invariants: +/// +/// A compile result is a pure function of (code, globals type, compile-time-static +/// trust policy) — none of which vary at runtime for a given key. +/// The stored error text is name-free (Roslyn Diagnostic.GetMessage() / +/// "Compilation exception: …"); callers prefix their own script-name context, so a +/// shared cached failure renders correctly for every caller. Failures are cached too — +/// a bad script body fails identically for every caller. +/// Roslyn Script<T> is immutable and thread-safe (RunAsync creates +/// per-run state), so one compiled instance may be shared across actors and threads. +/// +/// +/// +/// Bounded at entries — deliberately smaller than the verdict cache's +/// 4096 because entries pin compiled assemblies, not just verdict strings. On overflow the cache +/// is cleared wholesale (results are recomputable, so a coarse reset avoids eviction bookkeeping). +/// // are exposed for tests and diagnostics. +/// +/// +internal static class SiteScriptCompileCache +{ + /// Upper bound on cached entries; the cache is cleared wholesale on overflow. + internal const int MaxEntries = 1024; + + private static readonly ConcurrentDictionary Cache = new(); + private static long _hits; + + /// Number of cache hits observed since the last . + public static long Hits => Interlocked.Read(ref _hits); + + /// Current number of cached compile results. + public static int Count => Cache.Count; + + /// + /// Returns the cached compile result for against + /// , or computes it via and caches + /// it on a miss. A hit increments . Both success and failure results are + /// cached — the error text is name-free by construction. + /// + /// The script source code to look up (hashed to form the cache key). + /// The Roslyn globals surface the script compiles against; part of the key so identical source under different globals stays distinct. + /// Computes the compile result on a cache miss. + /// The cached or newly computed compile result. + public static ScriptCompilationResult GetOrAdd(string code, Type globalsType, Func factory) + { + var key = globalsType.FullName + ":" + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code))); + + if (Cache.TryGetValue(key, out var result)) + { + Interlocked.Increment(ref _hits); + return result; + } + + result = factory(); + + // Coarse bound: on overflow drop everything rather than track evictions. + if (Cache.Count >= MaxEntries) + Cache.Clear(); + + Cache[key] = result; + return result; + } + + /// Clears all cached compile results and resets the hit counter. + public static void Clear() + { + Cache.Clear(); + Interlocked.Exchange(ref _hits, 0); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs index 8de19199..e34ef736 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs @@ -56,5 +56,14 @@ public sealed class SiteRuntimeOptionsValidator : OptionsValidatorBase= 0, $"ScadaBridge:SiteRuntime:ConfigFetchRetryCount must be >= 0 " + $"(was {options.ConfigFetchRetryCount})."); + + builder.RequireThat(options.TagSubscribeRetryIntervalMs > 0, + $"ScadaBridge:SiteRuntime:TagSubscribeRetryIntervalMs must be greater than 0 " + + $"(was {options.TagSubscribeRetryIntervalMs}); a zero interval hot-loops the tag-subscribe " + + "retry and a negative one throws inside the Instance Actor's retry scheduling."); + + builder.RequireThat(options.StuckScriptGraceMs >= 0, + $"ScadaBridge:SiteRuntime:StuckScriptGraceMs must be >= 0 " + + $"(was {options.StuckScriptGraceMs}); a negative grace throws inside the stuck-script watchdog's delay."); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionReconnectTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionReconnectTests.cs index cb032d61..b50cbfd3 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionReconnectTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionReconnectTests.cs @@ -1,3 +1,4 @@ +using Grpc.Core; using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; @@ -47,6 +48,73 @@ public class MxGatewayDataConnectionReconnectTests Assert.False(loopTokens[1].IsCancellationRequested); // new loop live } + [Fact] + public async Task StaleEventLoopRpcFault_AfterReconnect_DoesNotSignalDisconnected() + { + var loops = new List(); + var loopTokens = new List(); + var adapter = CreateAdapterWithControllableLoops(loops, loopTokens, out var disconnectsRef); + + var details = new Dictionary { ["Endpoint"] = "http://gw:5000", ["ApiKey"] = "k" }; + + await adapter.ConnectAsync(details); + await WaitUntilAsync(() => loopTokens.Count == 1); + await adapter.ConnectAsync(details); // reconnect: cancels loop #0, resets _disconnectFired + await WaitUntilAsync(() => loopTokens.Count == 2); + + // The OLD loop observes its cancellation as a gRPC fault, not an OCE — + // the Grpc.Net default without ThrowOperationCanceledOnCancellation. + loops[0].SetException(new RpcException(new Status(StatusCode.Cancelled, "call cancelled"))); + await Task.Delay(200); + Assert.Equal(0, Volatile.Read(ref disconnectsRef.Value)); // stale fault absorbed — the fresh connection must not flap + + // The CURRENT loop's genuine fault must still signal, exactly once. + loops[1].SetException(new RpcException(new Status(StatusCode.Unavailable, "gateway gone"))); + await WaitUntilAsync(() => Volatile.Read(ref disconnectsRef.Value) == 1); + } + + [Fact] + public async Task StaleEventLoopObjectDisposedFault_AfterReconnect_DoesNotSignalDisconnected() + { + var loops = new List(); + var loopTokens = new List(); + var adapter = CreateAdapterWithControllableLoops(loops, loopTokens, out var disconnectsRef); + + var details = new Dictionary { ["Endpoint"] = "http://gw:5000", ["ApiKey"] = "k" }; + + await adapter.ConnectAsync(details); + await WaitUntilAsync(() => loopTokens.Count == 1); + await adapter.ConnectAsync(details); // reconnect + await WaitUntilAsync(() => loopTokens.Count == 2); + + // The old loop faults with the concurrent-DisposeAsync shape. + loops[0].SetException(new ObjectDisposedException("MxGatewayClient")); + await Task.Delay(200); + Assert.Equal(0, Volatile.Read(ref disconnectsRef.Value)); + } + + private sealed class IntRef { public int Value; } + + private static MxGatewayDataConnection CreateAdapterWithControllableLoops( + List loops, List loopTokens, out IntRef disconnectsRef) + { + var factory = Substitute.For(); + factory.Create().Returns(_ => + { + var c = Substitute.For(); + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + loops.Add(tcs); + c.RunEventLoopAsync(Arg.Any>(), Arg.Do(t => loopTokens.Add(t))) + .Returns(tcs.Task); + return c; + }); + var adapter = new MxGatewayDataConnection(factory, NullLogger.Instance); + var refHolder = new IntRef(); + adapter.Disconnected += () => Interlocked.Increment(ref refHolder.Value); + disconnectsRef = refHolder; + return adapter; + } + private static async Task WaitUntilAsync(Func condition) { for (var i = 0; i < 100 && !condition(); i++) diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs index 2707607d..3d5d2050 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs @@ -1054,4 +1054,31 @@ public class AlarmActorTests : TestKit, IDisposable var change = instanceProbe.ExpectMsg(TimeSpan.FromSeconds(10)); Assert.Equal(AlarmState.Active, change.State); } + + [Fact] + public void ExpressionEvalTaskFault_DoesNotKillTheAlarmTrigger() + { + var expr = CompileRawTriggerExpression("true"); + var alarmConfig = new ResolvedAlarm + { + CanonicalName = "ExprFaultAlarm", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"true\"}", + PriorityLevel = 1 + }; + var instanceProbe = CreateTestProbe(); + var alarm = ActorOf(Props.Create(() => new AlarmActor( + "ExprFaultAlarm", "Pump1", instanceProbe.Ref, alarmConfig, + null, _sharedLibrary, _options, + NullLogger.Instance, expr))); + + // Simulate the faulted scheduler task: must be handled (logged, in-flight + // cleared, false applied) — NOT an unhandled Status.Failure. + alarm.Tell(new AlarmActor.ExpressionEvalFailed(new ObjectDisposedException("scheduler"))); + + // The trigger must still be alive: the next change evaluates and activates. + alarm.Tell(new AttributeValueChanged("Pump1", "A", "A", 1, "Good", DateTimeOffset.UtcNow)); + var change = instanceProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.Equal(AlarmState.Active, change.State); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs index c5965c1b..1e1a722d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs @@ -343,13 +343,41 @@ public class ScriptActorTests : TestKit, IDisposable private static Script CompileRawTriggerExpression(string expression) { var opts = ScriptOptions.Default - .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly) + // The test assembly is referenced so a raw-compiled expression may reference + // test-only hooks like EvalGate (used by the faulted-task N2 test). + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, typeof(ScriptActorTests).Assembly) .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); var s = CSharpScript.Create(expression, opts, typeof(TriggerExpressionGlobals)); s.Compile(); return s; } + [Fact] + public void ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation() + { + var expr = CompileRawTriggerExpression( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.EvalGate.Block()"); + var (actor, _) = CreateTriggeredActor( + "ExprFault", "Expression", "{\"expression\":\"true\",\"mode\":\"OnTrue\"}", null, expr); + try + { + actor.Tell(Change("A", "1")); // eval starts on the scheduler and BLOCKS → _evalInFlight = true + AwaitAssert(() => Assert.Equal(1, EvalGate.Entries), TimeSpan.FromSeconds(10)); + actor.Tell(Change("A", "2")); // coalesces → _evalPending = true + + // Simulate the faulted scheduler task the PipeTo failure mapping now surfaces + // (e.g. ObjectDisposedException from a disposed ScriptExecutionScheduler). + actor.Tell(new ScriptActor.ExpressionEvalFailed(new ObjectDisposedException("scheduler"))); + + // In-flight cleared + pending drained ⇒ a SECOND evaluation starts and blocks too. + AwaitAssert(() => Assert.Equal(2, EvalGate.Entries), TimeSpan.FromSeconds(10)); + } + finally + { + EvalGate.Gate.Release(EvalGate.Entries); // ALWAYS free the shared scheduler threads + } + } + [Fact] public void ExpressionTrigger_EvaluatesOnScriptSchedulerThread_AndStillFires() { @@ -504,3 +532,17 @@ public class ScriptActorTests : TestKit, IDisposable instance.ExpectMsg(TimeSpan.FromSeconds(2)); } } + +/// +/// Test-only hook referenced by a raw-compiled trigger expression: +/// blocks the calling script-scheduler thread on so a test can hold an +/// evaluation "in flight" and count how many evaluations have entered. Used by +/// ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation. +/// +public static class EvalGate +{ + public static readonly SemaphoreSlim Gate = new(0); + private static int _entries; + public static int Entries => Volatile.Read(ref _entries); + public static bool Block() { Interlocked.Increment(ref _entries); Gate.Wait(); return false; } +} 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 f02aca5d..d654ff23 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs @@ -16,6 +16,7 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; /// Stopwatch stays allowed. The real execution-path compile against /// ScriptGlobals / TriggerExpressionGlobals is unchanged. /// +[Collection("SiteScriptCompileCache")] public class ScriptCompilationServiceTests { private readonly ScriptCompilationService _service; @@ -34,6 +35,28 @@ public class ScriptCompilationServiceTests Assert.Empty(result.Errors); } + [Fact] + public void Compile_SameCodeTwice_SharesOneRoslynCompile() + { + SiteScriptCompileCache.Clear(); + var r1 = _service.Compile("deploy-gate-copy", "return 1 + 1;"); + var r2 = _service.Compile("prestart-copy", "return 1 + 1;"); + + Assert.True(r1.IsSuccess); + Assert.Same(r1.CompiledScript, r2.CompiledScript); // one compile, shared Script (N4) + Assert.Equal(1, SiteScriptCompileCache.Hits); + } + + [Fact] + public void Compile_ScriptAndTriggerExpression_DoNotCrossContaminate() + { + SiteScriptCompileCache.Clear(); + var script = _service.Compile("s", "1 > 0"); + var trigger = _service.CompileTriggerExpression("t", "1 > 0"); + + Assert.NotSame(script.CompiledScript, trigger.CompiledScript); // different globals surfaces + } + [Fact] public void Compile_InvalidSyntax_ReturnsErrors() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs new file mode 100644 index 00000000..435863a9 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs @@ -0,0 +1,54 @@ +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; + +/// +/// Tests for the process-wide . The cache is +/// static/process-wide state, so this class shares the "SiteScriptCompileCache" xunit +/// collection with ScriptCompilationServiceTests (no cross-class parallelism) and +/// calls at the top of each test. +/// +[Collection("SiteScriptCompileCache")] +public class SiteScriptCompileCacheTests +{ + private static ScriptCompilationResult Ok() => + ScriptCompilationResult.Succeeded(CSharpScript.Create("1", ScriptOptions.Default, typeof(ScriptGlobals))); + + [Fact] + public void GetOrAdd_SameCodeAndGlobals_ComputesOnce() + { + SiteScriptCompileCache.Clear(); + var factoryCalls = 0; + ScriptCompilationResult Factory() { factoryCalls++; return Ok(); } + + var r1 = SiteScriptCompileCache.GetOrAdd("return 1;", typeof(ScriptGlobals), Factory); + var r2 = SiteScriptCompileCache.GetOrAdd("return 1;", typeof(ScriptGlobals), Factory); + + Assert.Equal(1, factoryCalls); + Assert.Same(r1, r2); + Assert.Equal(1, SiteScriptCompileCache.Hits); + } + + [Fact] + public void GetOrAdd_SameCode_DifferentGlobals_AreSeparateEntries() + { + SiteScriptCompileCache.Clear(); + SiteScriptCompileCache.GetOrAdd("1 > 0", typeof(ScriptGlobals), Ok); + SiteScriptCompileCache.GetOrAdd("1 > 0", typeof(TriggerExpressionGlobals), Ok); + + Assert.Equal(2, SiteScriptCompileCache.Count); + Assert.Equal(0, SiteScriptCompileCache.Hits); // no cross-globals hit + } + + [Fact] + public void Overflow_ClearsWholesale() + { + SiteScriptCompileCache.Clear(); + for (var i = 0; i <= SiteScriptCompileCache.MaxEntries; i++) + SiteScriptCompileCache.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok); + + Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs index e8bbf54e..1d627316 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs @@ -40,6 +40,24 @@ public class SiteRuntimeOptionsValidatorTests Assert.Contains("ScriptExecutionThreadCount", result.FailureMessage); } + [Fact] + public void ZeroTagSubscribeRetryIntervalMs_IsRejected() + { + var result = Validate(new SiteRuntimeOptions { TagSubscribeRetryIntervalMs = 0 }); + + Assert.True(result.Failed); + Assert.Contains("TagSubscribeRetryIntervalMs", result.FailureMessage); + } + + [Fact] + public void NegativeStuckScriptGraceMs_IsRejected() + { + var result = Validate(new SiteRuntimeOptions { StuckScriptGraceMs = -1 }); + + Assert.True(result.Failed); + Assert.Contains("StuckScriptGraceMs", result.FailureMessage); + } + [Fact] public void ZeroStreamBufferSize_IsRejected() {