dohertj2/lmxopcua
1
0
Fork 0
Code Issues 36 Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat(scripting): ScriptRunGuard quarantines runaway scripts across every seam (E1)

Browse Source 
A timed-out script is abandoned, not stopped, and every seam re-ran it on the
next trigger, leaking one pool thread per trigger (R03:1.1, R05:2, R07:L1).
A shared ScriptRunGuard, keyed per script instance, now:

- refuses a run while that script's previous orphan is still executing
  (Bad "previous evaluation still running" + otopcua.script.run_refused);
- quarantines after 3 consecutive non-completions or an allocation-budget
  breach (sampled with GC.GetAllocatedBytesForCurrentThread; default 64 MB):
  no further dispatch, Bad quality, one Error, otopcua.script.quarantined,
  cleared on source change / redeploy / reset;
- starts the budget clock when the pool picks the run up, so starvation is
  no longer charged to the script (R07:P5), and cancels the evaluator token
  on timeout.

Wired into RoslynVirtualTagEvaluator, CalculationEvaluator (new async
EvaluateAsync; the driver's pump and timer ticks now await the run behind a
SemaphoreSlim gate) and ScriptedAlarmEngine (a non-completing predicate
holds state and annotates the condition BadInternalError). ScriptingOptions
(Scripting:MaxAllocBytesPerRun, Scripting:QuarantineAfterConsecutiveTimeouts)
carries the knobs; the Calculation driver config accepts the same two keys.

Tests: guard unit suite (incl. the 30-trigger runaway leaving one orphan with
a healthy sibling as positive control), per-seam refusal/quarantine/reset
tests. The S12 race test now uses one vtag id per loop: twelve concurrent
evaluations of ONE id is a shape production never produces and the guard
refuses by design.

