fix(siteruntime): injectable ScriptExecutionScheduler + un-leakable expression-fault test
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
This commit was merged in pull request #21.
This commit is contained in:
Joseph Doherty
@@ -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<object?> CompileScript(string code)
|
||||
@@ -74,7 +84,12 @@ public class ScriptActorTests : TestKit, IDisposable
|
||||
scriptConfig,
|
||||
_sharedLibrary,
|
||||
_options,
|
||||
NullLogger<ScriptActor>.Instance)));
|
||||
NullLogger<ScriptActor>.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<ScriptActor>.Instance)));
|
||||
NullLogger<ScriptActor>.Instance,
|
||||
null, // compiledTriggerExpression
|
||||
null, // initialAttributes
|
||||
null, // healthCollector
|
||||
null, // serviceProvider
|
||||
_scheduler)));
|
||||
|
||||
var parameters = new Dictionary<string, object?> { ["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<ScriptActor>.Instance)));
|
||||
NullLogger<ScriptActor>.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<ScriptActor>.Instance)));
|
||||
NullLogger<ScriptActor>.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<ScriptActor>.Instance)));
|
||||
NullLogger<ScriptActor>.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<ScriptActor>.Instance)));
|
||||
NullLogger<ScriptActor>.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
|
||||
/// </summary>
|
||||
public static class EvalGate
|
||||
{
|
||||
public static readonly SemaphoreSlim Gate = new(0);
|
||||
private static SemaphoreSlim _gate = new(0);
|
||||
private static int _entries;
|
||||
|
||||
/// <summary>The semaphore that script-scheduler threads park on inside <see cref="Block"/>.</summary>
|
||||
public static SemaphoreSlim Gate => _gate;
|
||||
|
||||
/// <summary>Number of evaluations that have entered <see cref="Block"/> since the last <see cref="Reset"/>.</summary>
|
||||
public static int Entries => Volatile.Read(ref _entries);
|
||||
public static bool Block() { Interlocked.Increment(ref _entries); Gate.Wait(); return false; }
|
||||
|
||||
/// <summary>
|
||||
/// 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 <c>_entries</c> from whatever compiled an expression before it
|
||||
/// (Gitea #16).
|
||||
/// </summary>
|
||||
public static void Reset()
|
||||
{
|
||||
_gate = new SemaphoreSlim(0);
|
||||
Volatile.Write(ref _entries, 0);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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".
|
||||
/// </summary>
|
||||
public static bool Block()
|
||||
{
|
||||
Interlocked.Increment(ref _entries);
|
||||
_gate.Wait(TimeSpan.FromSeconds(30));
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user