fix(terminal): close pty master before reaping to avoid event-loop deadlock#10031
Merged
kirangadhave merged 2 commits intoJul 1, 2026
Merged
Conversation
…adlock When the terminal websocket disconnects while a foreground process holds the pty slave (e.g. a coding agent or REPL), cleanup() SIGKILLed the session-leader shell and then called a blocking os.waitpid(child, 0) on the asyncio event loop. The shell cannot finish exiting while the server still holds the pty master open, so it never becomes reapable and waitpid blocks forever, freezing the entire server (websockets dead, Ctrl-C ignored), recoverable only via kill -9. Close the pty master fd before reaping so the controlling-terminal teardown can complete; the child then reaps promptly. Adds a regression test asserting os.close(fd) runs before the blocking os.waitpid(pid, 0).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
mscolnick
approved these changes
Jun 30, 2026
Contributor
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.12-dev27 |
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
3 participants
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.
What
Fixes the event-loop deadlock described in #10030. Disconnecting the terminal while a foreground process holds the pty (for example a coding agent like
claude) could freeze the entire marimo server: "Failed to connect",Ctrl-Cignored, recoverable only withkill -9.Why
_create_process_cleanup_handlerinmarimo/_server/api/endpoints/terminal.pyruns a blockingos.waitpid(child, 0)on the asyncio event loop afterSIGKILLing the shell. The shell is the pty's session leader. It cannot be reaped while the server still holds the pty master fd open and a foreground child still holds the slave, so the kernel never finishes revoking its controlling terminal, the shell never becomes a reapable zombie, andwaitpidblocks forever. That freezes the whole event loop. See #10030 for the full root cause and a deterministic reproduction.Change
Close the pty master fd before reaping. With the master closed, the controlling-terminal teardown can complete and the child is reaped within a few milliseconds, so
waitpidreturns instead of blocking.Test
Adds
test_cleanup_closes_master_before_blocking_waitpid, which assertsos.close(fd)runs before the blockingos.waitpid(pid, 0). It fails on the previous ordering and passes with this change. The existingterminal.pytest suite still passes.Scope
This PR addresses only the freeze. The separate behavior of killing the terminal shell on any browser disconnect (#7829) is out of scope.