Skip to content

fix: drain accepted JSON-RPC requests after read EOF#3026

Open
matthewchen94 wants to merge 5 commits into
modelcontextprotocol:mainfrom
matthewchen94:fix/stdio-drain-inflight-eof-2678
Open

fix: drain accepted JSON-RPC requests after read EOF#3026
matthewchen94 wants to merge 5 commits into
modelcontextprotocol:mainfrom
matthewchen94:fix/stdio-drain-inflight-eof-2678

Conversation

@matthewchen94

Copy link
Copy Markdown

Fixes #2678.

This changes the JSON-RPC dispatcher shutdown path so read EOF does not immediately cancel request handlers that were already accepted. The dispatcher now:

  • marks the connection closed and wakes outbound waiters after read EOF,
  • keeps the write stream open briefly for accepted inbound requests to finish their response writes,
  • bounds that drain window to avoid hanging shutdown,
  • tracks active inbound requests through the response write window, not just while they are present in _in_flight.

This targets stdio servers driven with redirected stdin, where EOF can arrive immediately after the last JSON-RPC request is read while an async tool call is still mid-await. In that case the response should be flushed to stdout before shutdown.

Validation:

  • python3 -m py_compile src/mcp/shared/jsonrpc_dispatcher.py tests/shared/test_jsonrpc_dispatcher.py passed on the patched files.
  • Added a dispatcher-level regression test that accepts a request, closes the read side, and asserts the response is still written.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/mcp/shared/jsonrpc_dispatcher.py Outdated
@matthewchen94

Copy link
Copy Markdown
Author

Updated this PR to address the review and failed CI:\n\n- Scoped EOF draining behind an explicit dispatcher opt-in so default transport-close semantics still cancel in-flight handlers immediately.\n- Enabled that opt-in only for stdio read streams, where redirected stdin can EOF after the final accepted request while stdout still needs to flush the response.\n- Tracked the transport_builder rejection path in the drain counter, so spawned INTERNAL_ERROR responses are not cancelled before reaching the peer.\n- Merged latest main and added regression coverage for both accepted handler responses and builder-rejection responses.\n\nAll CI checks are green on f4a8fc9.

Rebuilds the PR on current main while preserving newly-added runner cache-hint handling. Keeps the stdio-only EOF drain behavior and regression coverage for accepted handler responses and transport-builder rejection responses.
@matthewchen94

Copy link
Copy Markdown
Author

Updated this PR onto latest main with a merge commit, without force-pushing.

What changed in the refresh:

  • rebuilt the stdio EOF-drain change on current main
  • preserved the new runner cache-hint handling that landed after the earlier PR head
  • kept EOF draining scoped to stdio/read streams via drain_inbound_on_read_eof
  • propagated the opt-in through both current dispatcher construction paths
  • kept regression coverage for both accepted handler responses and transport-builder rejection responses

Local validation on the rebuilt files:

python3 -m py_compile \
  src/mcp/shared/jsonrpc_dispatcher.py \
  src/mcp/server/runner.py \
  src/mcp/server/stdio.py \
  src/mcp/shared/_context_streams.py \
  tests/shared/test_jsonrpc_dispatcher.py

The PR is mergeable again; CI is rerunning on 14ed0cb.

@matthewchen94

Copy link
Copy Markdown
Author

Quick maintainer ping on this one.

The PR is now mergeable and the refreshed head 14ed0cb has the full required matrix green:

  • pre-commit
  • server/client conformance
  • Python 3.10–3.14 test matrix on Ubuntu/Windows
  • docs/readme snippets
  • all-green

The refresh was done as a merge commit on top of the previous PR head, not a force-push, and it preserves the newer runner cache-hint changes from current main.

I don't plan to change this further unless there is specific review feedback, because the current patch is scoped to the stdio EOF-drain behavior and CI is clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FastMCP/stdio: in-flight tool responses dropped on stdin EOF when input is bash-redirected from a file

1 participant