http3: close QUIC listeners created by Server on graceful shutdown#5101
http3: close QUIC listeners created by Server on graceful shutdown#5101marten-seemann merged 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves graceful shutdown behavior for QUIC listeners in the HTTP/3 server by distinguishing between listeners created internally and those supplied by the application.
- Introduces a new boolean field, createdByApp, in the listener struct.
- Updates addListener calls to set the createdByApp flag based on the listener’s origin.
- Modifies the Shutdown method’s documentation and closing logic for QUIC listeners.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5101 +/- ##
==========================================
- Coverage 84.24% 84.20% -0.04%
==========================================
Files 153 153
Lines 17235 17245 +10
==========================================
+ Hits 14518 14520 +2
- Misses 2165 2171 +6
- Partials 552 554 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
76b2578 to
bd3136d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes #5099 by ensuring that only the appropriate QUIC listeners are closed during a graceful shutdown.
- Introduces a new flag in the listener struct to mark if the listener was created externally.
- Updates addListener calls to pass the new flag based on the context of listener creation.
- Changes the shutdown procedure to conditionally close listeners based on the new flag.
bd3136d to
e1f0792
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates the QUIC listener shutdown behavior to ensure that only the listeners created by the Server (internally) are closed on graceful shutdown while leaving application-constructed listeners untouched.
- Introduces a new flag (createdLocally) in the listener struct.
- Modifies addListener() to accept the createdLocally parameter based on where the listener is created.
- Updates Shutdown() to close only server-created listeners.
Comments suppressed due to low confidence (1)
http3/server.go:97
- [nitpick] The comment on the 'createdLocally' field is a bit ambiguous. Since the flag is true for listeners created by the Server (which are intended to be closed on shutdown) and false for application-provided listeners, please clarify the comment to something like: 'If true, indicates that the listener was created by the Server and will be closed during graceful shutdown; if false, it was provided by the application and will remain open.'
// If this listener was constructed by the application, graceful shutdown will not close it.
20b1197 to
342b012
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes graceful shutdown behavior by ensuring that only QUIC listeners created internally by the http3.Server are closed on shutdown, while externally provided application listeners remain open.
- Added new tests in integrationtests/self/http_shutdown_test.go and http3/server_test.go to verify graceful shutdown behavior.
- Updated addListener, Close, and Shutdown methods in http3/server.go to track and act on whether a listener was created internally.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| integrationtests/self/http_shutdown_test.go | Added tests to differentiate behavior between application-created listeners and those created by the http3.Server. |
| http3/server_test.go | Introduced immediate graceful shutdown test to validate shutdown behavior. |
| http3/server.go | Updated listener handling to include a createdLocally flag and adjusted shutdown/close logic accordingly. |
342b012 to
30bdc6e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the graceful shutdown of the HTTP3 server by ensuring that QUIC listeners created locally are properly closed during shutdown, which fixes issue #5099. Key changes include:
- Updating test functions to distinguish between graceful and non‐graceful shutdown scenarios.
- Adjusting the number of repeated connection attempts in the test.
- Altering the Shutdown method in the HTTP3 server to iterate and close locally created listeners.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| integrationtests/self/http_shutdown_test.go | Refactored test functions and adjusted parameters/iterations for shutdown testing. |
| http3/server.go | Modified Shutdown to close only locally created listeners and return joined errors. |
Comments suppressed due to low confidence (1)
integrationtests/self/http_shutdown_test.go:211
- [nitpick] Consider using a consistent naming convention for test functions and helper functions. The mix of 'TestHTTP3ListenerGracefulShutdown' (a top-level test) and 'testHTTP3ListenerClosing' (a helper) might be confusing; a more distinct naming can clarify their purposes.
func TestHTTP3ListenerGracefulShutdown(t *testing.T) {
Fixes #5099.