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

This commit is contained in:
Joseph Doherty
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 IMeshCommandDialer? _dialer;
private IMeshPskProvider? _pskProvider; 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 ILoggingAdapter _log = Context.GetLogger();
private readonly bool _useClusterClient; private readonly bool _useClusterClient;
private readonly bool _useGrpc; private readonly bool _useGrpc;
@@ -382,7 +394,7 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers
{ {
var commandType = cmd.Message.GetType().Name; var commandType = cmd.Message.GetType().Name;
var dialer = _dialer; var dialer = Dialer;
if (dialer is null) if (dialer is null)
{ {
_log.Error( _log.Error(
@@ -796,8 +808,8 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers
/// </remarks> /// </remarks>
private void PublishKnownClusters(ContactsLoaded msg) private void PublishKnownClusters(ContactsLoaded msg)
{ {
_pskProvider?.SetKnownClusters(msg.ClusterIds); PskProvider?.SetKnownClusters(msg.ClusterIds);
_pskProvider?.InvalidateAll(); PskProvider?.InvalidateAll();
} }
/// <summary>Stops a cluster's existing client and creates a replacement for its new contact set.</summary> /// <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] [Theory]
public async Task The_deferred_dependencies_are_resolved_and_used_once_the_host_is_up() [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 // 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 // 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 // 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 // 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 // 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. // StartAsync at which a synchronous assertion is correct.
var signal = new SignallingPskProvider(); using var host = BuildAdminHost(mode, useSignallingProvider: true);
using var host = BuildAdminHost(MeshTransportOptions.ModeGrpc, signal);
await host.StartAsync(); await host.StartAsync();
try try
{ {
var signal = (SignallingPskProvider)host.Services.GetRequiredService<IMeshPskProvider>();
await signal.Published.WaitAsync(TimeSpan.FromSeconds(20)); await signal.Published.WaitAsync(TimeSpan.FromSeconds(20));
} }
finally 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. /// starter registration, with the mesh dependency graph wired as it is on a real central node.
/// </summary> /// </summary>
/// <param name="mode">The mesh transport mode to boot under.</param> /// <param name="mode">The mesh transport mode to boot under.</param>
/// <param name="pskProvider"> /// <param name="useSignallingProvider">
/// Substitute for the real <see cref="MeshPskProvider"/>. Only the resolution test supplies /// Substitutes <see cref="SignallingPskProvider"/> for the real <see cref="MeshPskProvider"/>.
/// one; the boot test deliberately keeps the production type so the graph it walks is the /// Only the resolution test sets it; the boot test keeps the production type so the graph it
/// production graph. /// 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> /// </param>
/// <returns>The unstarted host.</returns> /// <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 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. // never about registration order — it was about WHEN the graph is first walked.
services.AddSingleton<IAuditWriter, RegistryBoundAuditWriter>(); services.AddSingleton<IAuditWriter, RegistryBoundAuditWriter>();
services.AddSingleton<ISecretResolver, AuditingSecretResolver>(); services.AddSingleton<ISecretResolver, AuditingSecretResolver>();
if (pskProvider is null) if (useSignallingProvider)
services.AddSingleton<IMeshPskProvider, MeshPskProvider>(); services.AddSingleton<IMeshPskProvider>(sp =>
new SignallingPskProvider(sp.GetRequiredService<ISecretResolver>()));
else else
services.AddSingleton(pskProvider); services.AddSingleton<IMeshPskProvider, MeshPskProvider>();
services.AddSingleton<IMeshCommandDialer, MeshGrpcCommandDialer>(); services.AddSingleton<IMeshCommandDialer, MeshGrpcCommandDialer>();
services.AddAkka("otopcua-starter-order-test", (ab, sp) => services.AddAkka("otopcua-starter-order-test", (ab, sp) =>
@@ -188,6 +202,8 @@ public sealed class ControlPlaneStarterOrderTests
private readonly TaskCompletionSource _published = private readonly TaskCompletionSource _published =
new(TaskCreationOptions.RunContinuationsAsynchronously); new(TaskCreationOptions.RunContinuationsAsynchronously);
public SignallingPskProvider(ISecretResolver resolver) => ArgumentNullException.ThrowIfNull(resolver);
/// <summary>Completes once <see cref="SetKnownClusters"/> has been called.</summary> /// <summary>Completes once <see cref="SetKnownClusters"/> has been called.</summary>
public Task Published => _published.Task; public Task Published => _published.Task;