Refs #734

Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 06:39:43 -04:00
parent ad2b5c4044
commit eb9269c337
17 changed files with 1313 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs
+74 -8
View File
@@ -73,6 +73,14 @@ public sealed class ScriptedAlarmEngine : IDisposable
private readonly Func<DateTime> _clock;
private readonly TimeSpan _scriptTimeout;
/// <summary>
/// Per-alarm run quarantine (keyed by alarm id, source identity = predicate source). Refuses a
/// predicate run while the alarm's previous (timed-out, abandoned) run is still executing and
/// quarantines after three consecutive non-completions or an allocation-budget breach — one orphan
/// per alarm instead of one per upstream push. Reset on every <see cref="LoadAsync"/> (a redeploy).
/// </summary>
private readonly ScriptRunGuard _guard;
// ConcurrentDictionary, not a plain Dictionary: every mutation happens under
// _evalGate, but four read paths (GetState, GetAllStates, LoadedAlarmIds,
// RunShelvingCheck) touch _alarms from arbitrary threads (Admin UI request
@@ -204,13 +212,15 @@ public sealed class ScriptedAlarmEngine : IDisposable
/// <param name="engineLogger">The logger for engine-level diagnostics.</param>
/// <param name="clock">Optional function providing the current UTC time; defaults to DateTime.UtcNow.</param>
/// <param name="scriptTimeout">Optional timeout for script execution; defaults to the evaluator's default timeout.</param>
/// <param name="scripting">Run-guard budget + quarantine threshold; null ⇒ <see cref="ScriptingOptions"/> defaults.</param>
public ScriptedAlarmEngine(
ITagUpstreamSource upstream,
IAlarmStateStore store,
ScriptLoggerFactory loggerFactory,
ILogger engineLogger,
Func<DateTime>? clock = null,
TimeSpan? scriptTimeout = null)
TimeSpan? scriptTimeout = null,
ScriptingOptions? scripting = null)
{
_upstream = upstream ?? throw new ArgumentNullException(nameof(upstream));
_store = store ?? throw new ArgumentNullException(nameof(store));
@@ -218,8 +228,20 @@ public sealed class ScriptedAlarmEngine : IDisposable
_engineLogger = engineLogger ?? throw new ArgumentNullException(nameof(engineLogger));
_clock = clock ?? (() => DateTime.UtcNow);
_scriptTimeout = scriptTimeout ?? TimedScriptEvaluator<AlarmPredicateContext, bool>.DefaultTimeout;
_guard = new ScriptRunGuard(scripting, _engineLogger);
}
/// <summary>The run guard every predicate evaluation goes through (diagnostics / tests).</summary>
public ScriptRunGuard RunGuard => _guard;
/// <summary>
/// OPC UA <c>BadInternalError</c> — the quality a condition is annotated with while its predicate
/// cannot be evaluated (timed out, refused because the previous run is still executing, or
/// quarantined). The condition's state is frozen; the quality says why it cannot be trusted. The
/// next completed evaluation restores the input-derived quality through the normal bucket tracking.
/// </summary>
private const uint StatusBadInternalError = 0x80020000u;
/// <summary>Raised for every emission the Part9StateMachine produces that the engine should publish.</summary>
public event EventHandler<ScriptedAlarmEvent>? OnEvent;
@@ -268,6 +290,9 @@ public sealed class ScriptedAlarmEngine : IDisposable
// recompile this one. Skipping this is what made the earlier fix a
// no-op in production.
_compileCache.Clear();
// A (re)load is a deploy: lift every run-guard quarantine (an edited predicate already lifts
// its own through the guard's source identity; this covers "redeployed to retry").
_guard.ResetAll();
var compileFailures = new List<string>();
foreach (var def in definitions)
@@ -284,15 +309,17 @@ public sealed class ScriptedAlarmEngine : IDisposable
// Route through CompiledScriptCache so the emitted assembly's
// collectible ALC participates in publish-replace cleanup.
// The compiled script is run through _guard (timeout + quarantine) at evaluation time —
// the per-alarm TimedScriptEvaluator this used to wrap gave the timeout but could not
// see that the run it abandoned was still executing.
var evaluator = _compileCache.GetOrCompile(def.PredicateScriptSource);
var timed = new TimedScriptEvaluator<AlarmPredicateContext, bool>(evaluator, _scriptTimeout);
var logger = _loggerFactory.Create(def.AlarmId);
var templateTokens = MessageTemplate.ExtractTokenPaths(def.MessageTemplate);
var allInputs = new HashSet<string>(extraction.Reads, StringComparer.Ordinal);
foreach (var t in templateTokens) allInputs.Add(t);
_alarms[def.AlarmId] = new AlarmState(def, timed, extraction.Reads, templateTokens, logger,
_alarms[def.AlarmId] = new AlarmState(def, evaluator, extraction.Reads, templateTokens, logger,
AlarmConditionState.Fresh(def.AlarmId, _clock()));
foreach (var path in allInputs)
@@ -767,22 +794,35 @@ public sealed class ScriptedAlarmEngine : IDisposable
bool predicateTrue;
try
{
predicateTrue = await state.Evaluator.RunAsync(context, ct).ConfigureAwait(false);
predicateTrue = await _guard.RunAsync(
state.Definition.AlarmId, state.Definition.PredicateScriptSource, state.Evaluator, context,
_scriptTimeout, ct)
.ConfigureAwait(false);
}
catch (OperationCanceledException)
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
throw;
}
catch (ScriptTimeoutException tex)
{
state.Logger.Warning("Alarm predicate timed out after {Timeout} — state unchanged", tex.Timeout);
return seed;
return PredicateDidNotComplete(state, seed, scratch,
$"timed out after {tex.Timeout.TotalMilliseconds:F0} ms", pendingEmissions);
}
catch (ScriptRunRefusedException ex)
{
return PredicateDidNotComplete(state, seed, scratch, ex.Message, pendingEmissions);
}
catch (ScriptQuarantinedException ex)
{
return PredicateDidNotComplete(state, seed, scratch, ex.Message, pendingEmissions);
}
catch (Exception ex)
{
scratch.LastNonCompletion = null;
state.Logger.Error(ex, "Alarm predicate threw — state unchanged");
return seed;
}
scratch.LastNonCompletion = null;
var result = Part9StateMachine.ApplyPredicate(seed, predicateTrue, nowUtc);
var transition = result.Emission != EmissionKind.None
@@ -802,6 +842,28 @@ public sealed class ScriptedAlarmEngine : IDisposable
return result.State;
}
/// <summary>
/// The predicate did not return (timed out, refused because the previous run is still executing, or
/// quarantined): hold the condition at <paramref name="seed"/>, log the reason <b>once per change of
/// reason</b> (a quarantined alarm is re-attempted on every push and would otherwise log every time),
/// and annotate the condition <see cref="StatusBadInternalError"/> out of band — the frozen state
/// cannot be trusted and the quality is how the operator sees that. The next completed evaluation
/// restores the input-derived quality through the normal bucket tracking.
/// </summary>
private AlarmConditionState PredicateDidNotComplete(
AlarmState state, AlarmConditionState seed, AlarmScratch scratch, string reason,
List<ScriptedAlarmEvent> pendingEmissions)
{
if (!string.Equals(scratch.LastNonCompletion, reason, StringComparison.Ordinal))
{
scratch.LastNonCompletion = reason;
state.Logger.Warning("Alarm predicate did not complete ({Reason}) — state unchanged, quality Bad", reason);
}
if (TrackQualityBucket(state.Definition.AlarmId, StatusBadInternalError))
pendingEmissions.Add(BuildQualityEmission(state, seed, StatusBadInternalError));
return seed;
}
/// <summary>
/// Refill <paramref name="cache"/> in place from <c>_valueCache</c>, falling
/// back to a synchronous <c>ITagUpstreamSource.ReadTag</c> for paths whose
@@ -1201,7 +1263,7 @@ public sealed class ScriptedAlarmEngine : IDisposable
private sealed record AlarmState(
ScriptedAlarmDefinition Definition,
TimedScriptEvaluator<AlarmPredicateContext, bool> Evaluator,
ScriptEvaluator<AlarmPredicateContext, bool> Evaluator,
IReadOnlySet<string> Inputs,
IReadOnlyList<string> TemplateTokens,
ILogger Logger,
@@ -1227,6 +1289,10 @@ public sealed class ScriptedAlarmEngine : IDisposable
/// <summary>Gets the predicate evaluation context.</summary>
public AlarmPredicateContext Context { get; }
/// <summary>The reason the last evaluation did not complete (timeout / refused / quarantined), or null
/// after a completed run — the once-per-change log gate for <c>PredicateDidNotComplete</c>.</summary>
public string? LastNonCompletion { get; set; }
/// <summary>
/// Initializes a new AlarmScratch with the specified inputs, logger, and clock.
/// </summary>