Skip to content

fix: resolve implicit-sender leak under xUnit v3 parallel execution#735

Merged
Aaronontheweb merged 7 commits into
akkadotnet:devfrom
Aaronontheweb:xunit3-testkit-compat-cleanup
May 17, 2026
Merged

fix: resolve implicit-sender leak under xUnit v3 parallel execution#735
Aaronontheweb merged 7 commits into
akkadotnet:devfrom
Aaronontheweb:xunit3-testkit-compat-cleanup

Conversation

@Aaronontheweb
Copy link
Copy Markdown
Member

Summary

Fixes #733Akka.Hosting.TestKit implicit-sender leaks across ActorSystems under xUnit v3 parallel class execution.

Problem

InternalCurrentActorCellKeeper.Current is [ThreadStatic] and does not flow through ExecutionContext. Under xUnit v3's default parallel-class scheduling (MaxConcurrencySyncContext), await continuations resume on ThreadPool threads where a sibling test pinned its cell, causing Tell() to use the wrong implicit sender. Replies then cross ActorSystem boundaries and land in the wrong TestActor's mailbox.

Root Cause of Prior Spike Hang

The akka.net PR #8174 approach (AkkaCleanAmbientContextAttribute + ActorCellKeepingSynchronizationContext) replaces xUnit's SynchronizationContext with one that dispatches via raw ThreadPool.QueueUserWorkItem. This works for Akka.TestKit.Xunit but hangs Akka.Hosting.TestKit because Hosting's IHost async startup lifecycle depends on xUnit's MaxConcurrencySyncContext scheduling.

Fix

