fix(mcp): avoid nested runtime panic on stdio shutdown#2357
Conversation
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.
There was a problem hiding this comment.
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.
| } | ||
| if args.mcp { | ||
| mcp_server::run_mcp_server(workspace) | ||
| tokio::task::block_in_place(|| mcp_server::run_mcp_server(workspace)) |
There was a problem hiding this comment.
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.
| if args.mcp { | ||
| mcp_server::run_mcp_server(workspace) | ||
| tokio::task::block_in_place(|| mcp_server::run_mcp_server(workspace)) |
There was a problem hiding this comment.
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.
| 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!
Summary
Fixes #2352.
codewhale-tui serve --mcpcould panic when stdin closed, for example when an MCP client disconnected or when running:./target/debug/codewhale-tui serve --mcp < /dev/nullThe 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 -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist
Greptile Summary
This PR fixes a panic in
codewhale-tui serve --mcpthat occurred when stdin was closed — the root cause being thatMcpServer::runcreates its owntokio::Runtimeand dropping it from within the outer async context violates Tokio's "no blocking drop" invariant. The fix wraps the call intokio::task::block_in_place, which moves the current thread out of the async pool and allows the inner runtime to be dropped safely.crates/tui/src/main.rs:mcp_server::run_mcp_server(workspace)is now called insidetokio::task::block_in_place(|| ...), keeping the synchronous MCP implementation intact while resolving the shutdown panic.block_in_placerequires 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_placeis the right primitive here — it lets the synchronousrun_mcp_server(which creates and drops its owntokio::Runtime) execute safely from within the outer async context. The only latent concern is thatblock_in_placewill itself panic if the outer runtime is ever changed tocurrent_threadflavor, 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.rschanged; themcp_server.rsimplementation is unchanged and was read for context only.Important Files Changed
block_in_placeto prevent a nested-runtime drop panic; correct approach for the multi-threaded outer runtime, but silently breaks if the outer runtime is ever switched tocurrent_threadflavor.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 exitsReviews (1): Last reviewed commit: "fix(mcp): avoid nested runtime panic on ..." | Re-trigger Greptile