Repository navigation
fix: implement IAsyncLifetime on Akka.TestKit.Xunit (v3) to stop ActorSystem leaks (#8191) #8217
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
3e2499c
376cfd8
c142966
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,73 @@ | ||
| // ----------------------------------------------------------------------- | ||
| // <copyright file="Bugfix8191Spec.cs" company="Akka.NET Project"> | ||
| // Copyright (C) 2009-2022 Lightbend Inc. <http://www.lightbend.com> | ||
| // Copyright (C) 2013-2026 .NET Foundation <https://github.com/akkadotnet/akka.net> | ||
| // </copyright> | ||
| // ----------------------------------------------------------------------- | ||
|
|
||
| using System; | ||
| using System.Threading.Tasks; | ||
| using Xunit; | ||
|
|
||
| namespace Akka.TestKit.Xunit.Tests; | ||
|
|
||
| /// <summary> | ||
| /// Regression tests for https://github.com/akkadotnet/akka.net/issues/8191 | ||
| /// | ||
| /// xUnit v3 tears a test class instance down via <see cref="IAsyncDisposable.DisposeAsync"/> | ||
| /// in preference to <see cref="IDisposable.Dispose"/> whenever the type implements both. | ||
| /// <see cref="TestKit"/> must therefore drive its dispose chain — and shut the | ||
| /// <c>ActorSystem</c> down — from <c>DisposeAsync</c>, and a derived <c>DisposeAsync</c> | ||
| /// override must be able to chain to <c>base.DisposeAsync()</c>. Before the fix the | ||
| /// <c>ActorSystem</c> was silently leaked once a derived spec implemented | ||
| /// <c>IAsyncLifetime</c>. | ||
| /// </summary> | ||
| public class Bugfix8191Spec | ||
| { | ||
| private sealed class TrackingTestKit : TestKit | ||
| { | ||
| public bool AfterAllRan { get; private set; } | ||
|
|
||
| protected override void AfterAll() | ||
| { | ||
| AfterAllRan = true; | ||
| base.AfterAll(); | ||
| } | ||
| } | ||
|
|
||
| private sealed class AsyncTeardownTestKit : TestKit | ||
| { | ||
| public bool DisposeAsyncOverrideRan { get; private set; } | ||
|
|
||
| public override async ValueTask DisposeAsync() | ||
| { | ||
| DisposeAsyncOverrideRan = true; | ||
| await base.DisposeAsync(); | ||
| } | ||
| } | ||
|
|
||
| [Fact(DisplayName = "TestKit.DisposeAsync should run the dispose chain and shut down the ActorSystem")] | ||
| public async Task Should_run_dispose_chain_and_shut_down_system_When_disposed_via_DisposeAsync() | ||
| { | ||
| var testKit = new TrackingTestKit(); | ||
| var system = testKit.Sys; | ||
|
|
||
| // Exercise the exact path xUnit v3 uses to tear down a test class instance. | ||
| await ((IAsyncDisposable)testKit).DisposeAsync(); | ||
|
|
||
| Assert.True(testKit.AfterAllRan, "AfterAll() should run as part of the DisposeAsync chain"); | ||
| Assert.True(system.WhenTerminated.IsCompleted, "the ActorSystem should be shut down"); | ||
| } | ||
|
|
||
| [Fact(DisplayName = "A derived DisposeAsync override chaining to base should shut down the ActorSystem")] | ||
| public async Task Should_shut_down_system_When_derived_DisposeAsync_chains_to_base() | ||
| { | ||
| var testKit = new AsyncTeardownTestKit(); | ||
| var system = testKit.Sys; | ||
|
|
||
| await ((IAsyncDisposable)testKit).DisposeAsync(); | ||
|
|
||
| Assert.True(testKit.DisposeAsyncOverrideRan, "the derived DisposeAsync override should run"); | ||
| Assert.True(system.WhenTerminated.IsCompleted, "base.DisposeAsync() should shut the ActorSystem down"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| //----------------------------------------------------------------------- | ||
|
|
||
| using System; | ||
| using System.Threading.Tasks; | ||
| using Akka.Actor; | ||
| using Akka.Actor.Internal; | ||
| using Akka.Actor.Setup; | ||
|
|
@@ -22,7 +23,7 @@ namespace Akka.TestKit.Xunit; | |
| /// as its testing framework. | ||
| /// </summary> | ||
| [AkkaCleanAmbientContext] | ||
| public class TestKit : TestKitBase, IDisposable | ||
| public class TestKit : TestKitBase, IDisposable, IAsyncLifetime | ||
| { | ||
| private class PrefixedOutput : ITestOutputHelper | ||
| { | ||
|
|
@@ -84,6 +85,7 @@ public void WriteLine(string format, params object[] args) | |
|
|
||
| private bool _disposed; | ||
| private bool _disposing; | ||
| private bool _disposingAsync; | ||
|
|
||
| /// <summary> | ||
| /// <para> | ||
|
|
@@ -249,6 +251,14 @@ protected void InitializeLogger(ActorSystem system, string prefix) | |
| logger.Tell(new InitializeLogger(system.EventStream), ActorRefs.NoSender); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// xUnit lifecycle hook, invoked once before the test method runs. The default | ||
| /// implementation does nothing. Override to perform asynchronous test setup, and | ||
| /// call <c>await base.InitializeAsync()</c> from your override. | ||
| /// </summary> | ||
| public virtual ValueTask InitializeAsync() | ||
|
Member
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. so we have to do some sync-over-async stuff in the Initialize / Dispose methods in the TestKit today (such as terminating the
Member
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. Done in c142966. Added
The constructor side is still synchronous — the Heads-up: this touches |
||
| => default; | ||
|
|
||
| /// <summary> | ||
| /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. | ||
| /// </summary> | ||
|
|
@@ -270,7 +280,10 @@ protected virtual void Dispose(bool disposing) | |
| } | ||
| finally | ||
| { | ||
| Shutdown(); | ||
| // DisposeAsync() terminates the ActorSystem asynchronously and sets this flag, | ||
| // so we don't also block here with a synchronous Shutdown(). | ||
| if (!_disposingAsync) | ||
| Shutdown(); | ||
| _disposed = true; | ||
| } | ||
| } | ||
|
|
@@ -279,4 +292,37 @@ public void Dispose() | |
| { | ||
| Dispose(true); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// xUnit lifecycle hook, invoked once after the test method completes. The default | ||
| /// implementation runs the synchronous dispose chain (<see cref="Dispose(bool)"/> and | ||
| /// therefore <see cref="AfterAll"/>) and then asynchronously terminates the | ||
| /// <see cref="ActorSystem"/>, without blocking the calling thread. | ||
| /// <para> | ||
| /// Override this for asynchronous teardown, and always call <c>await base.DisposeAsync()</c> | ||
| /// from your override. xUnit v3 invokes <see cref="System.IAsyncDisposable.DisposeAsync"/> in | ||
| /// preference to <see cref="IDisposable.Dispose"/> for any type that implements both | ||
| /// interfaces, so an override that does not chain to the base will skip shutdown and leak | ||
| /// the <see cref="ActorSystem"/>. | ||
| /// </para> | ||
| /// </summary> | ||
| public virtual async ValueTask DisposeAsync() | ||
| { | ||
| if (_disposing || _disposed) | ||
| return; | ||
|
|
||
| // Run the synchronous dispose chain — AfterAll() plus any overridden Dispose(bool) — | ||
| // but suppress its blocking Shutdown() call; the ActorSystem is terminated | ||
| // asynchronously below instead. The finally guarantees shutdown still runs even if | ||
| // AfterAll() throws, matching the synchronous Dispose() behavior. | ||
| _disposingAsync = true; | ||
| try | ||
| { | ||
| Dispose(true); | ||
| } | ||
| finally | ||
| { | ||
| await ShutdownAsync(); | ||
| } | ||
| } | ||
| } | ||
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.
this is the primary change here - ensure that the
TestKitcomports with xUnit3'sIAsyncLifetime.