From 5c7e5725a5baabe9bf969d88b3ba80f9014cd604 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:05:45 -0400 Subject: [PATCH] =?UTF-8?q?test(runtime):=20first=20supervision-restart=20?= =?UTF-8?q?test=20=E2=80=94=20child=20re-initialises=20+=20re-subscribes?= =?UTF-8?q?=20(archreview=2003/U6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Crash seam: the planned null-Snapshot OnDataChange (no SupervisorStrategy fallback needed). Seen RED with T6 reverted: InitializeCount 1, not 2 — with a preceding ShutdownCount>=1 assertion proving the restart really happened, so the red is not ambiguous with 'never crashed'. --- ...R3-03-restart-resubscribe-watchdog-plan.md | 35 ++++++ ...rt-resubscribe-watchdog-plan.md.tasks.json | 2 +- .../Drivers/DriverHostActorRestartTests.cs | 107 ++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md index 6ab0d180..4b50050c 100644 --- a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md +++ b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md @@ -605,3 +605,38 @@ accept `RestartDriver`. The falsification is T7, below. Gate: `--filter "…~DriverHostActor|…~DriverInstanceActor"` → **183/183 passed** (179 before this task plus T5's four), no test moved. + +### T7 — first supervision-restart test + the force-reconnect verified negative (03/U6) + +**Crash seam that landed: the planned one** — `RecordingSubscribableDriver.FireMalformedDataChange` +raises `OnDataChange` with a null `Snapshot`; `DriverInstanceActor.OnDataChangeForward` dereferences +`msg.Snapshot.SourceTimestampUtc` on the actor thread and NREs; Akka's default decider Restarts. No +fallback (`OneForOneStrategy` + an internal test message) was needed, so no production surface was +added for the test. + +**RED observed against a T6-reverted build** (the `Context.Parent.Tell(new ReprovisionRequested(…))` +in `PostRestart` commented out): + +``` +Supervision_restart_reinitialises_and_re_subscribes_the_child [FAIL] + driver.InitializeCount should be 2 but was 1 + "the restarted child was never re-initialised — it is sitting in Connecting around a driver + its own PostStop already shut down (03/S6, the worse half of 03/S16)" +``` + +**Deviation, and the reason for it — the first red run was AMBIGUOUS and the test was strengthened.** +`InitializeCount == 1` is equally consistent with "the child restarted and was not re-provisioned" +and with "the child never crashed at all", so a seam that silently stopped working would read as a +passing regression test forever. A preceding `AwaitAssert(ShutdownCount >= 1)` was added — `PreRestart` +runs `PostStop`, which calls `ShutdownAsync`, so that is the evidence a restart genuinely happened. +On the re-run of the falsification that assertion **passed** and the failure landed squarely on the +re-initialise, which is what makes the red observation above meaningful. + +`Force_reconnect_keeps_the_desired_set_and_re_subscribes` passed in **both** the red and green runs — +correct, and the point: it is the verified negative (`ForceReconnect` reaches the same live actor, so +`_desiredRefs` survive and the reconnect's Connected entry re-applies them). It carries a 30 s budget +rather than `PresenceBudget`, because `SpawnChild` gives the child the production +`DefaultReconnectInterval` (10 s) and the retry is a periodic timer, so the reconnect cannot land +sooner; a budget is only spent on failure. + +After restoring the hook: **6/6 pass**, and the class was run **20× in isolation, 20/20 green**. diff --git a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json index 0e1d6c9f..625327b6 100644 --- a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json +++ b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json @@ -48,7 +48,7 @@ { "id": "T7", "subject": "First real supervision-restart test + force-reconnect verified-negative regression (03/U6)", - "status": "pending", + "status": "done", "blockedBy": [ "T4", "T6" diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRestartTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRestartTests.cs index 621213c1..e8abff55 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRestartTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRestartTests.cs @@ -284,4 +284,111 @@ public sealed class DriverHostActorRestartTests : RuntimeActorTestBase }, duration: PresenceBudget); } + + /// + /// The repo's first genuine Akka SUPERVISION-restart test (03/U6). Not a stop, not a stub: the + /// driver raises a malformed OnDataChange, the actor NREs on its own thread, and Akka's default + /// decider Restarts the child in place. + /// + /// A restart is strictly worse than the operator-Restart case 03/S16 names. It recreates the actor + /// object around the SAME driver instance without going through SpawnChild, so nothing + /// re-sends InitializeRequested — while PreRestart has already run PostStop, + /// which called ShutdownAsync on that very driver. The child therefore sat in + /// Connecting forever around a shut-down driver, with its desired set gone too, and + /// silently: Akka logs the fault, nothing logs the non-recovery. + /// + /// + /// The assertions are FUNCTIONS of the new incarnation — it re-initialised, it re-subscribed the + /// deployed refs — never that it exists. Created.Count == 1 is what distinguishes this from + /// the operator path: a restart must NOT produce a second driver object. + /// + /// + [Fact] + public void Supervision_restart_reinitialises_and_re_subscribes_the_child() + { + var db = NewInMemoryDbFactory(); + var (_, factory) = DeployOneDriver(db, new[] { (DriverId, "Modbus", "dev1", "speed", "Float", false) }); + + AwaitAssert( + () => + { + factory.Created.Count.ShouldBe(1); + factory.Created[0].SubscribeCount.ShouldBe(1); + }, + duration: PresenceBudget); + + var driver = factory.Created[0]; + driver.InitializeCount.ShouldBe(1); + + // The crash. Subscribing is what attaches the actor's OnDataChange handler, so this only reaches the + // actor thread because the arrangement above got as far as a live subscription. + driver.FireMalformedDataChange(SpeedPath); + + // FIRST: prove the crash seam actually tripped supervision, before asserting anything about the + // recovery. PreRestart runs PostStop, which calls ShutdownAsync on the driver — so a raised + // ShutdownCount is the evidence that a restart happened at all. Without this the assertions below + // would be ambiguous: "the child was not re-provisioned" and "the child never crashed" look + // identical from the outside, and a seam that silently stopped working would read as a passing + // regression test forever. + AwaitAssert( + () => driver.ShutdownCount.ShouldBeGreaterThanOrEqualTo( + 1, "the crash seam did not trip supervision — nothing below is testing a restart"), + duration: PresenceBudget); + + AwaitAssert( + () => + { + factory.Created.Count.ShouldBe( + 1, "a supervision restart must re-use the same driver object, not respawn the child"); + driver.InitializeCount.ShouldBe( + 2, + "the restarted child was never re-initialised — it is sitting in Connecting around a " + + "driver its own PostStop already shut down (03/S6, the worse half of 03/S16)"); + driver.SubscribeCount.ShouldBeGreaterThanOrEqualTo( + 2, "the restarted child never re-subscribed — its desired set died with the old instance"); + driver.LastSubscribedRefs.ShouldBe(new[] { SpeedPath }); + }, + duration: PresenceBudget); + } + + /// + /// The verified negative, pinned so this plan's edits to the restart paths cannot break the one that + /// was already correct. Force Reconnect (ReconnectDriver) Tells ForceReconnect to the + /// SAME child actor: the instance survives, so _desiredRefs/_desiredAlarmRefs are intact + /// and the reconnect's Connected entry re-applies them by itself. Nothing needs re-provisioning, and + /// no new driver object is created. + /// + /// + /// The budget here is deliberately wider than : + /// SpawnChild gives the child the production DefaultReconnectInterval (10 s) and the + /// retry is a periodic timer, so the reconnect cannot land sooner than that. A budget is spent only + /// when the assertion fails, so widening it costs nothing on the happy path. + /// + [Fact] + public void Force_reconnect_keeps_the_desired_set_and_re_subscribes() + { + var db = NewInMemoryDbFactory(); + var (host, factory) = DeployOneDriver(db, new[] { (DriverId, "Modbus", "dev1", "speed", "Float", false) }); + + AwaitAssert( + () => + { + factory.Created.Count.ShouldBe(1); + factory.Created[0].SubscribeCount.ShouldBe(1); + }, + duration: PresenceBudget); + + var driver = factory.Created[0]; + host.Tell(new ReconnectDriver("c1", DriverId, "tester", Guid.NewGuid())); + + AwaitAssert( + () => + { + factory.Created.Count.ShouldBe(1, "Force Reconnect must NOT respawn the child"); + driver.SubscribeCount.ShouldBeGreaterThanOrEqualTo( + 2, "the reconnected driver never re-subscribed its (surviving) desired set"); + driver.LastSubscribedRefs.ShouldBe(new[] { SpeedPath }); + }, + duration: TimeSpan.FromSeconds(30)); + } }