test(runtime): late-burst SourceRemoved test no longer races the 5 s PostStop budget (#828)

The test parks the removed child in PostStop (ShutdownGate) to hold the retired-but-not-terminated window open, but PostStop abandons a shutdown after DefaultShutdownTimeout (5 s). Under a loaded suite run more than 5 s can pass between the stop and the host processing the stale burst; the child then terminates, the host forgets it on Terminated, and the burst sent AS it is forwarded and re-retained. Reproduced deterministically with a 6 s sleep before the burst. Not a production path: a terminated actor cannot send. DriverHostActor gains a childShutdownTimeout test seam (last parameter, passed to DriverInstanceActor.Props). The test raises it to 5 min, asserts the child was really stopped (ShutdownCount) before the burst, checks the premise as a positive control (no Terminated by the time the fence has passed), and expects Terminated once the gate is released. With an 8 s sleep before the burst the fixed test passes.
2026-11-11 01:38:46 -05:00
parent 5bbe1e3c7a
commit dea5b84af0
2 changed files with 59 additions and 18 deletions
@@ -491,6 +491,14 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
    /// (<see cref="DriverInstanceActor.WriteAttribute.IssuedAt"/>) means the same thing on both sides.</summary>
    private readonly TimeProvider _writeClock;

+    /// <summary>
+    ///     Test seam: the PostStop shutdown budget every real (non-stub) child is spawned with; null =
+    ///     <see cref=DriverInstanceActor.DefaultShutdownTimeout/>. A test that parks a stopped child in its
+    ///     PostStop to observe the retired-but-not-terminated window raises it, so the window cannot close under
+    ///     load before the test has looked (lmxopcua#828).
+    /// </summary>
+    private readonly TimeSpan? _childShutdownTimeout;
+
    /// <summary>Whether any redundancy snapshot has reached this host — the HistoryUpdate gate stays
    /// closed until one has (see <see cref="PrimaryGatePolicy.ShouldServeHistoryUpdate"/>).</summary>
    private bool _redundancySnapshotReceived;
@@ -738,6 +746,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
    /// Ask is derived (<see cref="DriverWriteBounds.HostAsk"/>). Null ⇒ <see cref="DriverWriteBounds.Default"/>.</param>
    /// <param name="writeClock">The clock the write bounds (and the host's write Ask) run on, shared with every
    /// child. Null ⇒ <see cref="TimeProvider.System"/>.</param>
+    /// <param name="childShutdownTimeout">Test seam: each real child's PostStop shutdown budget; null ⇒
+    /// <see cref="DriverInstanceActor.DefaultShutdownTimeout"/> (lmxopcua#828).</param>
    /// <returns>The Akka.NET <see cref="Akka.Actor.Props"/> used to spawn this actor.</returns>
    public static Props Props(
        IDbContextFactory<OtOpcUaConfigDbContext>? dbFactory,
@@ -776,7 +786,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
        NativeAlarmAckPreflightView? ackPreflightView = null,
        DriverWriteBounds? writeBounds = null,
        TimeProvider? writeClock = null,
-        IScriptRunner? scriptedAlarmRunner = null) =>
+        IScriptRunner? scriptedAlarmRunner = null,
+        TimeSpan? childShutdownTimeout = null) =>
        // WARNING: this forwarding list is POSITIONAL, and Props.Create compiles it into an
        // expression tree. Six IActorRef? parameters and several interface-typed ones mean a
        // mis-ordered argument is usually type-compatible and therefore compiles clean, then binds
@@ -790,7 +801,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
            alarmStateStore, telemetryHub, primaryRoleSource, historizationRoleView,
            sharesHistorizationScopeWithLocal, applyWedgeTimeout, reprovisionBackoff, commsRecoveryAuditDelay,
            sharesAlarmQueueWithLocal, applyOutcomeTimeout, captureWhilePrimaryUnreachable, historyUpdateRoleView,
