Skip to content

Fix async enumerable source disposal ordering - #8265

Merged
Aaronontheweb merged 6 commits into
akkadotnet:devfrom
orange-dot:issue-7381-async-enumerable-repro
Jun 22, 2026
Merged

Fix async enumerable source disposal ordering#8265
Aaronontheweb merged 6 commits into
akkadotnet:devfrom
orange-dot:issue-7381-async-enumerable-repro

Conversation

@orange-dot

@orange-dot orange-dot commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes #7381.

Related: #6280, #6903.

Historical context: #6290, #6935.

  • Defer Source.From(IAsyncEnumerable<T>) cleanup until any pending MoveNextAsync() task has completed.
  • Keep the stage-owned cancellation token source alive until cleanup finishes.
  • Add regression coverage for overlapping MoveNextAsync() / DisposeAsync(), cancellation token source lifetime, deferred-cleanup reference release, and bounded cancel churn.
  • Add a release note for the observable cleanup semantics fix.

Risk / tradeoff

Cleanup intentionally waits for the in-flight MoveNextAsync() without a timeout. A non-cooperative async enumerator that ignores cancellation and never returns can therefore keep cleanup waiting and retain that materialization.

This is deliberate: applying a bounded wait and then calling DisposeAsync() would reintroduce the same overlapping MoveNextAsync() / DisposeAsync() race this PR fixes.

Validation

  • git diff --check origin/dev..HEAD - clean
  • dotnet test src/core/Akka.Streams.Tests/Akka.Streams.Tests.csproj -c Release --no-restore --filter "FullyQualifiedName~AsyncEnumerableSource_Does_Not_Dispose_Enumerator_While_MoveNextAsync_Is_In_Flight|FullyQualifiedName~AsyncEnumerableSource_Does_Not_Dispose_CancellationTokenSource_While_MoveNextAsync_Is_In_Flight" - 2 passed
  • dotnet test src/core/Akka.Streams.Tests/Akka.Streams.Tests.csproj -c Release --no-restore --filter "FullyQualifiedName~AsyncEnumerableSource_Releases_References_After_Cleanup|FullyQualifiedName~AsyncEnumerableSource_Disposes_Every_Enumerator_Under_Churn" - 2 passed
  • GC release test stability run - 5/5 passed
  • dotnet test src/core/Akka.Streams.Tests/Akka.Streams.Tests.csproj -c Release --no-restore --filter "FullyQualifiedName~Akka.Streams.Tests.Dsl.AsyncEnumerableSpec" - 17 passed
  • dotnet test src/core/Akka.Streams.Tests/Akka.Streams.Tests.csproj -c Release --no-restore - 1849 passed, 34 skipped, 0 failed

@orange-dot
orange-dot marked this pull request as ready for review June 14, 2026 14:45
@orange-dot
orange-dot marked this pull request as draft June 14, 2026 14:49
@orange-dot
orange-dot marked this pull request as ready for review June 14, 2026 14:50
@Aaronontheweb Aaronontheweb added this to the 1.5.70 milestone Jun 15, 2026
orange-dot and others added 5 commits June 19, 2026 17:27
@orange-dot
Merge branch 'dev' into issue-7381-async-enumerable-repro
3ab4008
@Aaronontheweb
Merge branch 'dev' into issue-7381-async-enumerable-repro
1f20227
@Aaronontheweb
Merge branch 'dev' into issue-7381-async-enumerable-repro
0124531
@Aaronontheweb
Merge branch 'dev' into issue-7381-async-enumerable-repro
a3237d1
@Aaronontheweb
test: remove non-differentiating AsyncEnumerableSource disposal tests
84e3553 
The GC/leak test (AsyncEnumerableSource_Releases_References_After_Cleanup)
and the soak/churn test (AsyncEnumerableSource_Disposes_Every_Enumerator_Under_Churn)
pass on BOTH the buggy and the fixed Ops.cs, so they provide no regression
protection for akkadotnet#7381. Verified by reverting only the Ops.cs fix: both still
pass (4/4 runs), while the two deterministic guards
(Does_Not_Dispose_Enumerator_While_MoveNextAsync_Is_In_Flight and
Does_Not_Dispose_CancellationTokenSource_While_MoveNextAsync_Is_In_Flight)
fail as expected. The churn test's CountingAsyncEnumerator never enforces the
no-overlap contract (DisposeAsync was always called in the old code too), and
the GC test only exercised general teardown, not the disposal-ordering fix.

