test(secrets): make the context-after-await pin deterministic (fixes flake)
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
This commit is contained in:
+59
-29
@@ -4,14 +4,14 @@ using Akka.TestKit.Xunit2;
|
|||||||
namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests;
|
namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Pins the Akka behaviour that <see cref="SecretReplicationActor"/> depends on: after a genuine
|
/// Pins the Akka behaviour that <see cref="SecretReplicationActor"/> depends on: on a thread with
|
||||||
/// thread hop, actor context is <b>gone</b>, so <c>Self</c> cannot be read.
|
/// no actor context, <c>Self</c> and <c>Context</c> cannot be read.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <remarks>
|
/// <remarks>
|
||||||
/// <para>
|
/// <para>
|
||||||
/// This is here because assuming otherwise produced a real bug. <c>Self</c> resolves through
|
/// This is here because assuming otherwise produced a real bug. <c>Self</c> resolves through
|
||||||
/// <c>Context</c>, which is <c>[ThreadStatic]</c>; a continuation that resumes on a thread-pool
|
/// <c>Context</c>, which is <c>[ThreadStatic]</c>; a continuation that resumes on a thread without
|
||||||
/// thread has none, and reading it throws <see cref="NotSupportedException"/>.
|
/// actor context throws <see cref="NotSupportedException"/> when reading either.
|
||||||
/// </para>
|
/// </para>
|
||||||
/// <para>
|
/// <para>
|
||||||
/// What made it dangerous is that it does <em>not</em> reliably throw. When the awaited operation
|
/// What made it dangerous is that it does <em>not</em> reliably throw. When the awaited operation
|
||||||
@@ -21,12 +21,24 @@ namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests;
|
|||||||
/// therefore unconditional: capture <c>Self</c> (and <c>Sender</c>) into locals while still on the
|
/// therefore unconditional: capture <c>Self</c> (and <c>Sender</c>) into locals while still on the
|
||||||
/// actor thread, and never touch actor context after an <c>await</c>.
|
/// actor thread, and never touch actor context after an <c>await</c>.
|
||||||
/// </para>
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// The illegal reads run on a <b>dedicated new thread</b>, not directly in the pool continuation.
|
||||||
|
/// An earlier revision read from the <c>ConfigureAwait(false)</c> 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 <c>Context</c> return <c>null</c> and <c>Self</c> return the
|
||||||
|
/// cleared ref <em>without throwing</em>. The very unreliability this test documents made its old
|
||||||
|
/// formulation unsound. A fresh thread is the one place the no-context state is guaranteed.
|
||||||
|
/// </para>
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
public sealed class ActorContextAfterAwaitTests : TestKit
|
public sealed class ActorContextAfterAwaitTests : TestKit
|
||||||
{
|
{
|
||||||
private sealed record Probe;
|
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
|
private sealed class ProbeActor : ReceiveActor
|
||||||
{
|
{
|
||||||
@@ -39,40 +51,52 @@ public sealed class ActorContextAfterAwaitTests : TestKit
|
|||||||
// The correct pattern: capture while the context is still current.
|
// The correct pattern: capture while the context is still current.
|
||||||
IActorRef capturedSelf = Self;
|
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.Delay(50).ConfigureAwait(false);
|
||||||
await Task.Yield();
|
|
||||||
|
|
||||||
string? selfError = null;
|
string? selfError = null;
|
||||||
string? contextError = null;
|
string? contextError = null;
|
||||||
|
string? staleSelfPath = null;
|
||||||
|
string? staleContextPath = null;
|
||||||
|
|
||||||
try
|
// 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.
|
||||||
ActorPath path = Self.Path;
|
await Task.Factory.StartNew(
|
||||||
GC.KeepAlive(path);
|
() =>
|
||||||
}
|
{
|
||||||
catch (Exception ex)
|
try
|
||||||
{
|
{
|
||||||
selfError = ex.GetType().Name;
|
// 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
|
try
|
||||||
{
|
{
|
||||||
IActorRef fromContext = Context.Self;
|
staleContextPath = Context.Self.Path.ToString();
|
||||||
GC.KeepAlive(fromContext);
|
}
|
||||||
}
|
catch (Exception ex)
|
||||||
catch (Exception ex)
|
{
|
||||||
{
|
contextError = ex.GetType().Name;
|
||||||
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]
|
[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");
|
IActorRef probe = Sys.ActorOf(Props.Create(() => new ProbeActor()), "context-probe");
|
||||||
probe.Tell(new Probe());
|
probe.Tell(new Probe());
|
||||||
@@ -80,8 +104,14 @@ public sealed class ActorContextAfterAwaitTests : TestKit
|
|||||||
Outcome outcome = ExpectMsg<Outcome>(TimeSpan.FromSeconds(10));
|
Outcome outcome = ExpectMsg<Outcome>(TimeSpan.FromSeconds(10));
|
||||||
|
|
||||||
// Both throw — Self is not a cached field, it goes through the ThreadStatic context.
|
// Both throw — Self is not a cached field, it goes through the ThreadStatic context.
|
||||||
Assert.Equal("NotSupportedException", outcome.SelfError);
|
Assert.True(
|
||||||
Assert.Equal("NotSupportedException", outcome.ContextError);
|
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.
|
// ...but a reference captured beforehand stays valid, which is what the actor relies on.
|
||||||
Assert.EndsWith("/user/context-probe", outcome.CapturedSelfPath, StringComparison.Ordinal);
|
Assert.EndsWith("/user/context-probe", outcome.CapturedSelfPath, StringComparison.Ordinal);
|
||||||
|
|||||||
Reference in New Issue
Block a user