From e54883a8e049dcd96b9e73a798447f6f1945de35 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 08:07:34 -0400 Subject: [PATCH] =?UTF-8?q?fix(mesh):=20memoize=20dialer/provider=20on=20f?= =?UTF-8?q?irst=20use=20=E2=80=94=20a=20lost=20PreStart=20race=20self-heal?= =?UTF-8?q?s=20at=20the=20first=20refresh=20(review-prescribed)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../CentralCommunicationActor.cs | 18 ++++++-- .../ControlPlaneStarterOrderTests.cs | 42 +++++++++++++------ 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs index 79166703..e25342f4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs @@ -73,6 +73,18 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers private IMeshCommandDialer? _dialer; private IMeshPskProvider? _pskProvider; + // Memoize-on-first-use, NOT PreStart-only: PreStart runs on a dispatcher thread concurrently + // with the builder's remaining starters, so its resolve can lose a millisecond-wide race + // against AuditWriterActorKey's registration (starter #4) and Resolve() would swallow that + // into a PERMANENT silent null — the known-cluster gate never published, fail-open. These + // properties retry on first use instead; the first RefreshContacts arrives as a message, + // long after every starter finished, so a lost boot race self-heals there. Actor-thread + // only, so ??= needs no synchronisation; both are container singletons, so a repeat resolve + // is a dictionary hit. + private IMeshPskProvider? PskProvider => _pskProvider ??= Resolve(_pskProviderFactory, nameof(IMeshPskProvider)); + + private IMeshCommandDialer? Dialer => _dialer ??= Resolve(_dialerFactory, nameof(IMeshCommandDialer)); + private readonly ILoggingAdapter _log = Context.GetLogger(); private readonly bool _useClusterClient; private readonly bool _useGrpc; @@ -382,7 +394,7 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers { var commandType = cmd.Message.GetType().Name; - var dialer = _dialer; + var dialer = Dialer; if (dialer is null) { _log.Error( @@ -796,8 +808,8 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// private void PublishKnownClusters(ContactsLoaded msg) { - _pskProvider?.SetKnownClusters(msg.ClusterIds); - _pskProvider?.InvalidateAll(); + PskProvider?.SetKnownClusters(msg.ClusterIds); + PskProvider?.InvalidateAll(); } /// Stops a cluster's existing client and creates a replacement for its new contact set. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs index 589c34f2..6c90b7c9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs @@ -80,24 +80,35 @@ public sealed class ControlPlaneStarterOrderTests } } - [Fact] - public async Task The_deferred_dependencies_are_resolved_and_used_once_the_host_is_up() + [Theory] + [InlineData(MeshTransportOptions.ModeDps)] + [InlineData(MeshTransportOptions.ModeClusterClient)] + [InlineData(MeshTransportOptions.ModeGrpc)] + public async Task The_deferred_dependencies_are_resolved_and_used_once_the_host_is_up(string mode) { // Deferring must not become "never resolved" — that would trade a loud boot failure for a // silent one, with the known-cluster gate permanently unpublished and the dialer permanently // absent. So this asserts the far end of the deferral: the actor resolved the PSK provider - // FROM THE CONTAINER and drove it, through the ordinary contact-refresh path. + // FROM THE CONTAINER and drove it, through the ordinary contact-refresh path. A Theory over + // all three modes because publication is mode-independent (the gate follows the endpoint). + // + // The provider is registered as a TYPE with an ISecretResolver constructor, so the resolve + // walks the same registry-bound chain the production MeshPskProvider does — which makes + // this fact the race detector for the PreStart-vs-starter-#4 window: a lost race that + // stayed null forever would never publish, and the TaskCompletionSource below would time + // out red. (Memoize-on-first-use in the actor is what heals the lost race at the first + // RefreshContacts message.) // // The signal is a TaskCompletionSource rather than a counter, because PreStart runs on the // actor's own thread and the contact load is a piped DB read: there is no instant after // StartAsync at which a synchronous assertion is correct. - var signal = new SignallingPskProvider(); - using var host = BuildAdminHost(MeshTransportOptions.ModeGrpc, signal); + using var host = BuildAdminHost(mode, useSignallingProvider: true); await host.StartAsync(); try { + var signal = (SignallingPskProvider)host.Services.GetRequiredService(); await signal.Published.WaitAsync(TimeSpan.FromSeconds(20)); } finally @@ -111,13 +122,15 @@ public sealed class ControlPlaneStarterOrderTests /// starter registration, with the mesh dependency graph wired as it is on a real central node. /// /// The mesh transport mode to boot under. - /// - /// Substitute for the real . Only the resolution test supplies - /// one; the boot test deliberately keeps the production type so the graph it walks is the - /// production graph. + /// + /// Substitutes for the real . + /// Only the resolution test sets it; the boot test keeps the production type so the graph it + /// walks is the production graph. The signalling provider is registered as a factory taking + /// from the container, so its resolution walks the same + /// registry-bound chain either way. /// /// The unstarted host. - private static IHost BuildAdminHost(string mode, IMeshPskProvider? pskProvider = null) + private static IHost BuildAdminHost(string mode, bool useSignallingProvider = false) { var clusterOptions = new AkkaClusterOptions { @@ -143,10 +156,11 @@ public sealed class ControlPlaneStarterOrderTests // never about registration order — it was about WHEN the graph is first walked. services.AddSingleton(); services.AddSingleton(); - if (pskProvider is null) - services.AddSingleton(); + if (useSignallingProvider) + services.AddSingleton(sp => + new SignallingPskProvider(sp.GetRequiredService())); else - services.AddSingleton(pskProvider); + services.AddSingleton(); services.AddSingleton(); services.AddAkka("otopcua-starter-order-test", (ab, sp) => @@ -188,6 +202,8 @@ public sealed class ControlPlaneStarterOrderTests private readonly TaskCompletionSource _published = new(TaskCreationOptions.RunContinuationsAsynchronously); + public SignallingPskProvider(ISecretResolver resolver) => ArgumentNullException.ThrowIfNull(resolver); + /// Completes once has been called. public Task Published => _published.Task;