Skip to content

Fix MCP server processes leaking as zombies - #54793

Merged
rtfeldman merged 4 commits into
mainfrom
SEC-267/fix-zombie-mcp-processes
May 6, 2026
Merged

Fix MCP server processes leaking as zombies#54793
rtfeldman merged 4 commits into
mainfrom
SEC-267/fix-zombie-mcp-processes

Conversation

@rtfeldman

@rtfeldman rtfeldman commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Two bugs caused MCP server child processes (e.g. npm/node for mcp-remote) to accumulate as zombie processes that were never cleaned up:

Bug 1: stop_server() only called stop() for Running servers

If a server completed initialization but was still in Starting state when stop_server() was called (a race between the init task completing and maintain_servers restarting), the client/transport/process were never released. The Arc<ContextServer> was moved into a Stopped state with its inner client still holding the transport and child process handle.

Fix: call stop() unconditionally in stop_server(). It is a safe no-op when the client has not been initialized (None).

Bug 2: kill_on_drop only killed the direct child, not the process tree

StdioTransport used a raw smol::process::Child with kill_on_drop(true), which sends SIGKILL only to the direct child process (the shell/npm wrapper). The actual MCP server (e.g. node mcp-remote) runs as a grandchild and survives the kill, getting reparented to launchd.

Fix: use util::process::Child, which already exists in the codebase for exactly this purpose. It calls setsid() via pre_exec to make the child a process group leader, and uses killpg() to terminate the entire process tree on kill. This requires passing a std::process::Command (via build_std_command) instead of a smol::process::Command (via build_smol_command), because that is what util::process::Child::spawn accepts — it needs to call pre_exec on the std::process::Command before internally converting it to smol::process::Command for async I/O.

Release Notes:

  • Fixed zombie MCP server processes accumulating over time

@rtfeldman rtfeldman self-assigned this Apr 24, 2026
@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Apr 24, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Apr 24, 2026
Two bugs caused MCP server child processes (e.g. npm/node for
mcp-remote) to accumulate as zombie processes:

1. stop_server() only called ContextServer::stop() for servers in
   Running state. If a server completed initialization but was still
   in Starting state when stopped (a race between the init task
   completing and maintain_servers restarting), the client/transport
   was never released, keeping the child process alive.

   Fix: call stop() unconditionally. It is a safe no-op when the
   client has not been initialized.

2. StdioTransport used a raw smol::process::Child with kill_on_drop,
   which only sends SIGKILL to the direct child process (the shell or
   npm wrapper). The actual MCP server (e.g. node mcp-remote) runs as
   a grandchild and survives, reparented to launchd.

   Fix: use util::process::Child, which already exists for this
   purpose. It calls setsid() via pre_exec to make the child a
   process group leader, and uses killpg() to terminate the entire
   process tree on kill.
@rtfeldman
rtfeldman force-pushed the SEC-267/fix-zombie-mcp-processes branch from 46c8e50 to 981e695 Compare April 24, 2026 14:46
@rtfeldman
rtfeldman force-pushed the SEC-267/fix-zombie-mcp-processes branch from e5d2c48 to 927f68b Compare April 24, 2026 15:06
@rtfeldman
rtfeldman marked this pull request as ready for review April 24, 2026 15:18
Comment thread crates/context_server/Cargo.toml Outdated
Comment thread crates/context_server/Cargo.toml Outdated
Co-authored-by: Richard Feldman <oss@rtfeldman.com>
@rtfeldman
rtfeldman enabled auto-merge May 4, 2026 15:39
@rtfeldman
rtfeldman added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit 29aad02 May 6, 2026
49 of 51 checks passed
@rtfeldman
rtfeldman deleted the SEC-267/fix-zombie-mcp-processes branch May 6, 2026 13:33
ubunatic pushed a commit to ubunatic/zed that referenced this pull request May 16, 2026
Two bugs caused MCP server child processes (e.g. `npm`/`node` for
`mcp-remote`) to accumulate as zombie processes that were never cleaned
up:

**Bug 1: `stop_server()` only called `stop()` for `Running` servers**

If a server completed initialization but was still in `Starting` state
when `stop_server()` was called (a race between the init task completing
and `maintain_servers` restarting), the client/transport/process were
never released. The `Arc<ContextServer>` was moved into a `Stopped`
state with its inner client still holding the transport and child
process handle.