-            scripting, ackPreflightView, writeBounds, writeClock, scriptedAlarmRunner));
+            scripting, ackPreflightView, writeBounds, writeClock, scriptedAlarmRunner, childShutdownTimeout));

    /// <summary>Initializes a new DriverHostActor with the specified dependencies.</summary>
    /// <param name="dbFactory">Database context factory for configuration database access.</param>
@@ -885,6 +896,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
    /// Ask is derived (<see cref="DriverWriteBounds.HostAsk"/>). Null ⇒ <see cref="DriverWriteBounds.Default"/>.</param>
    /// <param name="writeClock">The clock the write bounds (and the host's write Ask) run on, shared with every
    /// child. Null ⇒ <see cref="TimeProvider.System"/>.</param>
+    /// <param name="childShutdownTimeout">Test seam: each real child's PostStop shutdown budget; null ⇒
+    /// <see cref="DriverInstanceActor.DefaultShutdownTimeout"/> (lmxopcua#828).</param>
    public DriverHostActor(
        IDbContextFactory<OtOpcUaConfigDbContext>? dbFactory,
        CommonsNodeId localNode,
@@ -922,8 +935,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
        NativeAlarmAckPreflightView? ackPreflightView = null,
        DriverWriteBounds? writeBounds = null,
        TimeProvider? writeClock = null,
-        IScriptRunner? scriptedAlarmRunner = null)
+        IScriptRunner? scriptedAlarmRunner = null,
+        TimeSpan? childShutdownTimeout = null)
    {
+        _childShutdownTimeout = childShutdownTimeout;
        _scriptedAlarmRunner = scriptedAlarmRunner;
        _writeBounds = writeBounds ?? DriverWriteBounds.Default;
        _writeClock = writeClock ?? TimeProvider.System;
@@ -3957,7 +3972,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
                    clusterId: clusterId,
                    invoker: invoker,
                    writeTimeout: _writeBounds.Timeout,
-                    writeClock: _writeClock),
+                    writeClock: _writeClock,
+                    shutdownTimeout: _childShutdownTimeout),
                actorName);
            child.Tell(new DriverInstanceActor.InitializeRequested(spec.DriverConfig));
        }
@@ -47,7 +47,8 @@ public sealed class DriverHostActorSourceRemovedTests : RuntimeActorTestBase

    /// <summary>Spawns a host over a real mux with two enabled drivers, waits for both children to subscribe,
    /// registers a replay-opting holder (so the mux retains both refs) and publishes one value per driver.</summary>
-    private void ArrangeTwoDriversWithRetainedValues()
+    /// <param name="childShutdownTimeout">Each child's PostStop shutdown budget (null = the production 5 s).</param>
+    private void ArrangeTwoDriversWithRetainedValues(TimeSpan? childShutdownTimeout = null)
    {
        _db = NewInMemoryDbFactory();
        var revA = V3ArtifactSeeder.SeedV3Deployment(_db, RevA, BothDrivers, NoRefs, enabled: true);
@@ -61,7 +62,8 @@ public sealed class DriverHostActorSourceRemovedTests : RuntimeActorTestBase
            localRoles: new HashSet<string> { "driver" },
            dependencyMux: _mux,
            virtualTagEvaluator: NullVirtualTagEvaluator.Instance,
-            virtualTagHostOverride: CreateTestProbe().Ref));
+            virtualTagHostOverride: CreateTestProbe().Ref,
+            childShutdownTimeout: childShutdownTimeout));
        V3ArtifactSeeder.Dispatch(_host, _coordinator, revA, RevA);
        AwaitAssert(() => _factory.Created.Count.ShouldBe(2), PresenceBudget);
        // The spawn hands a child the PREVIOUS apply's set (empty on the first deploy) and the bulk push then
@@ -137,28 +139,51 @@ public sealed class DriverHostActorSourceRemovedTests : RuntimeActorTestBase

    /// <summary>Review I-1: a burst the removed child sent after the eviction (it stops asynchronously, so it keeps
    /// publishing through the rest of the apply) must be dropped by the host, not forwarded to re-retain the value.</summary>