A wrapping/decorating SynchronizationContext that preserves xUnit's scheduler while pinning the ambient actor cell across await continuations:

  • ActorCellKeepingSynchronizationContext — delegates Post()/Send() to the captured outer SC (preserving xUnit scheduling) while wrapping callbacks with a save/pin/restore window for InternalCurrentActorCellKeeper.Current. Falls back to ThreadPool dispatch when no outer SC exists.
  • HostingCleanAmbientContextAttributeBeforeAfterTestAttribute that captures the active SC before each test, installs the decorator, pins the cell, and restores everything in After().
  • EnsureImplicitSender() — guarded with Current == null to prevent overwriting the cell during actor message processing (matches akka.net's pattern).
  • Removed two redundant cell-pinning writes now handled by the attribute.
  • Enabled parallel test collections; 24 regression tests added (16 implicit-sender + 8 INoImplicitSender).

Upstream Path

This proves the wrapping SC concept in Hosting. Once validated in CI, the approach can be upstreamed to akka.net PR #8174 to replace the raw-ThreadPool SC, then Hosting can switch to the upstream attribute.

Changes

  • src/Akka.Hosting.TestKit/Internals/ActorCellKeepingSynchronizationContext.csNew: wrapping SC
  • src/Akka.Hosting.TestKit/Internals/HostingCleanAmbientContextAttribute.csNew: BeforeAfterTestAttribute
  • src/Akka.Hosting.TestKit/TestKit.cs — Apply [HostingCleanAmbientContext]
  • src/Akka.Hosting.TestKit/TestKit.Shared.cs — Guard EnsureImplicitSender, remove redundant writes
  • src/Akka.Hosting.TestKit.Tests/xunit.runner.json — Enable parallel execution
  • src/Akka.Hosting.TestKit.Tests/ParallelAmbientContextSpec.csNew: 24 regression tests

Test plan

  • 24 new ParallelAmbientContextSpec tests pass with parallel collections enabled (16 implicit-sender + 8 no-implicit-sender)
  • Full test suite: 303/303 pass, 0 failures, no hangs
  • Test discovery (--list-tests) completes without hanging
  • xUnit2 variant (Akka.Hosting.TestKit.Xunit2) builds clean, unaffected
  • CI validation across all test projects

…kkadotnet#733)

Add a wrapping SynchronizationContext that preserves xUnit's
MaxConcurrencySyncContext scheduling while pinning
InternalCurrentActorCellKeeper.Current across await continuations.

A new BeforeAfterTestAttribute captures the active SC before each test,
installs the decorator, and restores it afterward. This avoids the hang
caused by replacing xUnit's SC with raw ThreadPool dispatch.

Also guards EnsureImplicitSender with a Current == null check to prevent
overwriting the cell during actor message processing, and removes two
redundant cell-pinning writes that are now handled by the attribute.

Parallel test collections enabled; 24 regression tests added.
Prevent After() from wiping the thread's SynchronizationContext when
Before() skipped because the test class is not a TestKitBase subclass.
The assembly-level CollectionBehavior attribute was overriding
xunit.runner.json and forcing all tests to run sequentially.
Copy link
Copy Markdown
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big question

Comment thread src/Akka.Hosting.TestKit/Internals/HostingCleanAmbientContextAttribute.cs Outdated
xUnit v3's runner awaits the test body between Before() and After(),
so After() can resume on a different OS thread. ThreadStatic fields
set in Before() are invisible on the new thread, causing After() to
silently skip cleanup. AsyncLocal flows via ExecutionContext across
await boundaries, ensuring correct save/restore regardless of thread.
Copy link
Copy Markdown
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Aaronontheweb added a commit to akkadotnet/akka.net that referenced this pull request May 7, 2026
…tionContext (#8182)

* [xUnit 3] Fix parallel-class implicit-sender leak in Akka.TestKit.Xunit

Under xUnit v3 parallel-class scheduling, MaxConcurrencySyncContext
dispatches test ctors and bodies onto a dedicated pool of reused threads.
TestKitBase's constructor pins InternalCurrentActorCellKeeper.Current
(a ThreadStatic) on its ctor thread; when a sibling test's body lands on
that same reused thread, the pre-await synchronous prefix of the [Fact]
reads the sibling's cell as its implicit sender, causing Tell() to use
the wrong Sender. Replies then cross ActorSystem boundaries.

Fix: add a BeforeAfterTestAttribute that runs synchronously on the test-
body thread and (a) pins Current to the running test's TestActor cell,
(b) installs ActorCellKeepingSynchronizationContext so await continuations
also re-pin the cell. After the test, Current is cleared so the reused
worker doesn't carry the cell into the next test.

Applied to Akka.TestKit.Xunit.TestKit so derived test classes (including
Akka.Hosting.TestKit downstream) get parallel-safe behavior automatically
via attribute inheritance.

ActorCellKeepingSynchronizationContext promoted to [InternalApi] public
so the attribute can install an equivalent wrapper without duplicating
the save/pin/restore logic. TBD XML docs replaced with real documentation.

Regression tests:
- ParallelAmbientContextSpec: 16 + 8 sibling test classes asserting
  implicit-sender correctness and INoImplicitSender == null across awaits.
  Marked [LocalFact] — requires xUnit.ParallelizeTestCollections=true to
  exercise the bug (disabled in the shared src/xunit.runner.json).
- AkkaCleanAmbientContextAttributeSpec: reflection guards that the
  attribute is declared on the base TestKit, is inherited by subclasses,
  and has Inherited=true in its AttributeUsage.

* Update API approval list

* Keep xUnit ambient context plumbing internal

Keep ActorCellKeepingSynchronizationContext internal via friend assembly access and run Akka.TestKit.Xunit.Tests with parallel collections so CI exercises the implicit-sender leak regression by default.

* Remove exception-driven ambient context checks

Stop swallowing NullReferenceException when reading TestActor and drop the reflection-only attribute spec in favor of the parallel behavior tests that now run in CI.

* fix: wrap outer SynchronizationContext instead of replacing it

ActorCellKeepingSynchronizationContext now accepts an optional inner
SynchronizationContext and delegates Post/Send scheduling to it while
wrapping callbacks with the cell-pinning save/restore window. When no
inner SC exists (the default), behavior is identical to before.

AkkaCleanAmbientContextAttribute captures the active SC in Before()
and passes it as the inner SC, then restores it in After(). This
preserves xUnit v3's MaxConcurrencySyncContext scheduling, which
downstream consumers like Akka.Hosting.TestKit depend on for async
IHost lifecycle. Without this, applying the attribute to Hosting's
TestKit causes test hangs.

See akkadotnet/Akka.Hosting#735 and akkadotnet/Akka.Hosting#733.

* fix: use AsyncLocal instead of ThreadStatic in BeforeAfterTestAttribute

xUnit v3's runner awaits the test body between Before() and After(),
so After() can resume on a different OS thread. ThreadStatic fields
set in Before() are invisible on the new thread, causing After() to
silently skip cleanup. AsyncLocal flows via ExecutionContext across
await boundaries, ensuring correct save/restore regardless of thread.

* fix: harden xUnit ambient context reuse

* refactor: convert AmbientContextState to record

Aligns with project style guidance (sealed classes and records as default for
immutable data carriers). Behavior unchanged — instance is only stored/retrieved
through AsyncLocal, never compared. Per review feedback on PR #8182.

---------

Co-authored-by: Gregorius Soedharmo <arkatufus@yahoo.com>
Aaronontheweb added a commit to akkadotnet/akka.net that referenced this pull request May 8, 2026
…tionContext (#8182)

* [xUnit 3] Fix parallel-class implicit-sender leak in Akka.TestKit.Xunit

Under xUnit v3 parallel-class scheduling, MaxConcurrencySyncContext
dispatches test ctors and bodies onto a dedicated pool of reused threads.
TestKitBase's constructor pins InternalCurrentActorCellKeeper.Current
(a ThreadStatic) on its ctor thread; when a sibling test's body lands on
that same reused thread, the pre-await synchronous prefix of the [Fact]
reads the sibling's cell as its implicit sender, causing Tell() to use
the wrong Sender. Replies then cross ActorSystem boundaries.

Fix: add a BeforeAfterTestAttribute that runs synchronously on the test-
body thread and (a) pins Current to the running test's TestActor cell,
(b) installs ActorCellKeepingSynchronizationContext so await continuations
also re-pin the cell. After the test, Current is cleared so the reused
worker doesn't carry the cell into the next test.

Applied to Akka.TestKit.Xunit.TestKit so derived test classes (including
Akka.Hosting.TestKit downstream) get parallel-safe behavior automatically
via attribute inheritance.

ActorCellKeepingSynchronizationContext promoted to [InternalApi] public
so the attribute can install an equivalent wrapper without duplicating
the save/pin/restore logic. TBD XML docs replaced with real documentation.

Regression tests:
- ParallelAmbientContextSpec: 16 + 8 sibling test classes asserting
  implicit-sender correctness and INoImplicitSender == null across awaits.
  Marked [LocalFact] — requires xUnit.ParallelizeTestCollections=true to
  exercise the bug (disabled in the shared src/xunit.runner.json).
- AkkaCleanAmbientContextAttributeSpec: reflection guards that the
  attribute is declared on the base TestKit, is inherited by subclasses,
  and has Inherited=true in its AttributeUsage.

* Update API approval list

* Keep xUnit ambient context plumbing internal

Keep ActorCellKeepingSynchronizationContext internal via friend assembly access and run Akka.TestKit.Xunit.Tests with parallel collections so CI exercises the implicit-sender leak regression by default.

* Remove exception-driven ambient context checks

Stop swallowing NullReferenceException when reading TestActor and drop the reflection-only attribute spec in favor of the parallel behavior tests that now run in CI.

* fix: wrap outer SynchronizationContext instead of replacing it

ActorCellKeepingSynchronizationContext now accepts an optional inner
SynchronizationContext and delegates Post/Send scheduling to it while
wrapping callbacks with the cell-pinning save/restore window. When no
inner SC exists (the default), behavior is identical to before.

AkkaCleanAmbientContextAttribute captures the active SC in Before()
and passes it as the inner SC, then restores it in After(). This
preserves xUnit v3's MaxConcurrencySyncContext scheduling, which
downstream consumers like Akka.Hosting.TestKit depend on for async
IHost lifecycle. Without this, applying the attribute to Hosting's
TestKit causes test hangs.

See akkadotnet/Akka.Hosting#735 and akkadotnet/Akka.Hosting#733.

* fix: use AsyncLocal instead of ThreadStatic in BeforeAfterTestAttribute

xUnit v3's runner awaits the test body between Before() and After(),
so After() can resume on a different OS thread. ThreadStatic fields
set in Before() are invisible on the new thread, causing After() to
silently skip cleanup. AsyncLocal flows via ExecutionContext across
await boundaries, ensuring correct save/restore regardless of thread.

* fix: harden xUnit ambient context reuse

* refactor: convert AmbientContextState to record

Aligns with project style guidance (sealed classes and records as default for
immutable data carriers). Behavior unchanged — instance is only stored/retrieved
through AsyncLocal, never compared. Per review feedback on PR #8182.

---------

Co-authored-by: Gregorius Soedharmo <arkatufus@yahoo.com>
(cherry picked from commit 515a266)
…ET 1.5.68

Akka.NET v1.5.68 (PR #8182) made ActorCellKeepingSynchronizationContext a proper
decorator that wraps the outer SynchronizationContext instead of replacing it with
raw ThreadPool dispatch, and added AkkaCleanAmbientContextAttribute to
Akka.TestKit.Xunit as a public, inheritable BeforeAfterTestAttribute.

Switch Akka.Hosting.TestKit to the upstream attribute:
- Delete custom ActorCellKeepingSynchronizationContext (now covered upstream)
- Delete custom HostingCleanAmbientContextAttribute
- Apply [AkkaCleanAmbientContext] (Akka.TestKit.Xunit.Attributes) on TestKit base
- Restore the TestEventListener registration guard in TestKit.Shared.cs to prevent
  the serialization rebuild race on CI (regression vs dev introduced on this branch)
- Bump AkkaVersion to 1.5.68
…pat-cleanup

# Conflicts:
#	Directory.Build.props
…attribute

TestKit class is now decorated with [AkkaCleanAmbientContext] from Akka.TestKit.Xunit.Attributes,
which changes the public API surface captured by the Verify snapshot test.
This was referenced May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Akka.Hosting.TestKit implicit-sender leaks across ActorSystems under xUnit v3 parallel class execution

2 participants