encode / uvicorn

An ASGI web server, for Python. 🦄
https://www.uvicorn.org/
BSD 3-Clause "New" or "Revised" License
8.57k stars 745 forks source link

Child Processes Not Terminating with Uvicorn 0.29.0 #2289

Closed Kludex closed 5 months ago

Kludex commented 7 months ago

Discussed in https://github.com/encode/uvicorn/discussions/2281

Originally posted by **karimkalimu** March 21, 2024 Description I recently updated to Uvicorn 0.29.0 and encountered a significant issue where child processes spawned by my FastAPI application are not being terminated upon a graceful shutdown. This behavior seems to be specific to the latest version of Uvicorn, as everything worked as expected prior to the update. Despite trying various configurations and explicitly using the --timeout-graceful-shutdown option, these child processes persist, becoming orphaned and reassigned to PID 1 once the main Uvicorn process is killed. Environment Uvicorn Version: 0.29.0 Operating System: CentOS 7 FastAPI Version: 0.100.0 Python Version: 3.10.13 Expected Behavior I would expect all child processes, especially those flagged with daemon=True, to terminate alongside the main process during a graceful shutdown. Actual Behavior Instead, the child processes do not terminate as anticipated. They become orphaned and are reassigned to PID 1, persisting beyond the termination of the Uvicorn process. Steps to Reproduce Launch a Uvicorn server hosting a FastAPI application that spawns child processes. Initiate a graceful shutdown of the Uvicorn server. Notice that the child processes fail to terminate and are instead reassigned to PID 1. Additional Context This issue was not present in earlier versions of Uvicorn. The observed behavior was consistent across two separate systems, both running CentOS 7. All implicated child processes were configured with daemon=True. Seeking Insights and Suggestions Has anyone else encountered this issue with the most recent update of Uvicorn? I'm curious to know if this is an identified bug, if there are any known workarounds, or if perhaps there's a configuration detail I might be overlooking. Additionally, if this behavior indicates a potential bug introduced in Uvicorn 0.29.0, I'm hoping this discussion can aid in identifying and rectifying the issue. Any insights, advice, or shared experiences would be greatly appreciated. Thank you in advance for your help!

[!IMPORTANT]

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.

Fund with Polar

Kludex commented 7 months ago

@maxfischer2781 FYI

proever commented 7 months ago

I think I'm seeing something similar? If I run my FastAPI application with something like uvicorn <module.path:app> and end it with ^C I get something like this:

...
^C2024-03-26 18:56:10.932 | INFO     | uvicorn.server:shutdown:258 - Shutting down
2024-03-26 18:56:11.033 | INFO     | uvicorn.lifespan.on:shutdown:67 - Waiting for application shutdown.
2024-03-26 18:56:13.739 | INFO     | uvicorn.lifespan.on:shutdown:76 - Application shutdown complete.
2024-03-26 18:56:13.739 | INFO     | uvicorn.server:_serve:92 - Finished server process [72530]

Aborted!

Debugging the same command with VS Code and debugpy confirms an Exception is being raised:

Exception has occurred: SystemExit
...
  File "/Users/proever/Developer/base/path/.venv/lib/python3.10/site-packages/uvicorn/server.py", line 328, in capture_signals
    signal.raise_signal(captured_signal)
KeyboardInterrupt: 

The above exception was the direct cause of the following exception:

  File "/Users/proever/Developer/base/path/.venv/lib/python3.10/site-packages/click/core.py", line 1091, in main
    raise Abort() from e
click.exceptions.Abort: 

During handling of the above exception, another exception occurred:

  File "/Users/proever/Developer/base/path/.venv/lib/python3.10/site-packages/click/core.py", line 1121, in main
    sys.exit(1)
  File "/Users/proever/Developer/base/path/.venv/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/Users/proever/Developer/base/path/.venv/lib/python3.10/site-packages/uvicorn/__main__.py", line 4, in <module>
    uvicorn.main()
  File "/Users/proever/.pyenv/versions/3.10.13/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/Users/proever/.pyenv/versions/3.10.13/lib/python3.10/runpy.py", line 196, in _run_module_as_main (Current frame)
    return _run_code(code, main_globals, None,
SystemExit: 1

But maybe it's a problem with FastAPI?

maxfischer2781 commented 7 months ago

Thanks for the ping @Kludex . The timing indeed makes this suspiciously likely to be related to the new signal handling.

I'm not sure how the change itself would lead to that behaviour, though. Is there some cleanup (of subprocesses) happening after Server.serve completes that would now be skipped by the KeyboardInterrupt?

sid-38 commented 7 months ago

I think the reason for this issue is that the capture_signals context in server.py ensure that anything passed to Server.serve runs with the modified signal handlers instead of the default ones. If a child process is created within this context, python's default behavior makes the child processes inherit the parent's signal handlers which in this case is the handle_exit signal handler. Since the child process is now running with a signal handler that does not kill the process on SIGINT or SIGTERM, KeyboardInterrupt is not able to end it. I tested this theory out a bit, and it seems to be the case in my tests. A potential fix is to modify the handle_exit signal handler to do its logic only for the main process. This could be done by comparing the pid. I tested this out as well, and it seems to fix the problem. I could create a pull request if you think it could be a valid solution.

maxfischer2781 commented 7 months ago

I'm not sure how that would be the cause. Installing the custom signal handlers is something that happened before already. The change was only to restore the original signal handlers on exit.

sid-38 commented 7 months ago

My bad, forget everything I said. I was using multiprocess fork instead of spawn for the child process and that was messing things up. With spawned child process, I'm not able to reproduce the issue.

Kludex commented 5 months ago

Fixed on 0.30.0.

karimkalimu commented 2 months ago

the problem still exist on my side -> Version: 0.30.6

Should I include any additional parameters to ensure child processes are terminated?

uvicorn.run( "__main__:app", host="0.0.0.0", workers=1, reload=False, log_level="info", interface="asgi3", )

Version: 0.28.1: UVICORN_0281

Version: 0.30.6: UVICORN_0306