Drain run-history writes on shutdown - #13828
Open
hysts wants to merge 3 commits into
Open
Conversation
A record is filed as a detached task so a prediction never waits on the Hub, and app.state.history_tasks collects those tasks, but nothing ever awaits them. A shutdown therefore cancels whatever is still in flight. Measured on a real launched server, with the record pinned open so it could not finish by luck. record_run awaits externalize_assets twice and then the write limiter before it reaches the Hub, and where the cancellation lands decides the outcome: pinned at the Hub write the record survives, because anyio.to_thread.run_sync cannot interrupt its worker thread; pinned anywhere earlier it is lost outright. close() returned in 0.20 s either way. The prelude is the larger window in practice, since asset externalisation fetches remote media and the limiter can hold a task while other writes drain. Enter the drain last in the lifespan stack so it unwinds first, while the rest of the app is still up, and bound it at five seconds so a shutdown cannot hang on an unreachable Hub. The regression test puts its delay in the prelude rather than in the Hub call for the reason above: a write that has already started finishes either way and would not tell the two behaviours apart.
Collaborator
🪼 branch checks and previews
Install Gradio from this PR pip install https://huggingface.co/buckets/gradio/pypi-previews/resolve/5ab37f8760e224b9e715d9e97a510d01894cf8c3/gradio-6.26.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@5ab37f8760e224b9e715d9e97a510d01894cf8c3#subdirectory=client/python"Import Gradio JS Client from this PR via CDN import { Client } from "https://huggingface.co/buckets/gradio/npm-previews/resolve/5ab37f8760e224b9e715d9e97a510d01894cf8c3/browser.js"; |
Collaborator
🦄 change detectedThis Pull Request includes changes to the following packages.
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, includes a targeted regression test for the failure mode described, and the shutdown drain is bounded to avoid hanging.
Pull request overview
Adds a bounded shutdown “drain” step to Gradio’s lifespan handling so detached run-history recording tasks have a chance to finish before the app fully tears down, preventing records from being dropped during shutdown.
Changes:
- Introduces a lifespan context manager that waits (up to 5s) for in-flight
app.state.history_tasksto complete and cancels any stragglers. - Ensures the drain is entered last in the lifespan stack so it runs first during shutdown (while other app resources are still available).
- Adds a regression test that forces delay in the pre-Hub portion of
record_runto validate the fix.
File summaries
| File | Description |
|---|---|
gradio/route_utils.py |
Adds _drain_history_tasks() and wires it into the lifespan stack to flush run-history tasks on shutdown with a timeout. |
test/test_history.py |
Adds an end-to-end regression test asserting an in-flight run-history record is persisted across lifespan shutdown. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The drain read app.state directly, guarding only the attribute on it. The lifespan tests in test_routes.py pass a SimpleNamespace stand-in that has no state at all, so the drain raised AttributeError during their shutdown.
hysts
marked this pull request as ready for review
September 5, 2026 02:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A run-history record is filed as a detached task, so a prediction never waits on the Hub.
app.state.history_taskscollects those tasks, but nothing ever awaits them, so a shutdown cancels whatever is still in flight and the record is gone.record_runawaitsexternalize_assetstwice and then the write limiter before it reaches the Hub, and where the cancellation lands decides the outcome:cancelled=Truecancelled=Trueclose()returned in 0.20 s in both cases. The first row survives only becauseanyio.to_thread.run_synccannot interrupt its worker thread, so the thread runs to completion and its result is discarded; that also means it would not survive the process exit that normally follows a shutdown. The prelude is the larger window in practice, since asset externalisation fetches remote media and the limiter can hold a task while other writes drain.The fix adds a drain to the lifespan stack, entered last so it unwinds first and the records are flushed while the rest of the app is still up. It is bounded at five seconds so a shutdown cannot hang on an unreachable Hub, and anything still pending after that is cancelled as before.
Closes: #13825
Testing
Worth one note, because the obvious version of this test does not work. The regression test puts its delay in
record_run's prelude rather than in the Hub call. A write that has already started finishes either way, for theanyioreason above, so slowing the Hub call gives a test that passes with or without the fix. Verified in both directions: it fails withAssertionError: the record was dropped by the shutdownagainstmain'sroute_utils.py, and passes with the change.test/test_history.py::TestServerSideRecording::test_a_normal_prediction_is_recordedcan flake on this branch, which is cut frommain. That is #13823, fixed by #13824, and unrelated to this change.AI Disclosure
We encourage the use of AI tooling in creating PRs, but the any non-trivial use of AI needs be disclosed. E.g. if you used Claude to write a first draft, you should mention that. Trivial tab-completion doesn't need to be disclosed. You should self-review all PRs, especially if they were generated with AI.
🎯 PRs Should Target Issues
Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.
Not adhering to this guideline will result in the PR being closed.
Testing and Formatting Your Code
PRs will only be merged if tests pass on CI. We recommend at least running the backend tests locally, please set up your Gradio environment locally and run the backed tests:
bash scripts/run_backend_tests.shPlease run these bash scripts to automatically format your code:
bash scripts/format_backend.sh, and (if you made any changes to non-Python files)bash scripts/format_frontend.sh