From 9343b4f30ff34eedca0910d9d02b5aa6f6dcd14a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 00:20:12 -0400 Subject: [PATCH] fix(site-runtime): watch every Instance Actor; unexpected termination evicts the dead ref from routing (S6) --- .../Actors/DeploymentManagerActor.cs | 53 +++++++++++++++---- .../Actors/DeploymentManagerRedeployTests.cs | 53 ++++++++++++++++++- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index 5c33bbf8..c2ff3980 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -526,22 +526,48 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers } /// - /// Recreates an Instance Actor once its predecessor has fully terminated during a - /// redeployment, draining the buffered . + /// Handles an Instance Actor's signal. Two cases: + /// + /// The child was stopped as part of a redeployment — recreate it from the + /// buffered . + /// The child terminated OUTSIDE the redeploy flow — an init failure or crash. + /// Evict the now-dead ref from the routing map so the singleton never routes to a + /// stopped actor (S6). + /// /// private void HandleTerminated(Terminated terminated) { - if (!_pendingRedeploys.Remove(terminated.ActorRef, out var pending)) + // Case 1 — redeployment drain: the predecessor of a buffered redeploy has + // fully stopped and freed its actor name. + if (_pendingRedeploys.Remove(terminated.ActorRef, out var pending)) + { + // Drop the name-keyed shadow now that the predecessor has + // fully terminated and its actor name is free. Any deploy arriving after + // this point sees neither _instanceActors[name] (cleared when we stopped + // the predecessor) nor _terminatingActorsByName[name] (cleared here), so + // ApplyDeployment + Context.ActorOf below safely reuses the name. + _terminatingActorsByName.Remove(pending.Command.InstanceUniqueName); + + ApplyDeployment(pending.Command, pending.OriginalSender, isRedeploy: true); return; + } - // Drop the name-keyed shadow now that the predecessor has - // fully terminated and its actor name is free. Any deploy arriving after - // this point sees neither _instanceActors[name] (cleared when we stopped - // the predecessor) nor _terminatingActorsByName[name] (cleared here), so - // ApplyDeployment + Context.ActorOf below safely reuses the name. - _terminatingActorsByName.Remove(pending.Command.InstanceUniqueName); - - ApplyDeployment(pending.Command, pending.OriginalSender, isRedeploy: true); + // Case 2 — S6 fallback: an Instance Actor terminated unexpectedly (init + // failure or crash). Expected stops (disable/delete/redeploy/persistence + // rollback) all remove or replace the map entry BEFORE the child stops, so + // the equality guard is false for them and this branch is inert. Only a dead + // ref still mapped to this exact actor reaches here. + var name = terminated.ActorRef.Path.Name; + if (_instanceActors.TryGetValue(name, out var mapped) && mapped.Equals(terminated.ActorRef)) + { + _instanceActors.Remove(name); + UpdateInstanceCounts(); + _logger.LogError( + "Instance Actor {Instance} terminated unexpectedly (init failure or crash) — removed from routing", + name); + LogDeploymentEvent("Error", name, + $"Instance {name} terminated unexpectedly (init failure or crash) — removed from routing"); + } } /// @@ -1718,6 +1744,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers var actorRef = Context.ActorOf(props, instanceName); _instanceActors[instanceName] = actorRef; + // Watch every Instance Actor so an unexpected termination (init failure or + // crash) delivers a Terminated signal that evicts the dead ref from routing + // (S6). Redeploy already double-watches; a double Watch is idempotent in + // Akka.NET (a single Terminated is delivered). + Context.Watch(actorRef); _logger.LogDebug("Created Instance Actor for {Instance}", instanceName); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerRedeployTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerRedeployTests.cs index e6980a99..3258fb81 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerRedeployTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerRedeployTests.cs @@ -10,6 +10,7 @@ using ZB.MOM.WW.ScadaBridge.HealthMonitoring; using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; using System.Text.Json; namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; @@ -45,7 +46,8 @@ public class DeploymentManagerRedeployTests : TestKit, IDisposable try { File.Delete(_dbFile); } catch { /* cleanup */ } } - private IActorRef CreateDeploymentManager(ISiteHealthCollector? healthCollector = null) + private IActorRef CreateDeploymentManager( + ISiteHealthCollector? healthCollector = null, IServiceProvider? serviceProvider = null) { return ActorOf(Props.Create(() => new DeploymentManagerActor( _storage, @@ -57,9 +59,31 @@ public class DeploymentManagerRedeployTests : TestKit, IDisposable null, null, healthCollector, - null))); + serviceProvider))); } + /// + /// Builds a config that deserializes and compiles cleanly (so it passes the deploy + /// compile gate) but whose script's canonical name contains a space — illegal as an + /// Akka actor name, so Context.ActorOf(props, "script-dies on init") throws + /// InvalidActorNameException during the Instance Actor's PreStart. The failure + /// surfaces as an ActorInitializationException the DM decider stops — the exact + /// "Instance Actor dies during init" S6 scenario. + /// + private static string CtorThrowingConfig(string instanceName) => + JsonSerializer.Serialize(new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute { CanonicalName = "TestAttr", Value = "1", DataType = "Int32" } + ], + Scripts = + [ + new ResolvedScript { CanonicalName = "dies on init", Code = "return 1;" } + ] + }); + /// /// Minimal fake that records the most recent deployed-instance count. /// @@ -311,4 +335,29 @@ public class DeploymentManagerRedeployTests : TestKit, IDisposable // not a new instance, so the in-memory counter must not drift upward. Assert.Equal(1, health.LastDeployedCount); } + + // ── S6: an Instance Actor that dies during init must not linger as a dead ref ── + + [Fact] + public async Task InstanceActorInitFailure_OnStartup_EvictsDeadRefAndLogsSiteEvent() + { + // A stored config that compiles cleanly but whose Instance Actor cannot + // initialize (illegal child actor name → ActorInitializationException → Stop). + // The DM watches every Instance Actor, so on the unexpected Termination it must + // evict the dead ref from the routing map and log a `deployment` Error event + // rather than leaving a routing entry that points at a stopped actor (S6). + await _storage.StoreDeployedConfigAsync( + "poison", CtorThrowingConfig("poison"), "d1", "h1", true); + + var siteLog = new FakeSiteEventLogger(); + CreateDeploymentManager(serviceProvider: new SingleServiceProvider(siteLog)); + + AwaitAssert(() => + { + Assert.Contains(siteLog.OfType("deployment"), r => + r.Severity == "Error" && + r.InstanceId == "poison" && + r.Message.Contains("terminated unexpectedly", StringComparison.OrdinalIgnoreCase)); + }, TimeSpan.FromSeconds(10)); + } }