Shut down gracefully, and clean up when starting up fails - #499
Open
digitalresistor wants to merge 2 commits into
Open
Shut down gracefully, and clean up when starting up fails#499digitalresistor wants to merge 2 commits into
digitalresistor wants to merge 2 commits into
Conversation
Stopping a waitress server dropped whatever was in flight on the floor.
The main loop was torn down the moment the interrupt arrived, but the
task threads hand their output back to that loop and use the trigger to
wake it up, so anything that had not already been written to the socket
was lost. A task thread parked on outbuf_lock waiting for the loop to
drain a buffer never woke up again either, and the trigger's pipe and
the listening socket were left dangling.
Interrupting a 4MB response used to look like this:
received 327356 of 4194447 bytes
1 thread(s) still running
ResourceWarning: unclosed file <waitress.wasyncore.file_wrapper ...>
ResourceWarning: unclosed <socket.socket fd=6, ...>
ResourceWarning: unclosed <socket.socket fd=7, ...>
Shutting down now follows the steps laid out in #269: close the
listening sockets, stop reading new requests off the channels that are
still open, keep running the main loop until the requests that are
already being serviced have been answered, and only then stop the
worker threads and close the trigger. How long that may take is bounded
by a new shutdown_timeout adjustment, which also replaces the timeout
that was hardcoded in ThreadedTaskDispatcher.shutdown().
This is available as server.graceful_shutdown(), and as start()/stop()
for running a server in a background thread. stop() signals the main
loop through the trigger rather than closing things underneath it from
another thread, so it is safe to call from anywhere, including from the
WSGI application. Together with effective_port that covers what
WebTest's StopableWSGIServer had to work around.
Separately, failing to create a server leaked. The worker threads were
started before the first socket was bound, and a listener that bound
successfully was never closed if a later one failed, leaving threads
and sockets running that the caller had no way to reach. The threads
are now started only once every socket is bound, and anything created
before a failure is cleaned up again. BaseWSGIServer.close() also stops
the task dispatcher and closes the connections that are still open,
which is what MultiSocketServer.close() already did.
Fixes #480
Fixes #402
Fixes #134
Fixes #264
Fixes #290
This release adds a new shutdown_timeout adjustment and the start()/ stop() API, so it is not going out as a 3.0.x. Pinning it down now avoids the documented '.. versionadded:: 3.1.0' guessing at a version that turns out to be a different one by the time it ships.
digitalresistor
force-pushed
the
bugfix/graceful-shutdown
branch
from
August 2, 2026 23:25
a66168d to
009ba20
Compare
kgaughan
approved these changes
Aug 3, 2026
kgaughan
left a comment
Member
There was a problem hiding this comment.
I see nothing blocking merging this, though I think the error message fix is worth adding.
| servers = [] | ||
|
|
||
| try: | ||
| if adj.unix_socket and hasattr(socket, "AF_UNIX"): |
Member
There was a problem hiding this comment.
This is more something that bugged me about #215, and I know changing this would mean a change in behaviour so this PR probably isn't the place to deal with it, but shouldn't the path with Unix sockets when adj.unix_socket be mutually exclusive with adj.sockets not being empty, just like TCP sockets are? It'd simplify the logic a tad as a benefit to something more like this:
try:
if not adj.sockets:
if adj.unix_socket and hasattr(socket, "AF_UNIX"):
...
for sockinfo in adj.listen:
...
for sock in adj.sockets:
...
Comment on lines
+131
to
+132
| "There are no sockets to listen on, both 'listen' and 'sockets' " | ||
| "are empty." |
Member
There was a problem hiding this comment.
Missed unix_socket:
Suggested change
| "There are no sockets to listen on, both 'listen' and 'sockets' " | |
| "are empty." | |
| "There are no sockets to listen on: 'unix_socket', 'listen', " | |
| "and 'sockets' are all empty." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stopping a waitress server drops whatever is in flight on the floor. The main loop is torn down the moment the interrupt arrives, but the task threads hand their output back to that loop and use the trigger to wake it up — so anything not already written to the socket is lost, and a task thread parked on
outbuf_lockwaiting for the loop to drain a buffer never wakes up again.Interrupting a 4MB response on
main:Same thing on this branch: full 4,194,448 bytes, stderr clean, and it exits faster because nothing is left wedged.
This started as a fix for #480 so that #490 could land, and grew to cover the shutdown path it exposed.
Graceful shutdown
Follows the steps from #269 (comment) and @mmerickel's list in #198:
HTTPChannel.draining);Exposed as
server.graceful_shutdown().run()uses it onSIGINT/SystemExit, and a second interrupt during the drain gives up immediately, as discussed with @viktordick in #198.Closes #198, closes #264.
shutdown_timeout— closes #134New adjustment, default
5,0to tear everything down immediately. Bounds the drain, and replaces the timeout that was hardcoded inThreadedTaskDispatcher.shutdown(). Wired throughwaitress-serveand documented.start()/stop()— closes #290stop()signals the main loop through the trigger instead of closing things underneath it from another thread, so it is safe to call from anywhere — including from within the WSGI application, where it deliberately does not block (waiting there would deadlock on the request making the call). This is what WebTest'sStopableWSGIServerhas to work around today, and binding to port0and reading backeffective_portavoids the free-port race @evandrocoan raised in #290.Cleanup on startup failure — closes #480, closes #402
ThreadedTaskDispatchernobody has a reference to.BaseWSGIServer.__init__cleans up after itself — trigger, socket, map entry, dispatcher.BaseWSGIServer.close()shuts down the task dispatcher and closes the connections that are still open, matchingMultiSocketServer.close(). An ownership flag keeps one listener closing from killing aMultiSocketServer's shared thread pool.With #480 fixed, the socket leak that #490 ran into is gone — its
test_port_bind_failure_loggingno longer leaves anything behind. I left the bind logging itself out of this PR so it can land on its own.Also
pull_trigger()is a no-op once the trigger is closed. A task thread finishing as the server went away could previously write to a closed file descriptor — one that may well have been handed out to something else by then.Notes for review
black/isort/sphinx -Wclean. The new coverage includes end-to-end tests over real sockets that assert a large in-flight response survives shutdown intact. CI is green across the full matrix; the 29 new tests run everywhere except the two bind-failure ones, which are skipped on Windows becauseSO_REUSEADDRthere lets you rebind a port that is already being listened on.shutdown_timeoutis marked.. versionadded:: 3.1.0.pyproject.tomlis deliberately left at3.0.2andCHANGES.txtatUnreleased, since this repo bumps the version as a separate release step.