Fix: call `stop()` unconditionally in `stop_server()`. It is a safe
no-op when the client has not been initialized (`None`).

**Bug 2: `kill_on_drop` only killed the direct child, not the process
tree**

`StdioTransport` used a raw `smol::process::Child` with
`kill_on_drop(true)`, which sends SIGKILL only to the direct child
process (the shell/`npm` wrapper). The actual MCP server (e.g. `node
mcp-remote`) runs as a grandchild and survives the kill, getting
reparented to launchd.

Fix: use `util::process::Child`, which already exists in the codebase
for exactly this purpose. It calls `setsid()` via `pre_exec` to make the
child a process group leader, and uses `killpg()` to terminate the
entire process tree on kill. This requires passing a
`std::process::Command` (via `build_std_command`) instead of a
`smol::process::Command` (via `build_smol_command`), because that is
what `util::process::Child::spawn` accepts — it needs to call `pre_exec`
on the `std::process::Command` before internally converting it to
`smol::process::Command` for async I/O.

Release Notes:

- Fixed zombie MCP server processes accumulating over time
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
Two bugs caused MCP server child processes (e.g. `npm`/`node` for
`mcp-remote`) to accumulate as zombie processes that were never cleaned
up:

**Bug 1: `stop_server()` only called `stop()` for `Running` servers**

If a server completed initialization but was still in `Starting` state
when `stop_server()` was called (a race between the init task completing
and `maintain_servers` restarting), the client/transport/process were
never released. The `Arc<ContextServer>` was moved into a `Stopped`
state with its inner client still holding the transport and child
process handle.

Fix: call `stop()` unconditionally in `stop_server()`. It is a safe
no-op when the client has not been initialized (`None`).

**Bug 2: `kill_on_drop` only killed the direct child, not the process
tree**

`StdioTransport` used a raw `smol::process::Child` with
`kill_on_drop(true)`, which sends SIGKILL only to the direct child
process (the shell/`npm` wrapper). The actual MCP server (e.g. `node
mcp-remote`) runs as a grandchild and survives the kill, getting
reparented to launchd.

Fix: use `util::process::Child`, which already exists in the codebase
for exactly this purpose. It calls `setsid()` via `pre_exec` to make the
child a process group leader, and uses `killpg()` to terminate the
entire process tree on kill. This requires passing a
`std::process::Command` (via `build_std_command`) instead of a
`smol::process::Command` (via `build_smol_command`), because that is
what `util::process::Child::spawn` accepts — it needs to call `pre_exec`
on the `std::process::Command` before internally converting it to
`smol::process::Command` for async I/O.

Release Notes:

- Fixed zombie MCP server processes accumulating over time
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
Two bugs caused MCP server child processes (e.g. `npm`/`node` for
`mcp-remote`) to accumulate as zombie processes that were never cleaned
up:

**Bug 1: `stop_server()` only called `stop()` for `Running` servers**

If a server completed initialization but was still in `Starting` state
when `stop_server()` was called (a race between the init task completing
and `maintain_servers` restarting), the client/transport/process were
never released. The `Arc<ContextServer>` was moved into a `Stopped`
state with its inner client still holding the transport and child
process handle.

Fix: call `stop()` unconditionally in `stop_server()`. It is a safe
no-op when the client has not been initialized (`None`).

**Bug 2: `kill_on_drop` only killed the direct child, not the process
tree**

`StdioTransport` used a raw `smol::process::Child` with
`kill_on_drop(true)`, which sends SIGKILL only to the direct child
process (the shell/`npm` wrapper). The actual MCP server (e.g. `node
mcp-remote`) runs as a grandchild and survives the kill, getting
reparented to launchd.

Fix: use `util::process::Child`, which already exists in the codebase
for exactly this purpose. It calls `setsid()` via `pre_exec` to make the
child a process group leader, and uses `killpg()` to terminate the
entire process tree on kill. This requires passing a
`std::process::Command` (via `build_std_command`) instead of a
`smol::process::Command` (via `build_smol_command`), because that is
what `util::process::Child::spawn` accepts — it needs to call `pre_exec`
on the `std::process::Command` before internally converting it to
`smol::process::Command` for async I/O.

Release Notes:

- Fixed zombie MCP server processes accumulating over time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants