diff --git a/code-reviews/Driver.Calculation/findings.md b/code-reviews/Driver.Calculation/findings.md index 3269936f..0f9ec50c 100644 --- a/code-reviews/Driver.Calculation/findings.md +++ b/code-reviews/Driver.Calculation/findings.md @@ -262,11 +262,15 @@ run so a tag dropped mid-evaluation discards its result rather than publishing i RED first, two tests, both failing against the pre-fix code: `A_dependency_arrival_does_not_run_the_script_on_the_callers_thread` (the publish — which can only happen -after the script ran — landed on the caller's own thread id) and +after the script ran — landed on the caller's own stack, inside the callback) and `A_slow_script_blocks_neither_the_dependency_callback_nor_a_concurrent_read` (a 600 ms busy-wait script: the callback took 1.21 s, and `ReadAsync`, which needs the same state lock, was stuck behind it). Both carry a positive control asserting the value still computes and publishes, so neither can pass against a -driver that simply evaluates nothing. +driver that simply evaluates nothing. ⚠️ The first test's original form compared the publish's **thread id** +against the caller's and was itself flaky (1 failure in 6 runs): the pump is free to reuse the pooled thread +the test released while awaiting, so an id match means nothing. It asserts on a thread-local flag set for +exactly the callback window instead — which is exact, and was re-falsified by making `SchedulePump` run the +pump synchronously (red immediately, then reverted). Known trade, documented rather than hidden: the pending queue is unbounded. Under sustained arrival pressure the previous inline path applied backpressure by blocking the mux actor — which is the defect — diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverTests.cs index 5f9a9160..35945878 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverTests.cs @@ -118,28 +118,33 @@ public sealed class CalculationDriverTests /// DependencyConsumerMuxAdapter.OnDependencyValueChanged — i.e. on an Akka actor's own /// dispatcher thread. Evaluating there means a script run (bounded only by RunTimeout, /// default 2 s) occupies that thread, so the callback must queue the work and return instead. - /// The publish is the observable: it can only happen after the script ran, so a publish on - /// the caller's own thread proves the script ran inline. The value assertion is the positive - /// control — without it this would pass just as happily against a driver that evaluates nothing. + /// The publish is the observable: it can only happen after the script ran, so a publish that + /// lands on the caller's stack between entering and leaving the callback proves the script + /// ran inline. A thread-local flag set for exactly that window is the exact test — comparing thread + /// ids is not, because the pump is free to reuse the pooled thread the test released while + /// waiting, which makes an id match mean nothing. The value assertion is the positive control: + /// without it this would pass just as happily against a driver that evaluates nothing at all. /// [Fact] public async Task A_dependency_arrival_does_not_run_the_script_on_the_callers_thread() { var (driver, published, _) = Build( CalcTag("calc/e/dbl", "return (double)ctx.GetTag(\"src/a\").Value * 2.0;")); - var publishThreadId = 0; - driver.OnDataChange += (_, _) => publishThreadId = Environment.CurrentManagedThreadId; + using var insideCallback = new ThreadLocal(); + var publishedInline = false; + driver.OnDataChange += (_, _) => { if (insideCallback.Value) publishedInline = true; }; await driver.InitializeAsync("{}", TestContext.Current.CancellationToken); - var callerThreadId = Environment.CurrentManagedThreadId; + insideCallback.Value = true; driver.OnDependencyValue("src/a", 5.0, StatusGood, DateTime.UtcNow); + insideCallback.Value = false; - (await WaitUntil(() => !published.IsEmpty, TimeSpan.FromSeconds(5))) + (await WaitUntil(() => !published.IsEmpty, TimeSpan.FromSeconds(10))) .ShouldBeTrue("the queued evaluation must still compute and publish the value"); published.TryDequeue(out var e).ShouldBeTrue(); e!.Snapshot.Value.ShouldBe(10.0); - publishThreadId.ShouldNotBe(callerThreadId, + publishedInline.ShouldBeFalse( "the calc script must run on the driver's own evaluation pump, never on the mux callback thread"); }