Repository navigation
fix: Re-dispatch SIGTERM to the handler add_signal_handler displaces - #226
selimacerbas wants to merge 1 commit into
Conversation
loop.add_signal_handler installs a dummy handler through signal.signal, so installing ours silently replaces whatever the hosting ASGI server registered. uvicorn registers Server.handle_exit in Server.capture_signals(), and _on_sigterm never stops the server, so a uvicorn-hosted endpoint stopped reacting to SIGTERM once it had served one request. The host then gets force-killed when its supervisor's stop grace period expires. Remember the displaced handler and call it from _on_sigterm after the channels have been notified. Re-dispatch happens on the event loop rather than in signal context, hence frame=None. SIG_DFL and SIG_IGN are not callable, so the standalone case is unchanged.
|
Opened #236 with a standalone repro covering both Uvicorn ( I tested this PR on top of
Hypercorn registers its SIGTERM callback via If this PR checks |
What happens
asgi_appinstalls its SIGTERM handler on the first request it serves:Underneath, in both uvloop and stdlib asyncio,
add_signal_handlerinstalls a dummy handler throughsignal.signal. That silently replaces whatever the hosting ASGI server registered. uvicorn registersServer.handle_exitinServer.capture_signals(), so once the SDK has served one request, uvicorn no longer has a SIGTERM handler._on_sigtermnotifies the active receive channels and returns. Nothing tells the host server to shut down, so a uvicorn-hosted endpoint stops responding to SIGTERM at all.What I measured
Endpoint served by uvicorn in a container with a 45 second stop grace period:
docker kill --signal=SIGTERMleaves the process running. Still serving at T+72s./proc/1/statusshows SIGTERM caught, not blocked and not ignored, so the signal does arrive.signal.getsignal(SIGTERM)returns asyncio's_sighandler_nooprather than uvicorn'shandle_exit.The change
Remember the handler that
add_signal_handlerdisplaces, then call it from_on_sigtermonce the channels have been notified. The re-dispatch runs on the event loop rather than in signal context, which is whyframeisNone.If there was no previous handler, meaning
SIG_DFLorSIG_IGN, nothing is called and the standalone case behaves as before.I kept
add_signal_handlerinstead of switching tosignal.signal, since_on_sigtermtouches asyncio objects and wants the loop-deferred dispatch thatadd_signal_handlergives it.Tests
Two tests in
tests/server.py:signal.signalgets re-dispatched after a realraise_signal(SIGTERM)Notes
I ran into this on Restate workers that were being force-killed on every deploy. We worked around it downstream by re-asserting uvicorn's handler from a poll loop. That holds, but the displacement seemed like something the SDK should deal with rather than each host.