Fix RuntimeError: reentrant call inside _io.BufferedWriter on Ctrl+C - #15949
Conversation
Stopping the server directly inside the signal handler can interrupt the event loop in the middle of a buffered console write, making the console output triggered by Server.stop raise 'RuntimeError: reentrant call inside <_io.BufferedWriter>' (mainly on Windows). Closes streamlit#15740
|
Thanks for contributing to Streamlit! 🎈 Please make sure you have read our Contributing Guide. You can find additional information about Streamlit development in the wiki. The review process:
We're receiving many contributions and have limited review bandwidth — please expect some delay. We appreciate your patience! 🙏 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
| Filename | Overview |
|---|---|
| lib/streamlit/web/bootstrap.py | Updates signal handling so server shutdown runs as an event-loop callback when possible. |
| lib/tests/streamlit/web/bootstrap_test.py | Adds coverage for deferred shutdown behavior inside a running asyncio loop. |
Reviews (2): Last reviewed commit: "Merge branch 'develop' into fix-sigint-r..." | Re-trigger Greptile
There was a problem hiding this comment.
Summary
This PR fixes a RuntimeError: reentrant call inside <_io.BufferedWriter> crash that occurs when Ctrl+C (SIGINT) or SIGTERM is received while the asyncio event loop is mid-write to the console. The root cause is that Server.stop() calls cli_util.print_to_cli(" Stopping...", fg="blue"), which on Windows (with colorama) can re-enter the same buffered writer that was active when the signal fired.
The fix defers server.stop() to the event loop via loop.call_soon_threadsafe() when a running loop exists, falling back to the previous inline call when no loop is available.
Reviewer consensus: Both reviewers (claude-4.6-opus-high-thinking and gpt-5.3-codex-high) independently approved this PR with no blocking issues. All assessments below reflect full agreement between reviewers.
Code Quality
The implementation is clean, minimal, and follows Python best practices for signal handling with asyncio:
asyncio.get_running_loop()correctly detects whether an event loop is active.call_soon_threadsafe()is specifically designed to be safely callable from signal handlers — it writes to an internal wakeup fd rather than touching the event loop's internal state directly.- The fallback to direct
server.stop()when no loop is running preserves testability and handles edge cases (e.g., direct invocations in test harnesses). - Comments explain the why (reentrant write risk) rather than the what, following project conventions.
- The function remains private (
_-prefixed), andasynciois already imported at module level — no new imports needed.
No maintainability concerns or pattern violations were identified by either reviewer.
Test Coverage
Both reviewers confirmed adequate coverage:
- New test
test_signal_handler_defers_stop_to_running_event_loopvalidates the core invariant:server.stop()is not called inline inside the handler when an event loop is running, and is executed on the next loop iteration viaawait asyncio.sleep(0). - Existing test
test_signal_handler_stops_servernaturally covers the no-running-loop fallback because it invokes the handler outside ofasyncio.run(), causingget_running_loop()to raiseRuntimeError.
Together these two tests cover both branches of the try/except. The new test follows the file's existing TestCase pattern, uses clear assertions, and includes a docstring.
No E2E test is needed — this is a timing-dependent race condition in process signal handling that cannot be reliably exercised in Playwright.
One reviewer optionally suggested adding a unit assertion for repeated signal delivery (idempotent stop scheduling) to further harden regression coverage. This is a reasonable follow-up but not blocking.
Backwards Compatibility
No breaking changes. Both reviewers agreed:
- The external behavior is identical: SIGINT/SIGTERM still triggers
server.stop()and graceful shutdown. - The only difference is when
stop()executes — on the next loop callback rather than inline in the signal handler — which is imperceptible to users. - The fallback preserves the old behavior when no event loop is running.
Security & Risk
No security concerns identified by either reviewer:
- No new dependencies, endpoints, or network behavior.
- No changes to authentication, session handling, or data paths.
- The change purely affects internal shutdown scheduling, reducing crash risk.
Regression risk is very low. The worst case if call_soon_threadsafe fails to execute the callback is that shutdown doesn't complete cleanly — the same failure mode as the original crash.
External test recommendation
- Recommend external_test: No
- Triggered categories: None
- Key evidence from changed files:
lib/streamlit/web/bootstrap.py: Signal handler scheduling change — purely internal process management, no network/auth/embedding/asset behavior affected.lib/tests/streamlit/web/bootstrap_test.py: Corresponding unit test for the deferral invariant.
- Suggested external test focus areas: N/A
- Confidence: High
- Assumptions and gaps: None. The change is entirely internal to process lifecycle management and does not touch any externally observable behavior. Both reviewers independently reached this conclusion.
Accessibility
No frontend changes. Not applicable.
Recommendations
- Optional (non-blocking): Consider adding a unit test for repeated signal delivery while the loop is running to verify idempotent stop scheduling. This could be a follow-up PR.
Verdict
APPROVED: Clean, minimal bugfix that correctly defers shutdown I/O out of the signal handler using the standard asyncio pattern. Both reviewers (claude-4.6-opus-high-thinking and gpt-5.3-codex-high) approved unanimously with no blocking issues. Test coverage adequately validates both code paths.
Consolidated review by claude-4.6-opus-high-thinking. Individual reviews: claude-4.6-opus-high-thinking (APPROVED), gpt-5.3-codex-high (APPROVED).
lukasmasuch
left a comment
There was a problem hiding this comment.
LGTM 👍 Thanks for the contribution
Describe your changes
Stopping the server directly inside the SIGINT/SIGTERM handler can interrupt the asyncio event loop in the middle of a buffered console write.
Server.stop()immediately printsStopping...viaclick.secho, which re-enters the same_io.BufferedWriterand trips CPython's reentrancy guard (RuntimeError: reentrant call inside <_io.BufferedWriter>), mainly on Windows where colorama flushes the console handle on every write.The signal handler now schedules
server.stopon the running event loop withcall_soon_threadsafe, so the shutdown (and its console output) runs as a normal loop callback instead of inside the signal handler. When no loop is running (e.g. direct invocation in tests), the previous inline behavior is kept.Screenshot or video (only for visual changes)
N/A
GitHub Issue Link (if applicable)
Fixes #15740
Testing Plan
test_signal_handler_defers_stop_to_running_event_looptolib/tests/streamlit/web/bootstrap_test.py, asserting the stop call does not run inline in the handler and executes on the next loop iteration. The existing signal handler tests cover the no-running-loop fallback. Fullbootstrap_test.pypasses (44 tests),ruff check/formatclean.Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
Note
Low Risk
Narrow change to shutdown scheduling on signals with a no-loop fallback; new unit test pins the deferral behavior.
Overview
Fixes Ctrl+C / SIGTERM crashes where shutdown could raise
RuntimeError: reentrant call inside <_io.BufferedWriter>(especially on Windows) because the signal handler calledserver.stop()inline while the event loop was mid console write;Server.stopprintsStopping...via the CLI._set_up_signal_handlernow usesasyncio.get_running_loop()and, when a loop exists, schedulesserver.stopwithloop.call_soon_threadsafeso shutdown and its console output run on the next loop callback. If there is no running loop, behavior is unchanged:server.stop()is still invoked directly from the handler (covers tests and non-async paths).Adds
test_signal_handler_defers_stop_to_running_event_loopto assertstopis not called inside the handler and runs afterawait asyncio.sleep(0).Reviewed by Cursor Bugbot for commit a75a075. Bugbot is set up for automated code reviews on this repo. Configure here.