Skip to content

fix: use ThreadPoolExecutor for MCP stdio on all platforms#615

Closed
HouMinXi wants to merge 3 commits into
tirth8205:mainfrom
HouMinXi:fix/mcp-stdio-zombie-fd-inheritance
Closed

fix: use ThreadPoolExecutor for MCP stdio on all platforms#615
HouMinXi wants to merge 3 commits into
tirth8205:mainfrom
HouMinXi:fix/mcp-stdio-zombie-fd-inheritance

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

fix: use ThreadPoolExecutor for MCP stdio on all platforms

Problem

When code-review-graph runs as an MCP stdio server,
ProcessPoolExecutor creates a fork server (fork+exec) and workers
(fork) that both inherit stdin as a Unix socket to the host. When
the host disconnects, Linux delivers EOF only when all fd references
are closed. The inherited fds keep the socket alive indefinitely --
the server's stdin read loop never terminates, producing zombie
processes (20+ after 8h, ~765MB RSS).

Existing Workaround

_select_executor_kind() already switches to ThreadPoolExecutor on
Windows when stdin is not a TTY (#46, #136). The docstring
explains the rationale: "Tree-sitter parsing in the worker releases
the GIL during native parsing, so the speedup loss for falling back
to threads is small (typically <30% on the full-build path) and the
trade is worth it."

The same logic applies to all Unix platforms. MCP stdio transport
means stdin is a socket, not a TTY, on every platform.

Fix

Remove the sys.platform == "win32" guard from the thread-fallback
condition. When stdin is not a TTY (MCP stdio mode), use
ThreadPoolExecutor on all platforms.

ThreadPoolExecutor avoids fork entirely:

  • No fork server process
  • No fd inheritance
  • No zombie accumulation
  • tree-sitter still runs in parallel (GIL released during native
    parsing)

CRG_PARSE_EXECUTOR=process/thread still overrides.

Testing

Verified end-to-end on Linux (Python 3.14, Fedora 44):

  • MCP server spawned via stdio transport
  • build_or_update_graph_tool called with full_rebuild=True
  • stdin closed to simulate host disconnect
  • Server exited within 1s, zero zombie processes

HouMinXi added 2 commits June 25, 2026 19:46
Calls inside `if False:` and `if TYPE_CHECKING:` blocks are
statically unreachable but the parser still emits CALLS edges
for them, causing false positives in dead-code detection and
impact analysis.

Add a lightweight ancestor-walk helper that detects these two
common Python idioms and sets extra["reachable"] = False on the
affected CALLS edges.  Live edges omit the key entirely so
existing graph.db files remain forward-compatible (absent key
means live).

The detection covers all Tree-sitter languages routed through
_extract_calls.  Bespoke handlers (ReScript, Dart, Elixir) and
C preprocessor guards (#if 0) are left for follow-up work.

Closes tirth8205#576

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Extend _is_in_static_dead_guard to recognize the integer literal 0
as a dead condition, alongside False and TYPE_CHECKING.  The if 0:
pattern appears in C-extension bindings and some legacy codebases.

In tree-sitter-python, 0 parses as an "integer" node with text b"0",
so the check mirrors the existing "false" node check.

Suggested-by: tirth8205 (PR tirth8205#580 review)
Signed-off-by: Minxi Hou <houminxi@gmail.com>
@tirth8205

Copy link
Copy Markdown
Owner

Thanks for the clear report. The one-line change makes sense, but it changes the default executor for every Unix MCP stdio run. Before merge, please add an automated end-to-end regression test: start the server with pipe or socket stdin, trigger parsing, close the parent side, and prove the server exits within a short timeout without leaving worker, fork-server, or zombie processes. Please also keep tests for the CRG_PARSE_EXECUTOR=process/thread overrides and TTY behavior. Once that Linux test is green, this is ready for another review.

Add end-to-end regression tests verifying the MCP server exits cleanly
when the host closes stdin, leaving no orphan or zombie processes.

Unit tests cover CRG_PARSE_EXECUTOR overrides (process/thread, case
insensitivity, whitespace, empty/invalid values) and Windows-only
TTY auto-switch behavior.

The e2e tests spawn the real MCP server with pipe stdin, send JSON-RPC
messages, close stdin, and assert clean exit within timeout with no
remaining child processes.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
@HouMinXi
HouMinXi force-pushed the fix/mcp-stdio-zombie-fd-inheritance branch from b3222a6 to b272cc4 Compare July 17, 2026 12:57
tirth8205 added a commit that referenced this pull request Jul 17, 2026
Use threads only while FastMCP runs over stdio, preserving the process-pool default for ordinary Unix CLI and CI automation. Add a real parallel-build JSON-RPC shutdown regression.

Reconciles PR #615.

Co-authored-by: Minxi Hou <houminxi@gmail.com>
tirth8205 added a commit that referenced this pull request Jul 17, 2026
Use threads only while FastMCP runs over stdio, preserving the process-pool default for ordinary Unix CLI and CI automation. Add a real parallel-build JSON-RPC shutdown regression.

Reconciles PR #615.

Co-authored-by: Minxi Hou <houminxi@gmail.com>
@tirth8205

Copy link
Copy Markdown
Owner

Thanks again, Minxi. We adopted the MCP-stdio descriptor-inheritance diagnosis and the all-platform thread-parsing remedy in PR #648, retaining your co-authorship.

The source head cannot merge directly because it no longer contains the claimed production executor change, and its regression does not exercise the real server entry point, enter the parallel parsing path, or require a clean zero exit. The replacement explicitly scopes threads to active MCP stdio, preserves process pools for ordinary Unix CLI/CI/cron use, and validates a real JSON-RPC build and EOF shutdown end to end.

PR #648 is merged into main at 11ea234. Closing this source PR as superseded.

@tirth8205 tirth8205 closed this Jul 17, 2026
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.

2 participants