diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs
index d4edc62d..df2bf96c 100644
--- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs
@@ -380,15 +380,74 @@ public sealed class RoslynVirtualTagEvaluatorTests
CompiledCacheCount(sut).ShouldBe(1);
}
- /// 02/S12 concurrent stress repro (RED on current code): while one loop runs many compiled
- /// (non-passthrough) evaluations of the same expression, a second loop hammers
- /// — the apply-boundary clear. On the
- /// unfixed tree a clear landing between a child's GetOrCompile and the run's disposed-guard
- /// surfaces an ObjectDisposedException as a Failure("script threw: Cannot access a disposed
- /// object…"); the fix retries once and every result stays Success. The whole test is WaitAsync-
- /// bounded so a regression fails fast rather than hanging.
+ /// The one reason string the evaluator is documented to surface when its one-shot retry
+ /// is exhausted (a SECOND apply-boundary clear landing inside the same evaluation). Anything else
+ /// carrying "disposed" — notably the pre-fix generic escape "script threw: Cannot access a
+ /// disposed object…" — is a raw ObjectDisposedException leaking past the retry, i.e. the bug.
+ private const string RetryExhaustedReason = "evaluator disposed during evaluation";
+
+ /// How long the racing loops below keep hammering the evaluator. A DURATION rather than a
+ /// fixed iteration count: every clear forces a recompile, so a fixed workload's wall-clock cost tracks
+ /// whatever CPU the box has left, and a busy box then blows the test's own hang budget.
+ private static readonly TimeSpan RaceWindow = TimeSpan.FromSeconds(10);
+
+ /// Upper bound on evaluations per racing loop, so an idle box does bounded work rather than
+ /// spinning for the whole .
+ private const int MaxIterationsPerLoop = 400;
+
+ /// 02/S12 concurrent stress: while many loops run compiled (non-passthrough) evaluations of
+ /// the same expression, a second loop hammers
+ /// — the apply-boundary clear. A clear landing between a child's GetOrCompile and the run's
+ /// disposed-guard raises an inside the evaluation; on the unfixed
+ /// tree that escaped as Failure("script threw: Cannot access a disposed object…"), and the fix
+ /// re-fetches into the current cache generation and retries ONCE. The whole test is WaitAsync-bounded so
+ /// a regression fails fast rather than hanging.
+ ///
+ /// Shape history — why this is not a zero-failure assertion (renamed 2026-08-18 from
+ /// Evaluate_racing_ClearCompiledScripts_never_fails_with_disposed). The test previously
+ /// asserted failures.ShouldBeEmpty(), which silently encoded a TIMING assumption: that at most
+ /// ONE clear can land inside a single evaluation, so the product's one-shot retry always suffices. The
+ /// clear loop's spin cadence was tuned to hold that assumption. It does not hold under full-solution
+ /// parallel test load — with the thread pool saturated the fetch→run window stretches, TWO clears land
+ /// inside one evaluation, and the retry legitimately exhausts. The test failed under load while passing
+ /// 3/3 in isolation. That is a test-design defect, not a product defect: exhausting a one-shot retry
+ /// under a clear cadence no production apply comes close to is a documented, bounded outcome.
+ ///
+ /// The fix could have re-paced the clears against observed evaluation latency, but that only
+ /// swaps one timing assumption for a safer one — it stays load-sensitive, and needs machinery to
+ /// measure latency at runtime. Instead the assertion now targets the invariant the retry actually
+ /// guarantees, which is timing-independent: no raw disposed-script failure ever ESCAPES the
+ /// retry. Every failure this race produces must be the documented retry-exhaustion outcome
+ /// () and nothing else — a "script threw: Cannot access a disposed
+ /// object…" (the generic-catch escape) fails the test on the first occurrence, however loaded the box
+ /// is. A loose upper bound on the exhaustion RATE keeps the assertion non-vacuous: a build whose retry
+ /// never absorbs anything (every raced evaluation exhausting straight to the documented reason) would
+ /// pass a shape-only check, so exhaustions must stay a small minority of all evaluations.
+ ///
+ /// The workload is duration-bounded, and that is the SECOND timing assumption removed. The
+ /// loops used to run a fixed 12 × 400 = 4800 evaluations against a fixed 45 s WaitAsync budget.
+ /// Every clear forces the next evaluation to recompile (5-20 ms), so the wall-clock cost of that fixed
+ /// workload is set by how much CPU the box will spare — and running four instances of this test class
+ /// concurrently on the dev box reproduced a hard failure that has nothing to do with the race:
+ /// System.TimeoutException at the 45 s budget, 4/4 runs. That is almost certainly the shape the
+ /// original "fails under full-solution parallel test load" report saw. The loops now stop at
+ /// OR , whichever comes first, so a starved
+ /// box simply lands fewer evaluations instead of blowing a budget; the 45 s WaitAsync is left in
+ /// place purely as a hang detector, with 4× headroom over the window it now guards.
+ ///
+ /// Calibration of the exhaustion bound (be sceptical of it). The disposed-race did NOT
+ /// reproduce at all on the arm64 macOS dev box this reshape was written on — zero failures in isolation,
+ /// zero under concurrently-running Runtime.Tests / Host.Tests / OpcUaServer suites, and still zero with
+ /// the product's one-shot retry experimentally disabled, i.e. the fetch→run window never opened there
+ /// and this test was vacuous on that hardware whatever it asserted. So the bound could not be fitted to
+ /// a measured rate; it is 10% of the evaluations actually performed, hundreds of times looser than the
+ /// old fire-on-the-first-failure assertion. Exhaustion needs a clear inside BOTH the first attempt's
+ /// window and the retry's, so its rate goes as the square of the single-window hit rate — 10% of
+ /// evaluations exhausting implies roughly a third of them hitting a disposed evaluator on the first
+ /// try, which is a broken retry, not a busy machine. If a loaded box ever does trip THIS bound, the
+ /// rate itself is the finding: record it before widening the number.
[Fact]
- public async Task Evaluate_racing_ClearCompiledScripts_never_fails_with_disposed()
+ public async Task Evaluate_racing_ClearCompiledScripts_never_leaks_a_raw_disposed_failure()
{
// Generous timeout so the wall-clock budget never fires and confounds the race assertion.
using var sut = new RoslynVirtualTagEvaluator(
@@ -396,29 +455,31 @@ public sealed class RoslynVirtualTagEvaluatorTests
const string expr = "return (int)ctx.GetTag(\"a\").Value + 1;";
const int loopCount = 12;
- const int iterations = 400;
var failures = new System.Collections.Concurrent.ConcurrentBag();
- var remaining = loopCount; // number of concurrent evaluation loops still running
+ var evaluated = 0; // evaluations actually performed (the race window may cut loops short)
+ var remaining = loopCount; // number of concurrent evaluation loops still running
+ var stopAtMs = Environment.TickCount64 + (long)RaceWindow.TotalMilliseconds;
// Many concurrent evaluation loops put the thread pool under enough pressure that the inner
// Task.Run hop inside TimedScriptEvaluator is measurably delayed — widening the
- // fetch→run-disposed-guard window so a clear reliably lands inside SOME in-flight evaluation.
- // The clear loop clears on a short spin cadence (not back-to-back) so (a) most evaluations hit
- // the warm cache — keeping compile cost bounded — and (b) at most one clear lands within a
- // single evaluation, so the single retry the fix adds is sufficient (a back-to-back clear loop
- // would both compile-swamp the RED run into a timeout and defeat the one-shot retry). Pre-fix a
- // clear disposing the just-fetched evaluator surfaces as Failure("script threw: Cannot access a
- // disposed object…"); post-fix the retry re-fetches into the current generation → all Success.
+ // fetch→run-disposed-guard window so a clear can land inside an in-flight evaluation.
+ // The clear loop clears on a short spin cadence rather than back-to-back purely as a COST bound:
+ // most evaluations then hit the warm cache instead of recompiling. Neither the cadence nor the
+ // iteration count is a correctness knob any more — the assertions below hold for any number of
+ // clears landing inside a single evaluation, and for any number of evaluations completed.
var evalLoops = Enumerable.Range(0, loopCount).Select(_ => Task.Run(() =>
{
- for (var i = 0; i < iterations; i++)
+ var done = 0;
+ for (var i = 0; i < MaxIterationsPerLoop && Environment.TickCount64 < stopAtMs; i++)
{
var r = sut.Evaluate("vt-race", expr, new Dictionary { ["a"] = i });
+ done++;
if (!r.Success)
{
failures.Add(r.Reason ?? "");
}
}
+ Interlocked.Add(ref evaluated, done);
Interlocked.Decrement(ref remaining);
}, TestContext.Current.CancellationToken)).ToArray();
@@ -431,11 +492,28 @@ public sealed class RoslynVirtualTagEvaluatorTests
}
}, TestContext.Current.CancellationToken);
+ // Hang detector only: the loops self-terminate at RaceWindow, so firing this means something
+ // wedged, not that the box was slow.
await Task.WhenAll(evalLoops.Append(clearLoop))
.WaitAsync(TimeSpan.FromSeconds(45), TestContext.Current.CancellationToken);
- failures.ShouldBeEmpty(
- $"every evaluation must succeed despite racing clears; observed: {string.Join(" | ", failures)}");
+ // (1) The invariant: nothing but the documented retry-exhaustion outcome may surface. A raw
+ // ObjectDisposedException escaping the retry (pre-fix: "script threw: Cannot access a
+ // disposed object…") lands here, as does any other unexpected failure mode.
+ var leaked = failures.Where(reason => reason != RetryExhaustedReason).ToArray();
+ leaked.ShouldBeEmpty(
+ $"a racing ClearCompiledScripts must never surface anything but \"{RetryExhaustedReason}\"; " +
+ $"observed: {string.Join(" | ", leaked)}");
+
+ // (2) Non-vacuity: the retry must actually be absorbing the race, not merely relabelling it.
+ // Loose by design (see the doc-comment) — a broken-retry detector, not a timing gate. The
+ // absolute floor keeps a starved run (few evaluations landed) from tripping on a single one.
+ var exhausted = failures.Count;
+ var totalEvaluations = Volatile.Read(ref evaluated);
+ exhausted.ShouldBeLessThan(
+ Math.Max(totalEvaluations / 10, 8),
+ $"the one-shot retry must absorb the overwhelming majority of raced clears; " +
+ $"{exhausted}/{totalEvaluations} evaluations exhausted it");
}
/// Reads the count of the private compiled-script cache via reflection. The cache is a