[release/8.0] Fix h3 connection-level and stream-level abort locking - #69152
[release/8.0] Fix h3 connection-level and stream-level abort locking#69152BrennanConroy wants to merge 1 commit into
Conversation
BrennanConroy
added
feature-kestrel
area-networking
|
Hi @BrennanConroy. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. |
BrennanConroy
changed the title
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
This touches core HTTP/3 concurrency/locking in Kestrel where subtle ordering regressions can be hard to validate without targeted stress coverage.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs — Within AbortCore you're already holding _completionLock, but ApplyCompletionFlag() takes the… |
What changed in this PR
Backports the HTTP/3 stream abort locking fix to release/8.0 to avoid deadlocks between connection-level/stream-level abort paths and inline output flushing, improving server reliability under heavy abort activity.
Changes:
- Moves HTTP/3 abort side-effects (
_http3Output.Stop(), pipe completion, frame-writer abort) to run outside_completionLockto avoid_completionLock↔_dataWriterLockdeadlocks. - Adds an abort-completion barrier (
_abortCompletedTcs) and waits for in-flight abort side-effects during request finalization before disposing/pooling the stream. - Marks the stream as
Completedduring finalization so any subsequent abort becomes a no-op.
| File | Description |
|---|---|
| src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | Refactors abort locking to avoid deadlocks and adds synchronization to prevent late abort side-effects from impacting pooled/reused stream transports. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| var (oldState, newState) = ApplyCompletionFlag(StreamCompletionFlags.Aborted); | ||
|
|
||
| if (oldState == newState) | ||
| { | ||
| return; |
|
Hi @BrennanConroy. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed. |
Backport of #68102 to release/8.0
/cc @BrennanConroy @cincuranet
Fix h3 connection-level and stream-level abort locking
Description
Improve server reliability when using HTTP/3.
Fixes #68101
Customer Impact
Threads can get blocked resulting in more threads being created.
Regression?
Risk
Follows similar pattern already implemented in HTTP/2
Verification
Packaging changes reviewed?