Skip to content

fix: release pending stream ownership after local receive-buffer exhaustion - #842

Merged
SunSi12138 merged 7 commits into
devfrom
fix/local-stream-receive-overflow-owner-cleanup
Oct 11, 2026
Merged

SunSi12138 merged 7 commits into
devfrom
fix/local-stream-receive-overflow-owner-cleanup

Conversation

@SunSi12138

@SunSi12138 SunSi12138 commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fix locally exhausted stream receive buffers leaving live pending-call ownership after the consumer exits. This targets dev only; no merge, main cherry-pick, or 2.0.4 publication is part of this PR.

Root cause

In PooledAsyncStreamDispatcher<T>.DispatchAcquiredAsync, exceeding the 4096-element receive buffer used Complete(ResourceExhausted). That selected the same consumer terminal state as a genuine peer StreamComplete. DisposeAsync therefore skipped ConsumerAbandonedCallback. Closing a stream dispatch state only stops future dispatch; it does not independently remove the pending call and StreamManager route. Standalone RPC cancellation, deadlines, and connection-close cleanup retain their existing independent paths.

Fix

  • Distinguish local receive-buffer failure from genuine peer completion with an explicit terminal value, without changing the 4096-item cap or SharpLinkErrorCode.ResourceExhausted.
  • Preserve the existing completion-publication protocol (issue test: intermittent full-suite stall on many-core hosts (TUnit engine stops scheduling ~70 tests) #206 double-checked full-fence synchronization), first/second dispatch-state close ordering, active-dispatch drain, discarded-item receive-credit return, and one-shot dispose completion.
  • After local receive failure, consumer disposal invokes the existing abandonment callback once, following dispatch drain and credit return; ClientConnection then removes the pending slot, unregisters the inbound route, and emits ConsumerAbandoned Cancel for a published request. True peer completions still skip this callback.
  • Keep PooledAsyncStreamDispatcher.cs within the repository's existing source LOC baseline by extracting terminal-publication code into PooledAsyncStreamDispatcher.Terminal.cs (partial class), rather than raising thresholds.

Deterministic regression evidence (test-first)

  • RED: test-only commit 59ab11fd4cce53e8b8cf2e7d77bdebf7f3063d69 against dev base f0fee51e77a5a736469bffa837ceb8f31b1f6877: PR Fast run 38157457266 compiled, then failed exactly 3 new regression tests (2060 passed, 3 failed of 2063): actual pending/route leak, missing local-failure callback after ResourceExhausted, and local-completion/Dispose publication race.
  • Candidate: 493b9db62db7352ee35b379fde9f56fd2abb6687 (see current PR checks). Four focused cases cover early exit, actual pending/route cleanup and published-request Cancel, full credit return and ResourceExhausted, concurrent/duplicate Dispose with async callback, and real peer completion as a negative control. The published-frame case asserts credit-before-Cancel, without incorrectly imposing Request-before-WindowUpdate ordering: the send pump may prioritize WindowUpdate before Request, provided both precede Cancel.
  • Broader validation: PR Fast runs formatting, maintainability, Release build, all unit/generator/load tests, and allocation gate; PR Extended includes full integration (including generated ABI dynamic-module stream lifecycle exit modes), AOT/package/load and codec compatibility. Treat the linked checks as authoritative. A multi-hour generated-ABI soak is separate from these PR gates and is not claimed as performed here.

Review / sequencing

  1. Review and validate this PR against dev. Do not merge automatically.
  2. After explicit approval, prepare a separate targeted main patch PR for the not-yet-published 2.0.4 release. PR chore(release): promote reviewed dev fixes for 2.0.4 #804 is already merged and is not this fix's feature branch.
  3. Prior dump/ABI lifecycle context: Actions run 38148482335 (a dump reader job failed; not evidence of successful ABI validation).

@SunSi12138 SunSi12138 changed the title fix: clean pending stream ownership after local receive buffer failure fix: release pending stream ownership after local receive-buffer exhaustion Oct 11, 2026
@SunSi12138
SunSi12138 marked this pull request as ready for review October 11, 2026 17:15

SunSi12138 commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner Author

Verification result — head eb9f20cc18c8dec88b8d0e9d5f1a74af5bef7657

Current dev: the fix branch is synchronized with dev aaedfc3f80e8fc00c504a2018577218f224ec3c3 through a two-parent branch-only merge; the PR diff still contains exactly the three fix/test files. All checks below were rerun on that synchronized head and passed. The PR is open, not merged, with base dev.

Test-first RED: PR Fast 38157457266 on test-only 59ab11fd4cce53e8b8cf2e7d77bdebf7f3063d69, before the implementation fix: 2060/2063 unit tests passed, and exactly three added bug-focused tests failed (pending/route ownership, local ResourceExhausted abandonment, and local completion versus Dispose).

Fixed GREEN:

  • PR Fast 38159338206: 2063/2063 unit, 271/271 generator, 77/77 load-test cases passed; Release build, formatting, maintainability, reference-boundary and allocation checks passed.
  • PR Extended 38159360891: 454/454 integration cases passed (the full integration suite includes the generated API4 collectible dynamic-module streaming exit lifecycle cases for normal completion, cancellation before first item, cancellation mid-stream, consumer break, and service exception); Debug admission race, Release build, AOT transport/topology smoke, NuGet packaging and smoke, one-way demo, load smoke, and cross-platform codec compatibility all passed.
  • PR Package Smoke 38159338213: trim, isolated NativeAOT, package contract and NuGet smoke passed.
  • CodeQL 38159338191 and extension fault containment 38159338217: passed.

The published-request regression confirmed the outbound pump may prioritize WindowUpdate ahead of Request; the required invariant tested here is that both precede the single ConsumerAbandoned Cancel. No thresholds, deadlines, buffer caps or assertions were relaxed to suppress the original bug.

Scope: no main patch and no merge. A separate multi-hour generated-ABI soak was not executed on this PR; the generated-ABI dynamic-module lifecycle integration tests above were executed and passed. This PR is ready for human review.

@SunSi12138
SunSi12138 marked this pull request as draft October 11, 2026 17:22
@SunSi12138
SunSi12138 marked this pull request as ready for review October 11, 2026 17:22
@SunSi12138
SunSi12138 merged commit feef5f4 into dev Oct 11, 2026
29 checks passed
@SunSi12138
SunSi12138 deleted the fix/local-stream-receive-overflow-owner-cleanup branch October 11, 2026 17:48
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.

1 participant