Skip to content

fix(mcp): avoid nested runtime panic on stdio shutdown#2357

Open
reidliu41 wants to merge 1 commit into
Hmbown:mainfrom
reidliu41:fix/mcp-stdio-runtime-shutdown
Open

fix(mcp): avoid nested runtime panic on stdio shutdown#2357
reidliu41 wants to merge 1 commit into
Hmbown:mainfrom
reidliu41:fix/mcp-stdio-runtime-shutdown

Conversation

@reidliu41
Copy link
Copy Markdown
Contributor

@reidliu41 reidliu41 commented May 29, 2026

Summary

Fixes #2352.

codewhale-tui serve --mcp could panic when stdin closed, for example when an MCP client disconnected or when running:

./target/debug/codewhale-tui serve --mcp < /dev/null

The panic happened because the CLI entrypoint already runs inside Tokio, while the stdio MCP server creates and owns its own runtime. When stdin closed, that inner runtime was dropped from the async context, triggering Tokio's "Cannot drop a runtime in a context where blocking is not allowed" panic.

This change runs the stdio MCP server through tokio::task::block_in_place, keeping the existing synchronous MCP implementation while allowing its internal runtime to shut down safely.

Testing

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features
  • cargo test --workspace --all-features

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes

Greptile Summary

This PR fixes a panic in codewhale-tui serve --mcp that occurred when stdin was closed — the root cause being that McpServer::run creates its own tokio::Runtime and dropping it from within the outer async context violates Tokio's "no blocking drop" invariant. The fix wraps the call in tokio::task::block_in_place, which moves the current thread out of the async pool and allows the inner runtime to be dropped safely.

  • One-line change in crates/tui/src/main.rs: mcp_server::run_mcp_server(workspace) is now called inside tokio::task::block_in_place(|| ...), keeping the synchronous MCP implementation intact while resolving the shutdown panic.
  • Implicit runtime-flavor dependency: block_in_place requires a multi-threaded outer runtime; the default #[tokio::main] satisfies this, but no comment or assertion documents the assumption.

Confidence Score: 4/5

Safe to merge; the fix correctly resolves the nested-runtime drop panic with the current multi-threaded executor.

The change is minimal and well-targeted. block_in_place is the right primitive here — it lets the synchronous run_mcp_server (which creates and drops its own tokio::Runtime) execute safely from within the outer async context. The only latent concern is that block_in_place will itself panic if the outer runtime is ever changed to current_thread flavor, which is an undocumented assumption in the changed line. This doesn't affect correctness today but is worth a clarifying comment.

Only crates/tui/src/main.rs changed; the mcp_server.rs implementation is unchanged and was read for context only.

Important Files Changed

Filename Overview
crates/tui/src/main.rs Single-line fix wrapping the synchronous MCP server call in block_in_place to prevent a nested-runtime drop panic; correct approach for the multi-threaded outer runtime, but silently breaks if the outer runtime is ever switched to current_thread flavor.

Sequence Diagram

sequenceDiagram
    participant OS as OS / Shell
    participant Main as async main (Tokio multi-thread)
    participant BIP as block_in_place thread
    participant MCP as McpServer (sync)
    participant RT as Inner tokio::Runtime

    OS->>Main: launch codewhale-tui serve --mcp
    Main->>BIP: "tokio::task::block_in_place(|| ...)"
    Note over BIP: current thread exits async pool,<br/>blocking is now allowed
    BIP->>MCP: mcp_server::run_mcp_server(workspace)
    MCP->>RT: Runtime::new()
    MCP->>MCP: stdin line loop (blocking)
    MCP-->>RT: runtime.block_on(tool calls)
    OS-->>MCP: stdin closed / client disconnect
    MCP->>RT: drop Runtime (safe — not in async context)
    MCP-->>BIP: Ok(())
    BIP-->>Main: Result returned
    Main-->>OS: process exits
Loading

Fix All in Codex Fix All in Claude Code Fix All in Cursor

Reviews (1): Last reviewed commit: "fix(mcp): avoid nested runtime panic on ..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

  Run the stdio MCP server inside a blocking section when launched from the async CLI entrypoint.

  This prevents Tokio from panicking when the MCP server's internal runtime is dropped after stdin closes.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request wraps the execution of mcp_server::run_mcp_server in tokio::task::block_in_place to prevent immediate panics when dropping a nested runtime. The reviewer points out that creating a nested tokio::runtime::Runtime inside an active runtime is inefficient and recommends using tokio::runtime::Handle::current() with handle.block_on(...) instead of spawning a new runtime.

Comment thread crates/tui/src/main.rs
}
if args.mcp {
mcp_server::run_mcp_server(workspace)
tokio::task::block_in_place(|| mcp_server::run_mcp_server(workspace))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

While wrapping mcp_server::run_mcp_server in tokio::task::block_in_place prevents the immediate panic when dropping the nested runtime, creating a brand new tokio::runtime::Runtime inside mcp_server.rs while already running inside an active Tokio runtime is highly inefficient and considered an anti-pattern. It spawns a completely new thread pool and allocates redundant resources.

Instead of creating a new Runtime inside mcp_server::run_mcp_server, you can obtain a handle to the currently running runtime using tokio::runtime::Handle::current() and use handle.block_on(...) instead of runtime.block_on(...). This completely avoids the overhead of spawning a nested runtime and naturally prevents any shutdown panics without needing to manage nested runtime lifecycles.

Comment thread crates/tui/src/main.rs
Comment on lines 936 to +937
if args.mcp {
mcp_server::run_mcp_server(workspace)
tokio::task::block_in_place(|| mcp_server::run_mcp_server(workspace))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 block_in_place panics at runtime with "Cannot use blocking in a current_thread Tokio runtime" if the outer executor is ever switched to #[tokio::main(flavor = "current_thread")]. The current default multi-threaded runtime is fine, but the assumption isn't enforced anywhere. A small comment would make this dependency explicit and prevent a silent breakage if the runtime flavor changes later.

Suggested change
if args.mcp {
mcp_server::run_mcp_server(workspace)
tokio::task::block_in_place(|| mcp_server::run_mcp_server(workspace))
if args.mcp {
// block_in_place requires a multi-threaded runtime (the default for
// #[tokio::main]). It allows the inner tokio::Runtime created by
// run_mcp_server to be dropped safely without triggering the
// "Cannot drop a runtime in a context where blocking is not allowed" panic.
tokio::task::block_in_place(|| mcp_server::run_mcp_server(workspace))

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code Fix in Cursor

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.

codewhale-tui serve --mcp panics: 'Cannot drop a runtime in a context where blocking is not allowed'

1 participant