Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Child Processes Not Terminating with Uvicorn 0.29.0 #2289

Open
Kludex opened this issue Mar 26, 2024 Discussed in #2281 · 6 comments · May be fixed by #2314
Open

Child Processes Not Terminating with Uvicorn 0.29.0 #2289

Kludex opened this issue Mar 26, 2024 Discussed in #2281 · 6 comments · May be fixed by #2314

Comments

@Kludex
Copy link
Sponsor Member

Kludex commented Mar 26, 2024

Discussed in #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
Copy link
Sponsor Member Author

Kludex commented Mar 26, 2024

@maxfischer2781 FYI

@proever
Copy link

proever commented Mar 26, 2024

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
Copy link
Contributor

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
Copy link

sid-38 commented Apr 12, 2024

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
Copy link
Contributor

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
Copy link

sid-38 commented Apr 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants