fix: allow Ctrl+C to interrupt hanging lifespan#2752#2839
Conversation
hrv-dys
left a comment
There was a problem hiding this comment.
This solves a genuine pain point. If the lifespan handler hangs (broken app, stuck dependency, etc.), a second Ctrl+C should force-exit instead of also hanging.
The approach is solid — wrapping lifespan calls in _wait_safe_lifespan() with an asyncio.Event that gets set from the signal handler via call_soon_threadsafe. A few observations:
-
call_soon_threadsafeinhandle_exit— this is correct because signal handlers run in the main thread but the event loop may be running in a different context.loop.call_soon_threadsafe(self.exit_event.set)ensures the event is set safely without races. -
The
finallyblock in_wait_safe_lifespanproperly cancels whichever task didn't finish first. If the exit event fires, the lifespan task gets cancelled; otherwise the waiter is cleaned up. This prevents leaked tasks. -
One thing to consider: when the lifespan task gets cancelled mid-shutdown, the app's
lifespan.shutdownhandler may not have completed cleanup (e.g., closing database pools, flushing buffers). This is inherent to a force-exit though — the user explicitly chose to interrupt, so partial cleanup is the expected tradeoff. Worth documenting in the commit message or a comment if it isn't already. -
The
pragma: no coveron thewaiter_task.done()branch — this makes sense since it's hard to reliably trigger a second SIGINT in a test, but it's the more critical code path. If there's a way to test it (maybe by directly callingexit_event.set()instead of sending a signal), that'd strengthen confidence.
One thing this codebase has historically been careful about: lifespan shutdown failures don't automatically propagate to the server as an exit signal. This PR doesn't change that contract — it just provides an escape hatch when the lifespan call itself never returns. That's the right scope.
Tests all pass (16 lifespan + 10 main = 26/26).
Summary
reopen #2752
Checklist