fix(mesh): memoize dialer/provider on first use — a lost PreStart race self-heals at the first refresh (review-prescribed)

2026-08-06 08:07:34 -04:00
parent c0475f2df4
commit e54883a8e0
2 changed files with 44 additions and 16 deletions
@@ -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
    /// </remarks>
    private void PublishKnownClusters(ContactsLoaded msg)
    {
-        _pskProvider?.SetKnownClusters(msg.ClusterIds);
-        _pskProvider?.InvalidateAll();
+        PskProvider?.SetKnownClusters(msg.ClusterIds);
+        PskProvider?.InvalidateAll();
    }

    /// <summary>Stops a cluster's existing client and creates a replacement for its new contact set.</summary>
@@ -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<IMeshPskProvider>();
            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.
    /// </summary>
    /// <param name="mode">The mesh transport mode to boot under.</param>
-    /// <param name="pskProvider">
-    ///     Substitute for the real <see cref="MeshPskProvider"/>. Only the resolution test supplies
-    ///     one; the boot test deliberately keeps the production type so the graph it walks is the
-    ///     production graph.
+    /// <param name="useSignallingProvider">
+    ///     Substitutes <see cref="SignallingPskProvider"/> for the real <see cref="MeshPskProvider"/>.
+    ///     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
+    ///     <see cref="ISecretResolver"/> from the container, so its resolution walks the same
+    ///     registry-bound chain either way.
    /// </param>
    /// <returns>The unstarted host.</returns>
-    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<IAuditWriter, RegistryBoundAuditWriter>();
                services.AddSingleton<ISecretResolver, AuditingSecretResolver>();
-                if (pskProvider is null)
-                    services.AddSingleton<IMeshPskProvider, MeshPskProvider>();
+                if (useSignallingProvider)
+                    services.AddSingleton<IMeshPskProvider>(sp =>
+                        new SignallingPskProvider(sp.GetRequiredService<ISecretResolver>()));
                else
-                    services.AddSingleton(pskProvider);
+                    services.AddSingleton<IMeshPskProvider, MeshPskProvider>();
                services.AddSingleton<IMeshCommandDialer, MeshGrpcCommandDialer>();

                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);
+
        /// <summary>Completes once <see cref="SetKnownClusters"/> has been called.</summary>
        public Task Published => _published.Task;