http3: fix race between new streams and GOAWAY#5522
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5522 +/- ##
==========================================
+ Coverage 84.05% 84.11% +0.06%
==========================================
Files 159 159
Lines 16334 16342 +8
==========================================
+ Hits 13729 13746 +17
+ Misses 1973 1966 -7
+ Partials 632 630 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition between opening new streams and receiving GOAWAY frames in HTTP/3 client connections. The fix ensures that streams opened concurrently are properly tracked and rejected if they exceed the GOAWAY limit.
Key changes:
- Refactored test helper functions to use an options pattern for better flexibility
- Added double-check logic in
openRequestStreamto handle concurrent stream opening and GOAWAY reception - Added comprehensive test coverage for the concurrent GOAWAY scenario
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| http3/client.go | Fixed race condition by adding double-check pattern after stream opening to verify against GOAWAY limit, with improved comments |
| http3/http3_helper_test.go | Refactored connection pair creation from separate functions to unified newConnPair with options pattern for better flexibility |
| http3/client_test.go | Updated to use new test helper API and added TestClientConnGoConcurrent to verify concurrent stream opening with GOAWAY |
| http3/server_test.go | Updated test calls to use new withServerRecorder option |
| http3/stream_test.go | Updated test calls to use new withClientRecorder option |
| http3/frames_test.go | Updated test calls to use new withDatagrams and withServerRecorder options |
| http3/conn_test.go | Updated test calls to use new options-based test helper API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix race in http3 client by canceling newly opened streams on GOAWAY and making
ClientConn.openRequestStreamrespectmaxStreamIDwith monotoniclastStreamIDtrackingUpdate
ClientConn.openRequestStreamto re-checkmaxStreamIDafter opening and cancel streams withH3_REQUEST_CANCELEDwhenStreamID >= maxStreamID; makelastStreamIDupdates monotonic under concurrency; documentmaxStreamID/lastStreamIDsemantics inClientConn; add a concurrency test and consolidate test connection helpers in http3/http3_helper_test.go.📍Where to Start
Start with
ClientConn.openRequestStreamin http3/client.go, then review the new concurrency testTestClientConnGoConcurrentin http3/client_test.go.Macroscope summarized 682071a.