[release/9.0] Fix h3 connection-level and stream-level abort locking - #69151
[release/9.0] Fix h3 connection-level and stream-level abort locking#69151BrennanConroy wants to merge 1 commit into
Conversation
|
Hi @BrennanConroy. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The change alters HTTP/3 concurrency/locking and stream lifecycle ordering in a release backport, which warrants final human review despite appearing correct.
Review tier: Lite
Findings: None
What changed in this PR
This PR backports the HTTP/3 fix from #68102 to release/9.0 to prevent deadlocks between connection-level and stream-level abort paths by reordering locking and ensuring abort side-effects complete before stream reuse/pooling.
Changes:
- Moves HTTP/3 abort side-effects (notably
_http3Output.Stop()/ frame-writer abort) outside_completionLockto avoid lock-order inversion with the output producer’s writer lock. - Introduces an abort-completion
TaskCompletionSourceand awaits it during request finalization to prevent late abort side-effects from affecting a pooled/reused stream.
| File | Description |
|---|---|
| src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | Reworks abort locking and adds abort-completion synchronization before draining/disposing/pooling the underlying transport. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
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/9.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?