From 51bef634d0caf2cf794481ca64b3c06ee7cab3ab Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 18 Jul 2026 14:54:30 -0400 Subject: [PATCH] test(secrets): make the context-after-await pin deterministic (fixes flake) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ActorContextAfterAwaitTests asserted that a ConfigureAwait(false) continuation on the shared thread pool always throws NotSupportedException when reading Self/Context. That property is a timing accident, not an Akka guarantee: pool threads are exactly where Akka and the TestKit legitimately install the [ThreadStatic] actor cell during mailbox runs and async continuations, and Akka 1.5.62 has two non-throwing states besides — a cleared cell makes ActorBase.Context return null (NullReferenceException on .Self, not NotSupportedException) and ActorBase.Self return _clearedSelf without any throw. Under parallel suite load the assertion failed once at exactly that seam (2026-07-18); ironically the test's own doc comment said the behaviour "does not reliably throw" and then asserted reliability. The illegal reads now run on a dedicated new thread (LongRunning), the one place the no-context state is guaranteed, while the realistic ConfigureAwait(false) escape from the mailbox is kept. If the reads ever unexpectedly succeed again, the failure message reports whose context the thread was carrying. Verified 3 consecutive full-project runs green (38/38) after 8 instrumented runs hunting the original repro. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts --- .../ActorContextAfterAwaitTests.cs | 88 +++++++++++++------ 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ActorContextAfterAwaitTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ActorContextAfterAwaitTests.cs index 0327e7a..8ce1a02 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ActorContextAfterAwaitTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ActorContextAfterAwaitTests.cs @@ -4,14 +4,14 @@ using Akka.TestKit.Xunit2; namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests; /// -/// Pins the Akka behaviour that depends on: after a genuine -/// thread hop, actor context is gone, so Self cannot be read. +/// Pins the Akka behaviour that depends on: on a thread with +/// no actor context, Self and Context cannot be read. /// /// /// /// This is here because assuming otherwise produced a real bug. Self resolves through -/// Context, which is [ThreadStatic]; a continuation that resumes on a thread-pool -/// thread has none, and reading it throws . +/// Context, which is [ThreadStatic]; a continuation that resumes on a thread without +/// actor context throws when reading either. /// /// /// What made it dangerous is that it does not reliably throw. When the awaited operation @@ -21,12 +21,24 @@ namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests; /// therefore unconditional: capture Self (and Sender) into locals while still on the /// actor thread, and never touch actor context after an await. /// +/// +/// The illegal reads run on a dedicated new thread, not directly in the pool continuation. +/// An earlier revision read from the ConfigureAwait(false) continuation itself and flaked +/// (observed 2026-07-18 under parallel suite load): shared thread-pool threads are exactly where +/// Akka and the TestKit legitimately install the ThreadStatic cell during mailbox runs and async +/// continuations, so "no context here" is a timing accident, not a guarantee — worse, a cell whose +/// actor was already cleared makes Context return null and Self return the +/// cleared ref without throwing. The very unreliability this test documents made its old +/// formulation unsound. A fresh thread is the one place the no-context state is guaranteed. +/// /// public sealed class ActorContextAfterAwaitTests : TestKit { private sealed record Probe; - private sealed record Outcome(string? SelfError, string? ContextError, string? CapturedSelfPath); + private sealed record Outcome( + string? SelfError, string? ContextError, string? CapturedSelfPath, + string? StaleSelfPath, string? StaleContextPath); private sealed class ProbeActor : ReceiveActor { @@ -39,40 +51,52 @@ public sealed class ActorContextAfterAwaitTests : TestKit // The correct pattern: capture while the context is still current. IActorRef capturedSelf = Self; - // Force a real hop off the mailbox thread. + // The realistic escape: after this, the handler is off the mailbox thread. await Task.Delay(50).ConfigureAwait(false); - await Task.Yield(); string? selfError = null; string? contextError = null; + string? staleSelfPath = null; + string? staleContextPath = null; - try - { - ActorPath path = Self.Path; - GC.KeepAlive(path); - } - catch (Exception ex) - { - selfError = ex.GetType().Name; - } + // LongRunning = a brand-new thread, where no actor cell can ever have been + // installed. Reading Self/Context THERE is the deterministic form of the hazard. + await Task.Factory.StartNew( + () => + { + try + { + // If this ever succeeds the failure message needs to say whose + // context the thread was carrying — that is the whole diagnosis. + staleSelfPath = Self.Path.ToString(); + } + catch (Exception ex) + { + selfError = ex.GetType().Name; + } - try - { - IActorRef fromContext = Context.Self; - GC.KeepAlive(fromContext); - } - catch (Exception ex) - { - contextError = ex.GetType().Name; - } + try + { + staleContextPath = Context.Self.Path.ToString(); + } + catch (Exception ex) + { + contextError = ex.GetType().Name; + } + }, + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default).ConfigureAwait(false); - replyTo.Tell(new Outcome(selfError, contextError, capturedSelf.Path.ToString())); + replyTo.Tell(new Outcome( + selfError, contextError, capturedSelf.Path.ToString(), + staleSelfPath, staleContextPath)); }); } } [Fact] - public void Self_and_Context_are_both_unreadable_after_a_real_thread_hop() + public void Self_and_Context_are_both_unreadable_on_a_thread_without_actor_context() { IActorRef probe = Sys.ActorOf(Props.Create(() => new ProbeActor()), "context-probe"); probe.Tell(new Probe()); @@ -80,8 +104,14 @@ public sealed class ActorContextAfterAwaitTests : TestKit Outcome outcome = ExpectMsg(TimeSpan.FromSeconds(10)); // Both throw — Self is not a cached field, it goes through the ThreadStatic context. - Assert.Equal("NotSupportedException", outcome.SelfError); - Assert.Equal("NotSupportedException", outcome.ContextError); + Assert.True( + outcome.SelfError == "NotSupportedException", + $"Self read did not throw NotSupportedException (got: {outcome.SelfError ?? "no exception"}; " + + $"stale Self path: '{outcome.StaleSelfPath}')."); + Assert.True( + outcome.ContextError == "NotSupportedException", + $"Context read did not throw NotSupportedException (got: {outcome.ContextError ?? "no exception"}; " + + $"stale Context path: '{outcome.StaleContextPath}')."); // ...but a reference captured beforehand stays valid, which is what the actor relies on. Assert.EndsWith("/user/context-probe", outcome.CapturedSelfPath, StringComparison.Ordinal);