test(acl): race harness — hit diagnostics, 10k rounds, idiom notes (review round)

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
@@ -14,7 +15,7 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
 ///     different from the one asked. Each test below pins one dimension the key must discriminate on,
 ///     because a key that collides across any of them hands one session another's authority.
 /// </remarks>
-public sealed class SessionAclDecisionCacheTests
+public sealed class SessionAclDecisionCacheTests(ITestOutputHelper output)
 {
     private const string Session = "key-1";
     private const string Node = "Plant/Modbus/dev1/Speed";
@@ -540,8 +541,18 @@ public sealed class SessionAclDecisionCacheTests
     ///         threads per round land red about once in twenty thousand, a two-party <see cref="Barrier"/>
     ///         no better (its post-phase action releases from <i>inside</i> one racer, handing it a head
     ///         start), while the long-lived racers plus a third-thread starter used below turn roughly a
-    ///         quarter to a half of all rounds red. So a round or two is enough in practice, and the
-    ///         twenty thousand are for the machine that is slower or busier than this one.
+    ///         sixth to a half of all rounds red.
+    ///     </para>
+    ///     <para>
+    ///         ⚠️ <b>Do not cut the round count on that arithmetic.</b> A per-round rate near 25% suggests a
+    ///         couple of hundred rounds would be overwhelming — but the misses are not independent, they
+    ///         arrive in run-length bursts: the racers get descheduled <i>together</i> for a stretch, and the
+    ///         stretch can be the whole run. Measured, against the mutation, with the rest of this assembly
+    ///         running in parallel as it does in CI: <b>2,000 rounds passed vacuously in 1 run of 3</b>
+    ///         (stale=0 — the defect present and undetected), while 10,000 was red in 5 of 5, first hit
+    ///         within rounds 1-13, stale 1,595-4,591. The same 2,000 rounds run <i>alone</i> never missed
+    ///         (70-986 stale), so the load from the sibling classes is the variable that matters, and a
+    ///         standalone measurement will overstate the margin. The cost of the margin is ~20ms.
     ///     </para>
     ///     <para>
     ///         Every round gets a <b>fresh cache</b>: both racers must start from the <c>-1</c> seed, or the
@@ -564,21 +575,31 @@ public sealed class SessionAclDecisionCacheTests
     [Fact]
     public void Concurrent_lookups_at_mixed_generations_always_publish_the_highest()
     {
-        const int rounds = 20_000;
+        const int rounds = 10_000;
         const int spinsBeforeYield = 4_000;
 
-        // The cache under race, handed over through a slot the starter rewrites between rounds.
+        // Array-as-ref-cell: a one-element array is the cheapest storage a lambda can close over AND that
+        // Volatile.Read/Write can take a `ref` to — a captured local cannot be passed by ref, and a plain
+        // field would need the test moved onto the class.
         var slot = new SessionAclDecisionCache[1];
         var gate = 0;      // starter's release signal: the round number both racers are cleared to run
         var finished = 0;  // racers' completion count for the current round
         var firstStaleRound = -1;
         var staleRounds = 0;
 
+        // ⚠️ Deliberately NOT the file's Barrier-per-round idiom (see
+        // Concurrent_eviction_and_insertion_leave_the_counter_exact, which is the right shape for its job:
+        // many workers that only need to start together). This gate/finished spin protocol exists because
+        // the window here is nanoseconds wide and a Barrier releases from inside one of the racers — see the
+        // remarks above. Both idioms are in the file on purpose; neither is a stray.
+        //
         // Raw threads rather than the thread pool: the pool may serialise two work items onto one thread,
         // which would make the race unreachable. Held for the whole run so the release is the only skew.
         var low = new Thread(() => Race(1L)) { IsBackground = true };
         var high = new Thread(() => Race(2L)) { IsBackground = true };
 
+        var started = Stopwatch.GetTimestamp();
+
         low.Start();
         high.Start();
 
@@ -602,6 +623,21 @@ public sealed class SessionAclDecisionCacheTests
         low.Join();
         high.Join();
 
+        var elapsed = Stopwatch.GetElapsedTime(started);
+
+        // Emitted unconditionally, because the way this test fails on a constrained runner is by passing.
+        // ⚠️ It cannot report a hit rate: under the fixed code no round is stale BY DEFINITION, and how many
+        // rounds actually entered the read-then-exchange window is unobservable from out here — the only way
+        // to see it would be to instrument production, which is not worth a diagnostic. So it reports what it
+        // can honestly measure, and the reading is: fewer than 3 processors means the three threads cannot
+        // run concurrently and the race is unreachable, while a per-round cost well above the 1.5-3us/round
+        // this machine turns in means the racers are being descheduled rather than spinning (24us/round was
+        // observed under load, and that is the shape of a run that proves less than it looks like it does).
+        // The measured red rate under the single-shot mutation is 16-46% of rounds.
+        output.WriteLine(
+            $"rounds={rounds} stale={staleRounds} processors={Environment.ProcessorCount} " +
+            $"elapsed={elapsed.TotalMilliseconds:F1}ms perRound={elapsed.TotalMicroseconds / rounds:F2}us");
+
         firstStaleRound.ShouldBe(
             -1,
             $"round {firstStaleRound} (of {staleRounds} stale in {rounds}): the newer generation must " +