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
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -188,6 +188,15 @@ public sealed class ScriptedAlarmEngine : IDisposable
|
||||
/// (<c>DrainDirtyAsync</c>, <c>ShelvingCheckAsync</c>) runs on its token, so a store call that honours
|
||||
/// cancellation returns instead of outliving the engine.</summary>
|
||||
private readonly CancellationTokenSource _shutdownCts = new();
|
||||
|
||||
/// <summary><see cref="_shutdownCts"/>'s token, captured once: background work reads this, never the CTS,
|
||||
/// so <see cref="Dispose"/> can dispose the CTS without racing a late reader.</summary>
|
||||
private readonly CancellationToken _shutdownToken;
|
||||
|
||||
private volatile bool _shutdownCtsDisposed;
|
||||
|
||||
/// <summary>Test hook — whether <see cref="Dispose"/> disposed <see cref="_shutdownCts"/>.</summary>
|
||||
internal bool ShutdownCtsDisposedForTest => _shutdownCtsDisposed;
|
||||
private readonly Dictionary<string, HashSet<string>> _alarmsReferencing
|
||||
= new(StringComparer.Ordinal); // tag path -> alarm ids
|
||||
|
||||
@@ -247,6 +256,7 @@ public sealed class ScriptedAlarmEngine : IDisposable
|
||||
_clock = clock ?? (() => DateTime.UtcNow);
|
||||
_scriptTimeout = scriptTimeout ?? TimedScriptEvaluator<AlarmPredicateContext, bool>.DefaultTimeout;
|
||||
_guard = new ScriptRunGuard(scripting, _engineLogger);
|
||||
_shutdownToken = _shutdownCts.Token;
|
||||
}
|
||||
|
||||
/// <summary>The run guard every predicate evaluation goes through (diagnostics / tests).</summary>
|
||||
@@ -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));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -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));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -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-
|
||||
|
||||
@@ -43,6 +43,9 @@ public sealed class ScriptLogRateLimiter
|
||||
_clock = clock ?? (static () => DateTime.UtcNow);
|
||||
}
|
||||
|
||||
/// <summary>Entries admitted per source per window, as configured (after clamping to at least 1).</summary>
|
||||
public int LimitPerWindow => _limit;
|
||||
|
||||
/// <summary>The window length.</summary>
|
||||
public TimeSpan Window => _window;
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user