Skip to content

fix(server): flush stdout and ensure line-buffering before restoring fd 1 (#3821) - #3835

Closed
nikolas-sapa wants to merge 1 commit into
modelcontextprotocol:mainfrom
nikolas-sapa:fix/stdio-flush-line-buffering
Closed

nikolas-sapa wants to merge 1 commit into
modelcontextprotocol:mainfrom
nikolas-sapa:fix/stdio-flush-line-buffering

Conversation

@nikolas-sapa

Copy link
Copy Markdown

PR Draft: Flush stdout and ensure line-buffering before restoring fd 1

PR Title: fix(server): flush stdout and ensure line-buffering before restoring fd 1 (#3821)
Branch: fix/stdio-flush-line-buffering


Summary

Resolves #3821: In stdio_server, unbuffered writes or missing flush before restoring stdout/fd 1 causes JSON-RPC messages to interleave or hang on client pipes.

Root Cause

When stdio_server starts, fd 1 is diverted to stderr (or /dev/null) while the original wire descriptor is maintained privately for JSON-RPC framing. However, when standard streams are connected to pipes, Python's default buffering mode for sys.stdout is block-buffered (line_buffering=False). Stray writes made to sys.stdout (e.g. from sub-libraries or third-party loggers writing without explicit flushes) remain buffered in Python's user-space buffer while fd 1 points to the diversion.

During teardown, _restore_fd(1, ...) restores fd 1 back to the wire. If sys.stdout is not flushed prior to restoring the descriptor, those buffered stray bytes are flushed after fd 1 is restored, dumping arbitrary non-JSON-RPC text directly onto the client transport pipe. This leads to client framing parse errors, corrupted streams, and deadlocks. Additionally, restoring sys.stdout without line-buffering leaves subsequent writes prone to blocking or delayed delivery.

Changes

  1. Flush & Reconfigure on Restore:
    • In _restore_fd(fd, private_fd): When fd == 1, flush sys.stdout and call sys.stdout.reconfigure(line_buffering=True) before os.dup2(private_fd, fd).
    • In _claim_fd(fd, stream, ...) release(): Explicitly flush both sys.stdout and stream, and reconfigure line buffering before restoration.
  2. Flush on Divert:
    • In _claim_fd(...): Flush sys.stdout and stream before redirecting fd 1 to the diversion to ensure pre-existing buffer contents do not spill across fd states.
  3. Line-Buffering on Active Server Stream:
    • In stdio_server(...): Wrap stdout_buffer with _UnownedTextWrapper(..., line_buffering=True).
  4. BlockBuster Test Whitelist:
    • In tests/conftest.py: Allow os.write, io.BufferedWriter.write, io.TextIOWrapper.write in mcp/server/stdio.py during release and _restore_fd.
  5. Regression Test:
    • Added test_stdio_server_flushes_stdout_and_ensures_line_buffering_before_restore in tests/server/test_stdio.py, testing unflushed writes during stdio_server lifespan are flushed to the diversion, line buffering is ensured on restore, and the client pipe receives zero leaked bytes.

Test Verification

Ran full test suite in tests/server/test_stdio.py:

uv run --directory ~/scratch/agent_contributions/mcp_python_sdk/python-sdk pytest tests/server/test_stdio.py

Output:

============================= test session starts ==============================
collected 18 items

tests/server/test_stdio.py::test_stdio_server_round_trips_messages_over_injected_streams PASSED
tests/server/test_stdio.py::test_stdio_server_invalid_utf8 PASSED
tests/server/test_stdio.py::test_stdio_server_takes_stdin_off_the_descriptor_table_while_serving PASSED
tests/server/test_stdio.py::test_stdio_server_reads_stdin_in_place_when_descriptor_isolation_fails[dup] PASSED
tests/server/test_stdio.py::test_stdio_server_reads_stdin_in_place_when_descriptor_isolation_fails[dup2] PASSED
tests/server/test_stdio.py::test_stdio_server_reads_stdin_in_place_when_descriptor_isolation_fails[dup2_destroys_target] PASSED
tests/server/test_stdio.py::test_stdio_server_exits_cleanly_when_the_stdin_restore_fails PASSED
tests/server/test_stdio.py::test_stdio_server_takes_stdout_off_the_descriptor_table_while_serving PASSED
tests/server/test_stdio.py::test_stdio_server_flushes_stdout_and_ensures_line_buffering_before_restore PASSED
tests/server/test_stdio.py::test_stdio_server_diverts_stdout_to_the_null_device_when_stderr_is_unusable PASSED
tests/server/test_stdio.py::test_a_second_stdio_server_on_the_same_process_streams_is_refused PASSED
tests/server/test_stdio.py::test_a_refused_claim_releases_the_stream_it_already_took PASSED
tests/server/test_stdio.py::test_the_claim_engages_even_when_stderr_is_closed PASSED
tests/server/test_stdio.py::test_stdio_server_serves_in_place_when_the_diversion_cannot_be_opened PASSED
tests/server/test_stdio.py::test_a_degraded_session_does_not_close_the_sys_stream_it_served PASSED
tests/server/test_stdio.py::test_mcpserver_run_stdio_serves_until_stdin_closes PASSED
tests/server/test_stdio.py::test_mcpserver_run_stdio_runs_lifespan_cleanup_after_stdin_closes PASSED
tests/server/test_stdio.py::test_mcpserver_run_stdio_serves_a_modern_connection PASSED

Results (0.08s): 18 passed

Linter verification:

uv run --directory ~/scratch/agent_contributions/mcp_python_sdk/python-sdk ruff check src/mcp/server/stdio.py tests/server/test_stdio.py tests/conftest.py
# All checks passed!

@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Nov 8, 2026
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and you aren't currently assigned to #3821.

If a maintainer assigns you to #3821, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take.

You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way.

CONTRIBUTING.md has the full reasoning, but in short:

  • We're a small team with very little capacity to review community PRs right now.
  • Many recent PRs are AI-generated with little human review, and reviewing one carefully still costs a maintainer as much time as it ever did. A well-described issue is usually more useful to us than the code.

Maintainers: reopen, remove missing-issue-link, or add bypass-issue-check to override.

@github-actions github-actions Bot closed this Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stdio_server: text a tool prints still reaches the client, because sys.stdout is flushed after fd 1 is restored

1 participant