-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[xUnit 3] Fix parallel-class implicit-sender leak in Akka.TestKit.Xunit #8174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ddbf563
3404dee
2e9b786
80d4dc1
0c7bb19
449b125
b52beba
1fb7272
29356e1
a5b3ad1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| //----------------------------------------------------------------------- | ||
| // <copyright file="ParallelAmbientContextSpec.cs" company="Akka.NET Project"> | ||
| // Copyright (C) 2009-2022 Lightbend Inc. <http://www.lightbend.com> | ||
| // Copyright (C) 2013-2025 .NET Foundation <https://github.com/akkadotnet/akka.net> | ||
| // </copyright> | ||
| //----------------------------------------------------------------------- | ||
|
|
||
| using System; | ||
| using System.Threading.Tasks; | ||
| using Akka.Actor; | ||
| using Akka.Actor.Internal; | ||
| using Akka.TestKit.Xunit.Attributes; | ||
| using Xunit; | ||
|
|
||
| namespace Akka.TestKit.Xunit.Tests; | ||
|
|
||
| // Regression guard for the xUnit v3 parallel-class implicit-sender leak | ||
| // (see ActorCellKeepingSynchronizationContext for the mechanism). | ||
| // | ||
| // This project provides its own xunit.runner.json with parallel collections | ||
| // enabled so CI and local runs exercise the reported failure mode by default. | ||
|
|
||
| public abstract class ParallelAmbientContextSpecBase : TestKit, IAsyncLifetime | ||
| { | ||
| // Forces the post-ctor continuation onto a different SC worker — the | ||
| // thread pollution only manifests when the body thread differs from the | ||
| // ctor thread. | ||
| public async ValueTask InitializeAsync() => await Task.Yield(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
|
|
||
| public ValueTask DisposeAsync() => default; | ||
|
|
||
| [Fact] | ||
| public async Task Implicit_sender_should_resolve_to_own_TestActor() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
| { | ||
| // Pre-await prefix — before the fix, this window read whatever cell a | ||
| // sibling ctor pinned on the body thread. | ||
| TestActor.Tell("ping"); | ||
| await ExpectMsgAsync<string>("ping", TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); | ||
| Assert.Equal(TestActor, LastSender); | ||
|
|
||
| await Task.Yield(); | ||
| TestActor.Tell("ping-after-yield"); | ||
| await ExpectMsgAsync<string>("ping-after-yield", TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); | ||
| Assert.Equal(TestActor, LastSender); | ||
| } | ||
| } | ||
|
|
||
| public class ParallelAmbientContextSpec01 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec02 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec03 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec04 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec05 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec06 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec07 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec08 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec09 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec10 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec11 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec12 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec13 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec14 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec15 : ParallelAmbientContextSpecBase { } | ||
| public class ParallelAmbientContextSpec16 : ParallelAmbientContextSpecBase { } | ||
|
|
||
| // Regression test for INoImplicitSender under the same xUnit v3 parallel | ||
| // scheduling. INoImplicitSender tests contractually have no implicit sender — | ||
| // Current must be null both at body entry (pre-await prefix) and across any | ||
| // await continuations resumed on a reused worker thread that a sibling's | ||
| // Before hook may have pinned with a non-null cell. | ||
| public abstract class ParallelNoImplicitSenderSpecBase : TestKit, IAsyncLifetime, INoImplicitSender | ||
| { | ||
| public async ValueTask InitializeAsync() => await Task.Yield(); | ||
|
|
||
| public ValueTask DisposeAsync() => default; | ||
|
|
||
| [Fact] | ||
| public async Task Current_should_be_null_both_pre_and_post_await() | ||
| { | ||
| // Invariant: body must enter with Current == null. | ||
| Assert.Null(InternalCurrentActorCellKeeper.Current); | ||
|
|
||
| // Force continuation onto a potentially polluted worker. | ||
| await Task.Yield(); | ||
| Assert.Null(InternalCurrentActorCellKeeper.Current); | ||
|
|
||
| await Task.Yield(); | ||
| Assert.Null(InternalCurrentActorCellKeeper.Current); | ||
| } | ||
| } | ||
|
|
||
| public class ParallelNoImplicitSenderSpec01 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec02 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec03 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec04 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec05 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec06 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec07 : ParallelNoImplicitSenderSpecBase { } | ||
| public class ParallelNoImplicitSenderSpec08 : ParallelNoImplicitSenderSpecBase { } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "$schema": "https://xunit.github.io/schema/current/xunit.runner.schema.json", | ||
| "longRunningTestSeconds": 60, | ||
| "parallelizeAssembly": true, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensures we run the |
||
| "parallelizeTestCollections": true | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| //----------------------------------------------------------------------- | ||
| // <copyright file="AkkaCleanAmbientContextAttribute.cs" company="Akka.NET Project"> | ||
| // Copyright (C) 2009-2022 Lightbend Inc. <http://www.lightbend.com> | ||
| // Copyright (C) 2013-2025 .NET Foundation <https://github.com/akkadotnet/akka.net> | ||
| // </copyright> | ||
| //----------------------------------------------------------------------- | ||
|
|
||
| using System; | ||
| using System.Reflection; | ||
| using System.Threading; | ||
| using Akka.Actor; | ||
| using Akka.Actor.Internal; | ||
| using Xunit; | ||
| using Xunit.v3; | ||
|
|
||
| namespace Akka.TestKit.Xunit.Attributes; | ||
|
|
||
| /// <summary> | ||
| /// Makes a test class parallel-safe under xUnit v3's parallel-collection | ||
| /// scheduling by pinning <see cref="InternalCurrentActorCellKeeper.Current"/> | ||
| /// to the running test's TestActor cell on the body thread, and installing | ||
| /// an <see cref="ActorCellKeepingSynchronizationContext"/> that re-pins the | ||
| /// cell across <c>await</c> continuations. | ||
| /// <para/> | ||
| /// Applied to <see cref="TestKit"/> (and inherited by derived test | ||
| /// classes) so users get parallel-safe behavior automatically. See | ||
| /// <see cref="ActorCellKeepingSynchronizationContext"/> for the underlying | ||
| /// mechanism and the ThreadStatic-vs-ExecutionContext rationale. | ||
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] | ||
| public sealed class AkkaCleanAmbientContextAttribute : BeforeAfterTestAttribute | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix. Since xUnit 3 can switch thread executor/context between the constructor,
|
||
| { | ||
| /// <inheritdoc/> | ||
| public override void Before(MethodInfo methodUnderTest, IXunitTest test) | ||
| { | ||
| var instance = TestContext.Current.TestClassInstance; | ||
| if (instance is not TestKitBase testKit) | ||
| return; | ||
|
|
||
| // Null cell for INoImplicitSender mirrors TestKitBase.InitializeTest: | ||
| // the Post wrapper will pin Current = null so no sibling cell leaks in. | ||
| var cell = testKit is INoImplicitSender ? null : TryGetCell(testKit); | ||
|
|
||
| InternalCurrentActorCellKeeper.Current = cell; | ||
| SynchronizationContext.SetSynchronizationContext(new ActorCellKeepingSynchronizationContext(cell)); | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public override void After(MethodInfo methodUnderTest, IXunitTest test) | ||
| { | ||
| InternalCurrentActorCellKeeper.Current = null; | ||
| } | ||
|
|
||
| private static ActorCell? TryGetCell(TestKitBase testKit) | ||
| { | ||
| return testKit.TestActor is ActorRefWithCell withCell | ||
| ? withCell.Underlying as ActorCell | ||
| : null; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| using Akka.Actor.Setup; | ||
| using Akka.Configuration; | ||
| using Akka.Event; | ||
| using Akka.TestKit.Xunit.Attributes; | ||
| using Akka.TestKit.Xunit.Internals; | ||
| using Xunit; | ||
|
|
||
|
|
@@ -20,6 +21,7 @@ namespace Akka.TestKit.Xunit; | |
| /// This class represents an Akka.NET TestKit that uses <a href="https://xunit.github.io/">xUnit</a> | ||
| /// as its testing framework. | ||
| /// </summary> | ||
| [AkkaCleanAmbientContext] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mark |
||
| public class TestKit : TestKitBase, IDisposable | ||
| { | ||
| private class PrefixedOutput : ITestOutputHelper | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are just |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| //----------------------------------------------------------------------- | ||
|
|
||
| using System.Reflection; | ||
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.InteropServices; | ||
|
|
||
| // General Information about an assembly is controlled through the following | ||
|
|
@@ -19,3 +20,4 @@ | |
|
|
||
| // The following GUID is for the ID of the typelib if this project is exposed to COM | ||
| [assembly: Guid("ae01b790-1478-4917-9299-b4855ba997cb")] | ||
| [assembly: InternalsVisibleTo("Akka.TestKit.Xunit")] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make Akka.TestKit.Xunit a friend assembly - that's preferable to exposing more things on the public API. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to use a unique, project-specific xunit settings file for this so we can enable test parallelism and see about reproducing issues like akkadotnet/Akka.Hosting#733 in CI/CD.