Keep the two deterministic guards as the actual regression coverage; drop the
two non-differentiating tests and their dedicated helpers (ResourceCounter,
CountingAsyncEnumerable, CancelMidMoveThenReleaseAsync) to save CI time and
remove GC-test flake risk.
@Aaronontheweb

Aaronontheweb commented Jun 22, 2026

Copy link
Copy Markdown
Member

Pushed a commit (84e3553) that drops two of the four new tests - AsyncEnumerableSource_Releases_References_After_Cleanup and AsyncEnumerableSource_Disposes_Every_Enumerator_Under_Churn.

Both pass whether or not the fix is in place, so neither actually guards the regression. I checked it directly - reverted just the Ops.cs change and reran. The two deterministic guards (Does_Not_Dispose_Enumerator_While_MoveNextAsync_Is_In_Flight and Does_Not_Dispose_CancellationTokenSource_While_MoveNextAsync_Is_In_Flight) fail on the buggy code like they should; these other two stayed green 4/4.

The churn test can't catch it by construction - CountingAsyncEnumerator never enforces the no-overlap contract, it just counts Acquire / Release, and the old code always called DisposeAsync so the count balanced either way. The WeakReference test only exercised general graph teardown rather than the disposal ordering, and it carries GC-timing flake risk across our TFMs for no regression value in return.

The two deterministic guards are the real coverage and they stay - they park a MoveNextAsync on a gate, cancel mid-flight, and assert the ordering. That's the actual #7381 repro and it doesn't lean on timing.

The Ops.cs fix itself is solid - the overlapping MoveNextAsync / DisposeAsync race is real, and deferring cleanup until the in-flight move completes is the right call.

@orange-dot

orange-dot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, makes sense.

Aaronontheweb
Aaronontheweb approved these changes Jun 22, 2026
View reviewed changes

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread src/core/Akka.Streams/Implementation/Fusing/Ops.cs
private readonly CancellationTokenSource _completionCts;

private IAsyncEnumerator<T> _enumerator;
private Task _inFlightMove;

@Aaronontheweb Aaronontheweb Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread src/core/Akka.Streams/Implementation/Fusing/Ops.cs
{
await _enumerator.DisposeAsync();
if (inFlightMove != null)
await inFlightMove.ConfigureAwait(false);

@Aaronontheweb Aaronontheweb Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The ConfigureAwait(false) stuff is fine here because this is all running inside a detached task anyway

Comment thread src/core/Akka.Streams/Implementation/Fusing/Ops.cs
}

_ = ProcessTask();
_inFlightMove = ProcessTask();

@Aaronontheweb Aaronontheweb Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb
Aaronontheweb merged commit c619025 into akkadotnet:dev Jun 22, 2026
11 checks passed
@Aaronontheweb Aaronontheweb mentioned this pull request Jun 22, 2026
Merged
Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this pull request Jun 22, 2026
@Aaronontheweb
Backport PR akkadotnet#8265 and PR akkadotnet#8264 to v1.5
bb0081d 
- akkadotnet#8265: Fix async enumerable source disposal ordering
- akkadotnet#8264: Improve BroadcastHub high-consumer wheel performance

Cherry-picked from upstream/dev (c619025, 10b9831)
Aaronontheweb added a commit that referenced this pull request Jun 22, 2026
@Aaronontheweb
Backport PR #8265 and PR #8264 to v1.5
9b56af8 
- #8265: Fix async enumerable source disposal ordering
- #8264: Improve BroadcastHub high-consumer wheel performance

Cherry-picked from upstream/dev (c619025, 10b9831)
@Aaronontheweb Aaronontheweb mentioned this pull request Jul 2, 2026
Merged
2 tasks
This was referenced Jul 3, 2026
Merged
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Jul 27, 2026
Merged
Open
This was referenced Aug 18, 2026
Closed
Merged
Merged
Closed
Closed
Closed
Merged
This was referenced Aug 27, 2026
Closed
Open
Open
Open
Open
Open
Open
Open
Open
Closed
Closed
Merged
Open
Closed
Open
Open
@dependabot dependabot Bot mentioned this pull request Sep 6, 2026
Merged
This was referenced Sep 21, 2026
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aaronontheweb Aaronontheweb Aaronontheweb approved these changes

Assignees

No one assigned

Labels

akka-streams

Projects

None yet

Milestone

1.5.70

Development

Successfully merging this pull request may close these issues.

Akka.Streams: System.NotSupportedException when disposing stage with materialized IAsyncEnumerable

2 participants

@orange-dot @Aaronontheweb