+    /// <remarks>
+    ///     lmxopcua#828: the window this test needs — the child RETIRED by the host but not yet TERMINATED — is held
+    ///     open by parking the child in its PostStop on <see cref="RecordingSubscribableDriver.ShutdownGate"/>. PostStop
+    ///     abandons a shutdown after its budget (5 s in production), the child then terminates, the host forgets it on
+    ///     <c>Terminated</c>, and a burst sent AS it afterwards is forwarded — so under a loaded suite run (more than
+    ///     5 s from the stop to the host processing the burst) the test failed for a reason that is not a production
+    ///     path (a terminated actor cannot send). Reproduced deterministically by sleeping 6 s before the burst. The
+    ///     budget is therefore raised for this test, and the premise is a checked positive control: the child must
+    ///     still be alive when the fence has passed, or the absence assertion below would mean nothing.
+    /// </remarks>
    [Fact]
    public void A_late_burst_from_a_removed_child_does_not_re_retain_its_value()
    {
-        ArrangeTwoDriversWithRetainedValues();
+        ArrangeTwoDriversWithRetainedValues(childShutdownTimeout: TimeSpan.FromMinutes(5));
        var oldChild = ChildOf("DI-1");
+        var watcher = CreateTestProbe();
+        watcher.Watch(oldChild);
        // Hold the removed child in its PostStop so it has NOT terminated when the stale burst lands; a terminated
        // actor cannot send, and the host forgets a retired child on Terminated.
        var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
        Driver("DI-1").ShutdownGate = gate;
+        try
+        {
+            var revB = V3ArtifactSeeder.SeedV3Deployment(_db, RevB, [("DI-2", "Modbus2", "dev2", "flow", "Double", false)], NoRefs, enabled: true);
+            V3ArtifactSeeder.Dispatch(_host, _coordinator, revB, RevB);
+            // The child really was stopped (it is parked in PostStop now), so the burst below is a RETIRED child's.
+            AwaitAssert(() => Driver("DI-1").ShutdownCount.ShouldBe(1), PresenceBudget, cancellationToken: TestContext.Current.CancellationToken);

-        var revB = V3ArtifactSeeder.SeedV3Deployment(_db, RevB, [("DI-2", "Modbus2", "dev2", "flow", "Double", false)], NoRefs, enabled: true);
-        V3ArtifactSeeder.Dispatch(_host, _coordinator, revB, RevB);
+            // The stale burst, sent AS the stopped child.
+            _host.Tell(ValuePublishes.Single("DI-1", Path1, 11.0, OpcUaStatus.Good, DateTime.UtcNow), oldChild);
+            // Positive control / ordering fence: a live driver's value sent after it is delivered, so the stale burst
+            // has been processed by the host by the time we look.
+            Driver("DI-2").FireDataChange(Path2, 23.0);
+            AwaitHolder(Path2, 23.0);
+            // Premise: the burst came from a retired child that had NOT terminated (it is still parked on the gate).
+            watcher.HasMessages.ShouldBeFalse("the removed child terminated before the fence; the test premise is void");

-        // The stale burst, sent AS the stopped child.
-        _host.Tell(ValuePublishes.Single("DI-1", Path1, 11.0, OpcUaStatus.Good, DateTime.UtcNow), oldChild);
-        // Positive control / ordering fence: a live driver's value sent after it is delivered, so the stale burst
-        // has been processed by the host by the time we look.
-        Driver("DI-2").FireDataChange(Path2, 23.0);
-        AwaitHolder(Path2, 23.0);
-
-        Replayed().Keys.ToArray().ShouldBe(new[] { Path2 }, "the removed child's late burst re-retained its last Good");
-        gate.TrySetResult();
+            Replayed().Keys.ToArray().ShouldBe(new[] { Path2 }, "the removed child's late burst re-retained its last Good");
+        }
+        finally
+        {
+            gate.TrySetResult();
+        }
+        // And the gate really was what held it: released, the child terminates.
+        watcher.ExpectTerminated(oldChild, PresenceBudget, cancellationToken: TestContext.Current.CancellationToken);
    }

    /// <summary>Review m-3: a config edit respawns the driver (stop + spawn); the OLD object's value is evicted, and