test(scripting): timeout under starvation is not a leak; share the exhaustion-reason constant

Review follow-ups to 34829ea0.

IMPORTANT - the leak filter had its own flake vector. This test saturates the
thread pool by design, and TimedScriptEvaluator starts its 5s clock at WaitAsync
rather than when the inner Task.Run gets a thread, so a legitimately-enqueued
evaluation can blow the budget while still queued and surface "script timed out
after 5.0s". The reason != exhaustion filter classified that as a raw disposed
escape and hard-failed on it - handing the test back the load-fragility the
reshape removed. Timeouts are now their own bucket, tolerated and rate-bounded
exactly as exhaustions are; a raw disposed escape still fails on first occurrence.
Doc-comment names this second residual vector alongside the exhaustion calibration.

MINOR (scope extension authorised by review) - "evaluator disposed during
evaluation" existed as three unshared literals. Promoted to
ScriptEvalFailureReasons alongside VirtualTagEvalResult in Commons.Engines (the
assembly all three already reference) and referenced from both evaluators and the
test. The timeout prefix joins it, since the test now classifies on it too. String
values are byte-identical - no behaviour change.

Falsified: with a 12ms script budget the race produced real timeouts (3/2695).
Classified as leaks (the pre-review filter) they hard-failed the test 6/6 items;
bucketed as starvation they sit 0.11% under the 10% bound and the test is green.
This commit is contained in:
Joseph Doherty
2026-08-18 08:05:51 -04:00
parent 3f04a4d4e5
commit 837d8a17eb
4 changed files with 86 additions and 31 deletions
@@ -37,6 +37,36 @@ public sealed record VirtualTagEvalResult(bool Success, object? Value, string? R
public static VirtualTagEvalResult Failure(string reason) => new(false, null, reason);
}
/// <summary>
/// Failure reasons the script evaluators put on <see cref="VirtualTagEvalResult.Failure"/> and that
/// callers / tests classify on. The two evaluators — <c>RoslynVirtualTagEvaluator</c> (virtual tags)
/// and <c>CalculationEvaluator</c> (calc tags) — are deliberate mirrors of one another, so these
/// strings are a shared contract rather than two coincidentally-equal literals. They lived as three
/// unshared copies (the two evaluators plus a test const); a reader could not tell the contracts were
/// meant to be identical, and drift on one side would silently break a classifier keyed on the other.
/// Only the reasons something outside the evaluator matches on live here — the purely descriptive ones
/// (<c>compile error:</c>, <c>sandbox violation:</c>, <c>script threw:</c>) stay inline.
/// </summary>
public static class ScriptEvalFailureReasons
{
/// <summary>
/// The evaluator's one-shot retry was exhausted: a SECOND concurrent apply-boundary
/// <c>ClearCompiledScripts</c> disposed the compiled script inside the same evaluation. This is the
/// documented, well-formed outcome of losing that race twice — distinct from an
/// <see cref="ObjectDisposedException"/> escaping to the generic catch as <c>"script threw: …"</c>,
/// which is a bug.
/// </summary>
public const string EvaluatorDisposedDuringEvaluation = "evaluator disposed during evaluation";
/// <summary>
/// Prefix of the wall-clock-timeout failure; the evaluator completes it with the budget it applied
/// (<c>"{seconds:F1}s"</c>). Note the timeout covers the enqueue too — <c>TimedScriptEvaluator</c>
/// starts its clock at <c>WaitAsync</c>, not when the inner <c>Task.Run</c> gets a thread — so on a
/// starved pool this reason can mean "never got to run", not "ran too long".
/// </summary>
public const string TimedOutPrefix = "script timed out after ";
}
/// <summary>Returns <see cref="VirtualTagEvalResult.NoChange"/> from every call. Bound by default
/// when the production <c>VirtualTagEngine</c> adapter hasn't been registered (Mac dev, tests).</summary>
public sealed class NullVirtualTagEvaluator : IVirtualTagEvaluator
@@ -148,7 +148,8 @@ public sealed class CalculationEvaluator : IScriptCacheOwner, IDisposable
}
catch (ScriptTimeoutException)
{
return VirtualTagEvalResult.Failure($"script timed out after {_runTimeout.TotalSeconds:F1}s");
return VirtualTagEvalResult.Failure(
$"{ScriptEvalFailureReasons.TimedOutPrefix}{_runTimeout.TotalSeconds:F1}s");
}
catch (ObjectDisposedException) when (!_disposed && attempt == 0)
{
@@ -159,7 +160,7 @@ public sealed class CalculationEvaluator : IScriptCacheOwner, IDisposable
}
catch (ObjectDisposedException)
{
return VirtualTagEvalResult.Failure("evaluator disposed during evaluation");
return VirtualTagEvalResult.Failure(ScriptEvalFailureReasons.EvaluatorDisposedDuringEvaluation);
}
catch (Exception ex)
{
@@ -136,7 +136,8 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac
}
catch (ScriptTimeoutException)
{
return VirtualTagEvalResult.Failure($"script timed out after {_runTimeout.TotalSeconds:F1}s");
return VirtualTagEvalResult.Failure(
$"{ScriptEvalFailureReasons.TimedOutPrefix}{_runTimeout.TotalSeconds:F1}s");
}
catch (ObjectDisposedException) when (!_disposed && attempt == 0)
{
@@ -148,7 +149,7 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac
}
catch (ObjectDisposedException)
{
return VirtualTagEvalResult.Failure("evaluator disposed during evaluation");
return VirtualTagEvalResult.Failure(ScriptEvalFailureReasons.EvaluatorDisposedDuringEvaluation);
}
catch (Exception ex)
{