fix: avoid shutdown hang on warmup thread#19008
fix: avoid shutdown hang on warmup thread#19008kuafou wants to merge 1 commit intosgl-project:mainfrom
Conversation
…20s before finally exit
Summary of ChangesHello @kuafou, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical server shutdown issue that occurred when the HTTP server failed to start, such as due to a port conflict. The changes ensure that the server process exits promptly instead of appearing stuck, by improving the handling of the warmup thread's lifecycle and its interaction with the application's shutdown sequence. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a server shutdown hang that occurs when the uvicorn server fails to start. The approach of setting the warmup thread as a daemon and using a timed join is correct and well-implemented. The changes are clear and directly address the issue. I have one minor suggestion to improve maintainability by replacing a magic number with a named constant.
|
/tag-run-ci-label |
Motivation
When
uvicornfails to bind (e.g., the port is already in use),the HTTP server process hangs for around 120 seconds before exiting.
The reason is:
/model_infoand may run for up to 120s.finallyblock callswarmup_thread.join(), which blocks indefinitely while waiting for the thread.uvicorn.run()returns, Python’s internalthreading._shutdown()also waits for non-daemon threads, causing another blocking point.
As a result, the process appears stuck instead of exiting promptly.
Modifications
daemon=Truewhen creating the warmup thread so interpreter shutdown does not wait for it.warmup_thread.join(timeout=5.0)This prevents indefinite blocking during shutdown and allows the process to exit quickly if startup fails.
Notes
grpc_server.pyalready contains similar explicit cleanup logic.Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ci