From b1b487409019e7765e01504e3f07d6028f5e2095 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 17 Jul 2026 15:36:39 -0400 Subject: [PATCH] fix(siteruntime): injectable ScriptExecutionScheduler + un-leakable expression-fault test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea #18: the process-wide ScriptExecutionScheduler was reached via a static singleton (Shared) directly inside ScriptActor, ScriptExecutionActor, AlarmActor, AlarmExecutionActor, and ScriptSchedulerStatsReporter, so it could not be substituted — a shared mutable global for anything running more than one logical site in a process, most visibly the test assembly. Add an optional scheduler injection seam to all five: the Host injects nothing and gets the shared pool (byte-for-byte unchanged), while a test (or a future multi-site host) hands an actor its own instance. Spawning actors thread their scheduler down to the execution children they create. Shared() now also recreates a disposed cached instance rather than returning it (new IsDisposed guard), so a disposed scheduler can never silently poison every later script/alarm execution. Gitea #16: ScriptActorTests now runs on its OWN scheduler (disposed at class teardown), so the faulted-task test can no longer strand a worker on the process-wide pool and starve unrelated test classes (one prior run failed 22 tests). EvalGate's wait is bounded to 30 s (was unbounded — the actual leak mechanism) and reset per test; the teardown releases one permit per worker instead of Release(Entries), which under-released in exactly the failure case. Full SiteRuntime suite: 6/6 runs green (524/524); was 3/6 failing on clean main. Fixes: #18 Fixes: #16 --- docs/requirements/Component-SiteRuntime.md | 1 + .../Actors/AlarmActor.cs | 22 +++- .../Actors/AlarmExecutionActor.cs | 17 ++- .../Actors/ScriptActor.cs | 21 +++- .../Actors/ScriptExecutionActor.cs | 19 +++- .../Scripts/ScriptExecutionScheduler.cs | 28 +++-- .../Scripts/ScriptSchedulerStatsReporter.cs | 13 ++- .../Actors/ScriptActorTests.cs | 103 ++++++++++++++++-- .../Scripts/ScriptExecutionSchedulerTests.cs | 18 +++ 9 files changed, 205 insertions(+), 37 deletions(-) diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 4aae2261..87561c90 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -216,6 +216,7 @@ When the Instance Actor is stopped (due to disable, delete, or redeployment), Ak - Executes the script in the Akka actor context. - Has access to the full Script Runtime API (see below). - Returns the script's return value (if defined) to the caller, then stops. +- The script body itself runs on the **dedicated `ScriptExecutionScheduler`** (a bounded set of dedicated threads), not the shared .NET thread pool, so blocking script I/O cannot starve the global pool or stall Akka dispatchers. The scheduler is **process-wide by default** (one pool per host, sized from `SiteRuntimeOptions.ScriptExecutionThreadCount`), but each script/alarm actor takes it through an **optional injection seam** rather than reaching for the static directly: the Host injects nothing and gets the shared pool, while tests (or a future multi-site host) can hand an actor its own instance. The shared accessor also recreates a disposed pool rather than returning it, so a disposed scheduler can never silently poison later executions. ### Handling `Instance.CallScript` - When an external caller (another Script Execution Actor, an Alarm Execution Actor, or a routed call from the Inbound API) sends a `CallScript` message to the Script Actor, it spawns a Script Execution Actor to handle the call. diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs index 477c1fb1..2abb5f24 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs @@ -41,6 +41,15 @@ public class AlarmActor : ReceiveActor private readonly ISiteHealthCollector? _healthCollector; private readonly IServiceProvider? _serviceProvider; + /// + /// Script-execution scheduler seam (#18): the process-wide + /// when null, or an injected instance so this + /// alarm's trigger-expression evaluation and spawned on-trigger scripts run on a + /// caller-owned pool. Resolved lazily at each use so the null (host) path is + /// unchanged. + /// + private readonly ScriptExecutionScheduler? _scheduler; + /// /// The optional site operational-event log, resolved once from /// at construction and cached. The @@ -125,6 +134,7 @@ public class AlarmActor : ReceiveActor /// The on-trigger script's per-script /// execution timeout in seconds (from its ), /// or null/non-positive to use the global default. + /// Optional script-execution scheduler override (#18); null uses the process-wide shared scheduler. public AlarmActor( string alarmName, string instanceName, @@ -139,7 +149,9 @@ public class AlarmActor : ReceiveActor ISiteHealthCollector? healthCollector = null, IServiceProvider? serviceProvider = null, // Per-script timeout for the on-trigger script (null = global). - int? onTriggerExecutionTimeoutSeconds = null) + int? onTriggerExecutionTimeoutSeconds = null, + // Script-execution scheduler seam (#18); null uses the process-wide shared scheduler. + ScriptExecutionScheduler? scheduler = null) { _alarmName = alarmName; _instanceName = instanceName; @@ -149,6 +161,7 @@ public class AlarmActor : ReceiveActor _logger = logger; _healthCollector = healthCollector; _serviceProvider = serviceProvider; + _scheduler = scheduler; // Resolve the optional site event logger once and cache it, // rather than calling GetService on every alarm transition. _siteEventLogger = serviceProvider?.GetService(); @@ -554,7 +567,7 @@ public class AlarmActor : ReceiveActor return false; } }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, - ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, + _scheduler ?? ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, success: r => new ExpressionEvalResult(r), failure: ex => new ExpressionEvalFailed(ex)); } @@ -685,7 +698,10 @@ public class AlarmActor : ReceiveActor // Per-script timeout from the on-trigger script (null = global). _onTriggerExecutionTimeoutSeconds, // The firing context's execution id (null today). - parentExecutionId)); + parentExecutionId, + // Scheduler seam (#18): share this alarm's scheduler override with the + // spawned on-trigger script body (null = process-wide shared). + _scheduler)); Context.ActorOf(props, executionId); } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs index f23fed30..2ad7d07e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs @@ -52,7 +52,10 @@ public class AlarmExecutionActor : ReceiveActor // alarm on-trigger script. Null or non-positive falls back to the global. int? executionTimeoutSeconds = null, // The firing context's execution id (null today). - Guid? parentExecutionId = null) + Guid? parentExecutionId = null, + // Script-execution scheduler seam (#18): the process-wide scheduler by + // default; null selects the shared default. + ScriptExecutionScheduler? scheduler = null) { var self = Self; var parent = Context.Parent; @@ -61,7 +64,7 @@ public class AlarmExecutionActor : ReceiveActor alarmName, instanceName, level, priority, message, compiledScript, instanceActor, sharedScriptLibrary, options, self, parent, logger, - executionTimeoutSeconds, parentExecutionId); + executionTimeoutSeconds, parentExecutionId, scheduler); } private static void ExecuteAlarmScript( @@ -78,7 +81,8 @@ public class AlarmExecutionActor : ReceiveActor IActorRef parent, ILogger logger, int? executionTimeoutSeconds, - Guid? parentExecutionId) + Guid? parentExecutionId, + ScriptExecutionScheduler? scheduler) { // Per-script timeout overrides the global default. A null or // non-positive per-script value (≤ 0) falls back to the global. @@ -88,8 +92,9 @@ public class AlarmExecutionActor : ReceiveActor : options.ScriptExecutionTimeoutSeconds); // Run the alarm on-trigger body on the dedicated - // script-execution scheduler, not the shared .NET thread pool. - var scheduler = ScriptExecutionScheduler.Shared(options); + // script-execution scheduler, not the shared .NET thread pool. An injected + // scheduler (#18) overrides the process-wide default. + var executionScheduler = scheduler ?? ScriptExecutionScheduler.Shared(options); _ = Task.Factory.StartNew(async () => { @@ -157,6 +162,6 @@ public class AlarmExecutionActor : ReceiveActor { self.Tell(PoisonPill.Instance); } - }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, scheduler).Unwrap(); + }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, executionScheduler).Unwrap(); } } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs index 65448f1f..cd7e4a98 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs @@ -40,6 +40,15 @@ public class ScriptActor : ReceiveActor, IWithTimers private readonly ISiteHealthCollector? _healthCollector; private readonly IServiceProvider? _serviceProvider; + /// + /// Script-execution scheduler seam (#18): the process-wide + /// when null, or an injected instance so + /// this actor's trigger-expression evaluation and spawned script bodies run on a + /// caller-owned pool instead of the shared one. Resolved lazily at each use so the + /// null (host) path stays byte-for-byte identical to the previous static call. + /// + private readonly ScriptExecutionScheduler? _scheduler; + private Script? _compiledScript; private ScriptTriggerConfig? _triggerConfig; private TimeSpan? _minTimeBetweenRuns; @@ -101,6 +110,7 @@ public class ScriptActor : ReceiveActor, IWithTimers /// Initial attribute snapshot used to seed expression trigger evaluation state. /// Optional health metrics collector. /// Optional DI service provider for script execution context services. + /// Optional script-execution scheduler override (#18); null uses the process-wide shared scheduler. public ScriptActor( string scriptName, string instanceName, @@ -113,7 +123,8 @@ public class ScriptActor : ReceiveActor, IWithTimers Script? compiledTriggerExpression = null, IReadOnlyDictionary? initialAttributes = null, ISiteHealthCollector? healthCollector = null, - IServiceProvider? serviceProvider = null) + IServiceProvider? serviceProvider = null, + ScriptExecutionScheduler? scheduler = null) { _scriptName = scriptName; _instanceName = instanceName; @@ -124,6 +135,7 @@ public class ScriptActor : ReceiveActor, IWithTimers _logger = logger; _healthCollector = healthCollector; _serviceProvider = serviceProvider; + _scheduler = scheduler; _minTimeBetweenRuns = scriptConfig.MinTimeBetweenRuns; _executionTimeoutSeconds = scriptConfig.ExecutionTimeoutSeconds; _scope = scriptConfig.Scope; @@ -313,7 +325,7 @@ public class ScriptActor : ReceiveActor, IWithTimers return false; } }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, - ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, + _scheduler ?? ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, success: r => new ExpressionEvalResult(r), failure: ex => new ExpressionEvalFailed(ex)); } @@ -480,7 +492,10 @@ public class ScriptActor : ReceiveActor, IWithTimers // an inbound-API-routed call supplies the inbound request's id. parentExecutionId, // Per-script timeout override (null = use global). - _executionTimeoutSeconds)); + _executionTimeoutSeconds, + // Scheduler seam (#18): thread this actor's scheduler override down so + // spawned script bodies share the same pool (null = process-wide shared). + _scheduler)); Context.ActorOf(props, executionId); } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs index 0f705995..c568a913 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs @@ -69,7 +69,12 @@ public class ScriptExecutionActor : ReceiveActor Guid? parentExecutionId = null, // Per-script execution timeout override (seconds). Null or // non-positive falls back to the global ScriptExecutionTimeoutSeconds. - int? executionTimeoutSeconds = null) + int? executionTimeoutSeconds = null, + // Script-execution scheduler seam (#18): the process-wide + // ScriptExecutionScheduler by default; a test (or a future multi-site + // host) can inject its own instance so script bodies never run on the + // shared process-wide pool. Null selects the shared default. + ScriptExecutionScheduler? scheduler = null) { // Immediately begin execution var self = Self; @@ -79,7 +84,7 @@ public class ScriptExecutionActor : ReceiveActor scriptName, instanceName, compiledScript, parameters, callDepth, instanceActor, sharedScriptLibrary, options, replyTo, correlationId, self, parent, logger, scope, healthCollector, serviceProvider, - parentExecutionId, executionTimeoutSeconds); + parentExecutionId, executionTimeoutSeconds, scheduler); } private static void ExecuteScript( @@ -100,7 +105,8 @@ public class ScriptExecutionActor : ReceiveActor ISiteHealthCollector? healthCollector, IServiceProvider? serviceProvider, Guid? parentExecutionId, - int? executionTimeoutSeconds) + int? executionTimeoutSeconds, + ScriptExecutionScheduler? scheduler) { // Per-script timeout overrides the global default. A null or // non-positive per-script value (≤ 0) falls back to the global. @@ -111,8 +117,9 @@ public class ScriptExecutionActor : ReceiveActor // Run the script body on the dedicated script-execution // scheduler, not the shared .NET thread pool, so blocking script I/O cannot - // starve the global pool and stall Akka dispatchers / HTTP handling. - var scheduler = ScriptExecutionScheduler.Shared(options); + // starve the global pool and stall Akka dispatchers / HTTP handling. An + // injected scheduler (#18) overrides the process-wide default. + var executionScheduler = scheduler ?? ScriptExecutionScheduler.Shared(options); // Notification Outbox: the site communication actor that Notify.Status queries // central through. Resolved by actor path so the Notify helper does not need an @@ -329,6 +336,6 @@ public class ScriptExecutionActor : ReceiveActor // Stop self after execution completes self.Tell(PoisonPill.Instance); } - }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, scheduler).Unwrap(); + }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, executionScheduler).Unwrap(); } } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs index 1cc102ba..8c66777a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs @@ -32,20 +32,30 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable private static readonly object SharedLock = new(); /// - /// The process-wide script-execution scheduler. Lazily created on first use with the - /// thread count from ; the - /// first caller wins, subsequent calls reuse the existing instance. + /// The process-wide script-execution scheduler, used as the default when no scheduler + /// is injected. Lazily created on first use with the thread count from + /// ; the first caller wins, + /// subsequent calls reuse the existing instance. + /// + /// If the cached instance has been disposed it is recreated rather than handed back: + /// a disposed scheduler can execute no work, so returning it would silently poison + /// every subsequent script and alarm execution for the rest of the process. Nothing + /// disposes the shared instance today, but the guard keeps that latent failure mode + /// closed regardless of a future shutdown path or a multi-site host. /// /// Site runtime options supplying the thread count for the scheduler. - /// The process-wide instance, creating it on first call. + /// A live process-wide instance, creating it on first call. public static ScriptExecutionScheduler Shared(SiteRuntimeOptions options) { - if (_shared != null) - return _shared; + var existing = _shared; + if (existing is { IsDisposed: false }) + return existing; lock (SharedLock) { - return _shared ??= new ScriptExecutionScheduler(options.ScriptExecutionThreadCount); + if (_shared is null || _shared.IsDisposed) + _shared = new ScriptExecutionScheduler(options.ScriptExecutionThreadCount); + return _shared; } } @@ -73,6 +83,10 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable } } + /// once has run; a disposed scheduler + /// can execute no further work and must not be handed out by . + public bool IsDisposed => Volatile.Read(ref _disposed) != 0; + /// public override int MaximumConcurrencyLevel => _threads.Count; diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs index 83dd9ab1..16cf2bd6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs @@ -24,21 +24,30 @@ public sealed class ScriptSchedulerStatsReporter : BackgroundService private readonly ILogger _logger; private readonly TimeSpan _pollInterval; + /// + /// Script-execution scheduler seam (#18): the process-wide scheduler when null, or + /// an injected instance so the reporter samples the same pool the actors run on. + /// + private readonly ScriptExecutionScheduler? _scheduler; + /// Initializes a new instance of . /// The site health collector that receives the scheduler gauges. /// Site runtime options supplying the shared scheduler's thread count. /// Logger instance. /// Poll interval override; defaults to (10 s). + /// Optional script-execution scheduler override (#18); null samples the process-wide shared scheduler. public ScriptSchedulerStatsReporter( ISiteHealthCollector collector, SiteRuntimeOptions options, ILogger logger, - TimeSpan? pollInterval = null) + TimeSpan? pollInterval = null, + ScriptExecutionScheduler? scheduler = null) { _collector = collector ?? throw new ArgumentNullException(nameof(collector)); _options = options ?? throw new ArgumentNullException(nameof(options)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _pollInterval = pollInterval ?? DefaultPollInterval; + _scheduler = scheduler; } /// @@ -67,7 +76,7 @@ public sealed class ScriptSchedulerStatsReporter : BackgroundService { try { - var scheduler = ScriptExecutionScheduler.Shared(_options); + var scheduler = _scheduler ?? ScriptExecutionScheduler.Shared(_options); _collector.SetScriptSchedulerStats( scheduler.QueueDepth, scheduler.BusyThreadCount, 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 1e1a722d..44bc846d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs @@ -26,6 +26,14 @@ public class ScriptActorTests : TestKit, IDisposable private readonly SiteRuntimeOptions _options; private readonly ScriptCompilationService _compilationService; + // Gitea #16/#18: every ScriptActor built here runs its trigger-expression + // evaluation and spawned script bodies on THIS class's own scheduler, never the + // process-wide singleton. That is what makes the faulted-task test below + // un-leakable across the assembly: any worker a test strands (or the 30 s bounded + // EvalGate wait leaves parked) lives on this instance, which is disposed at class + // teardown, so it can never starve unrelated test classes' script executions. + private readonly ScriptExecutionScheduler _scheduler; + public ScriptActorTests() { _compilationService = new ScriptCompilationService( @@ -37,11 +45,13 @@ public class ScriptActorTests : TestKit, IDisposable MaxScriptCallDepth = 10, ScriptExecutionTimeoutSeconds = 30 }; + _scheduler = new ScriptExecutionScheduler(_options.ScriptExecutionThreadCount); } void IDisposable.Dispose() { Shutdown(); + _scheduler.Dispose(); } private Script CompileScript(string code) @@ -74,7 +84,12 @@ public class ScriptActorTests : TestKit, IDisposable scriptConfig, _sharedLibrary, _options, - NullLogger.Instance))); + NullLogger.Instance, + null, // compiledTriggerExpression + null, // initialAttributes + null, // healthCollector + null, // serviceProvider + _scheduler))); // Ask pattern (WP-22) for CallScript scriptActor.Tell(new ScriptCallRequest("GetAnswer", null, 0, "corr-1")); @@ -103,7 +118,12 @@ public class ScriptActorTests : TestKit, IDisposable scriptConfig, _sharedLibrary, _options, - NullLogger.Instance))); + NullLogger.Instance, + null, // compiledTriggerExpression + null, // initialAttributes + null, // healthCollector + null, // serviceProvider + _scheduler))); var parameters = new Dictionary { ["x"] = 3, ["y"] = 4 }; scriptActor.Tell(new ScriptCallRequest("Add", parameters, 0, "corr-2")); @@ -131,7 +151,12 @@ public class ScriptActorTests : TestKit, IDisposable scriptConfig, _sharedLibrary, _options, - NullLogger.Instance))); + NullLogger.Instance, + null, // compiledTriggerExpression + null, // initialAttributes + null, // healthCollector + null, // serviceProvider + _scheduler))); scriptActor.Tell(new ScriptCallRequest("Broken", null, 0, "corr-3")); @@ -161,7 +186,12 @@ public class ScriptActorTests : TestKit, IDisposable scriptConfig, _sharedLibrary, _options, - NullLogger.Instance))); + NullLogger.Instance, + null, // compiledTriggerExpression + null, // initialAttributes + null, // healthCollector + null, // serviceProvider + _scheduler))); // Send an attribute change that matches the trigger scriptActor.Tell(new AttributeValueChanged( @@ -194,7 +224,12 @@ public class ScriptActorTests : TestKit, IDisposable scriptConfig, _sharedLibrary, _options, - NullLogger.Instance))); + NullLogger.Instance, + null, // compiledTriggerExpression + null, // initialAttributes + null, // healthCollector + null, // serviceProvider + _scheduler))); // First trigger -- should execute scriptActor.Tell(new AttributeValueChanged( @@ -228,7 +263,12 @@ public class ScriptActorTests : TestKit, IDisposable scriptConfig, _sharedLibrary, _options, - NullLogger.Instance))); + NullLogger.Instance, + null, // compiledTriggerExpression + null, // initialAttributes + null, // healthCollector + null, // serviceProvider + _scheduler))); // First call -- fails scriptActor.Tell(new ScriptCallRequest("Failing", null, 0, "corr-fail-1")); @@ -293,7 +333,8 @@ public class ScriptActorTests : TestKit, IDisposable triggerExpression, null, null, - null))); + null, + _scheduler))); return (actor, instance); } @@ -355,6 +396,7 @@ public class ScriptActorTests : TestKit, IDisposable [Fact] public void ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation() { + EvalGate.Reset(); // fresh gate + zeroed counter; never inherit a prior run's state var expr = CompileRawTriggerExpression( "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.EvalGate.Block()"); var (actor, _) = CreateTriggeredActor( @@ -363,6 +405,12 @@ public class ScriptActorTests : TestKit, IDisposable { actor.Tell(Change("A", "1")); // eval starts on the scheduler and BLOCKS → _evalInFlight = true AwaitAssert(() => Assert.Equal(1, EvalGate.Entries), TimeSpan.FromSeconds(10)); + + // #18 seam: the blocked evaluation is running on THIS class's injected + // scheduler — not the process-wide singleton — so a worker it strands can + // never starve another test class. + Assert.Equal(1, _scheduler.BusyThreadCount); + actor.Tell(Change("A", "2")); // coalesces → _evalPending = true // Simulate the faulted scheduler task the PipeTo failure mapping now surfaces @@ -374,7 +422,12 @@ public class ScriptActorTests : TestKit, IDisposable } finally { - EvalGate.Gate.Release(EvalGate.Entries); // ALWAYS free the shared scheduler threads + // Release generously (one per worker) so every parked/queued Block returns + // at once and this class's scheduler drains promptly at teardown. The old + // Release(Entries) under-released in exactly the failure case (Entries read + // as 1 while a second eval was still queued), stranding a worker; the + // bounded 30 s wait in Block is the ultimate backstop. + EvalGate.Gate.Release(_options.ScriptExecutionThreadCount); } } @@ -541,8 +594,38 @@ public class ScriptActorTests : TestKit, IDisposable /// public static class EvalGate { - public static readonly SemaphoreSlim Gate = new(0); + private static SemaphoreSlim _gate = new(0); private static int _entries; + + /// The semaphore that script-scheduler threads park on inside . + public static SemaphoreSlim Gate => _gate; + + /// Number of evaluations that have entered since the last . public static int Entries => Volatile.Read(ref _entries); - public static bool Block() { Interlocked.Increment(ref _entries); Gate.Wait(); return false; } + + /// + /// Resets the gate to a fresh, permit-free state and zeroes the entry counter. + /// Called at the start of the test so counts never carry across runs — the old + /// static leaked _entries from whatever compiled an expression before it + /// (Gitea #16). + /// + public static void Reset() + { + _gate = new SemaphoreSlim(0); + Volatile.Write(ref _entries, 0); + } + + /// + /// Counts the entry, then blocks the calling script-scheduler thread until a permit + /// is released. The wait is BOUNDED (30 s): even if a test releases too few permits, + /// a stranded evaluation frees its worker instead of parking it for the rest of the + /// process — the leak mechanism behind Gitea #16. Returns false so a trigger + /// expression built on it evaluates to "not fired". + /// + public static bool Block() + { + Interlocked.Increment(ref _entries); + _gate.Wait(TimeSpan.FromSeconds(30)); + return false; + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptExecutionSchedulerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptExecutionSchedulerTests.cs index 86ad91f0..620a9c0e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptExecutionSchedulerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptExecutionSchedulerTests.cs @@ -45,6 +45,24 @@ public class ScriptExecutionSchedulerTests Assert.Same(a, b); } + // Gitea #18: IsDisposed backs the Shared() recreate guard so a disposed scheduler + // can never be handed back and silently poison every later execution. Verified on a + // local instance — deliberately NOT by disposing the process-wide singleton, which + // would strand any script another test class queued on it (the very static hazard + // #18 is about). + [Fact] + public void IsDisposed_FlipsOnDispose() + { + var scheduler = new ScriptExecutionScheduler(1); + Assert.False(scheduler.IsDisposed); + + scheduler.Dispose(); + Assert.True(scheduler.IsDisposed); + + scheduler.Dispose(); // idempotent — stays disposed, does not throw + Assert.True(scheduler.IsDisposed); + } + // SiteRuntime-S2/UA5: the scheduler exposes observability gauges so a stuck / // saturated script-execution pool is visible to operators via site health. [Fact]