[release/10.0] Fix h3 connection-level and stream-level abort locking - #69150
Conversation
BrennanConroy
added
feature-kestrel
area-networking
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
AbortCore currently takes _completionLock and then calls ApplyCompletionFlag which also takes _completionLock, creating a re-entrancy risk (potential self-deadlock) in the updated locking scheme.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs — AbortCore takes _completionLock and then calls ApplyCompletionFlag(...), which also takes… |
What changed in this PR
This PR backports a reliability fix for HTTP/3 stream abort paths in Kestrel by restructuring abort locking to avoid deadlocks between stream completion state and output writer locks, and by ensuring request finalization waits for in-flight abort side-effects before pooling/reuse.
Changes:
- Move HTTP/3 abort side-effects (e.g.,
_http3Output.Stop()and transport teardown) to run outside the stream completion lock to avoid lock-order deadlocks. - Introduce an abort-completion signal (
_abortCompletedTcs) so request finalization can await abort side-effects before disposing/pooling the stream. - Mark streams as
Completedduring finalization and ensure late aborts become no-ops once completion is set.
| File | Description |
|---|---|
| src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | Refactors abort and finalization synchronization to prevent deadlocks and avoid abort side-effects racing with stream pooling. |
💡 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/10.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?