fix: flush console output buffer before marking cell idle#9164
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a race where buffered stdout/stderr messages can reach the frontend after a cell is marked idle (and completed-run is sent), causing console output to be cleared on the next run before the user ever sees it. It introduces an explicit flush mechanism so console output is delivered before transitioning the cell to idle.
Changes:
- Add a
FlushMarkersentinel to the console output queue and teach the background writer to flush immediately and signal completion. - Implement
flush_console()on the stream and wireThreadSafeStdout/ThreadSafeStderr.flush()to it (instead of no-ops). - Add a post-execution hook that flushes console output just before setting the cell status to idle.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
marimo/_runtime/runner/hooks_post_execution.py |
Adds a post-execution hook to flush console output before marking a cell idle. |
marimo/_messaging/types.py |
Extends the Stream interface with a default flush_console() hook point. |
marimo/_messaging/streams.py |
Implements flush_console() in ThreadSafeStream and connects stdout/stderr.flush() to it. |
marimo/_messaging/console_output_worker.py |
Adds FlushMarker and updates the buffered writer to support immediate flush + completion signaling. |
Console messages (stdout/stderr) are batched by a background thread with a 10ms buffer for performance. Without an explicit flush, messages arrive at the frontend after the cell is marked idle and completed-run is sent. A subsequent run clears the console (console=[]) before the user sees the previous output. Add a FlushMarker sentinel to the console message queue. When the buffered writer encounters it, it flushes immediately and signals the caller. Implement flush() on ThreadSafeStdout/ThreadSafeStderr (previously no-ops with a TODO) and add a _flush_console post-execution hook that runs just before _set_status_idle. Closes #4821
- Replace Optional[FlushMarker] with FlushMarker | None to fix F821 (module uses from __future__ import annotations, no typing.Optional imported). - Bound flush_console() with a timeout and skip when the buffered writer thread is dead, so callers can't hang forever on shutdown. - Wrap all console queue appends (stdout, stop) in the condition-variable lock so a FlushMarker establishes a reliable ordering boundary relative to concurrent writes. - Add tests covering FlushMarker: buffered data flushes promptly and done is set; an empty-queue marker still signals completion.
1c4a5a2 to
1426e9d
Compare
akshayka
approved these changes
Apr 24, 2026
akshayka
left a comment
Contributor
There was a problem hiding this comment.
I rebased on main and tests are green. I also tested interactively, including with mo.Threads, and performance feels unaffected. This should hopefully also deflake a couple of tests. Nice!
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.3-dev44 |
manzt
added a commit
to marimo-team/marimo-lsp
that referenced
this pull request
May 15, 2026
marimo 0.23.3 added a `_flush_console` post-execution hook (marimo-team/marimo#9164) that drains the buffered console-output worker before transitioning a cell to idle. The hook runs on the same kernel thread immediately before `_set_status_idle`, so by the time an idle cell-op reaches us, all stdout/stderr cell-ops for that run have already been emitted on the same stream. We were pinned to 0.23.6, so the guarantee has been in place for a while. Drop the 50ms waits in `ExecutionRegistry` and the scratchpad stream and finalize on idle directly. The `lastRunId` stale-check the timer needed goes away too.
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.
Console messages (stdout/stderr) are batched by a background thread with
a 10ms buffer for performance. Without an explicit flush, messages arrive
at the frontend after the cell is marked idle and completed-run is sent.
A subsequent run clears the console (console=[]) before the user sees
the previous output.
Add a FlushMarker sentinel to the console message queue. When the
buffered writer encounters it, it flushes immediately and signals the
caller. Implement flush() on ThreadSafeStdout/ThreadSafeStderr (previously
no-ops with a TODO) and add a _flush_console post-execution hook that
runs just before _set_status_idle.
Closes #4821