[amd-clb]

Summary

Fixes the MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 drain listeners added to [WriteStream] warning that occurs when using local (stdio) MCP servers.

There are two independent sources of unbounded listener growth in StdioClientTransport, both fixed here:

Root cause

1. send() accumulated drain listeners on stdin.
   Every send() whose write() hit a full buffer added its own stdin.once('drain', resolve). Under sustained back-pressure (a server that reads slowly, or not at all) these pile up without bound and trip the default MaxListeners limit of 10.

2. close() left the start() listeners attached and never ended the stderr pass-through.

   • The stdout.on('data', …) listener kept the stream flowing, so late non-JSON output after close() still hit processReadBuffer() → JSON.parse() → SyntaxError (this is onerror and other listener not remove after client close (stdio) #780).
   • stderr.pipe(this._stderrStream) was never torn down. pipe() attaches a drain listener to the destination; if the caller pipes the transport's stderr into a shared stream (e.g. a host log, as opencode does), those listeners persist until the source ends. When a grandchild keeps the child's stderr write-end open, the natural end cascade never fires, so they leak.

Changes

packages/client/src/client/stdio.ts

• send() now shares a single drain wait instead of adding one listener per back-pressured write, so stdin never accumulates drain listeners; a successful write returns early instead of leaving a stale resolver in the wait array. It also rejects with NotConnected if the process has already exited (exitCode/signalCode set) — so a send() issued in the [exited, close-pending] window (e.g. a grandchild holding the child's stdio, delaying close) cannot register a drain waiter that only close could settle.
• New settlePendingSends() resolves any send() promises still waiting on a drain the process will no longer emit and removes the shared drain listener, so nothing lingers on a dead stream. It is called from the process exit handler (settle-on-exit — the earliest point the process is gone), the close handler (an idempotent backstop), close(), and _dispose(), so sends that were waiting at exit can never hang — even if close lags behind exit. Together with the exitCode guard in send(), the [exited, close-pending] window is closed on both sides (already-waiting sends settle; late sends reject). They resolve rather than reject: the bytes were already handed to stdin.write(), so delivery past process exit is out of scope — onclose is the authoritative "the connection is gone" signal.
• close() detaches only the specific data/error handlers installed in start() — not removeAllListeners(), so Node's own close listeners on stdout/stderr are preserved and the process close/onclose still fires — and installs no-op error handlers so a late EPIPE (a write racing the child's exit) cannot throw uncaught. The stderr pass-through is unpiped and ended only after the escalation ladder (stdin EOF → SIGTERM → SIGKILL) has run, so any stderr the child emits while dying still reaches consumers, while the pass-through is torn down deterministically — dropping a shared stream's pipe/drain listeners — once the child is gone.

packages/client/test/client/stdio.test.ts

Focused, deterministic regression tests (the first two each verified to fail on the unfixed code):

• send() under back-pressure keeps stdin drain listeners ≤ 1 and emits no MaxListenersExceededWarning, then settles all back-pressured sends on close().
• close() removes the drain listener the stderr pipe attached to the pass-through (the pipe's destination), even when a grandchild keeps the child's stderr open so pipe() never auto-cleans up. Verified to fail with the unpipe removed.
• close() removes the shared drain listener from stdin when the child never drains (the defensive removeListener in settlePendingSends()).
• The settle-on-death contract: a back-pressured send() still pending when the child dies resolves (not hangs) and onclose fires exactly once — pinned for both the explicit close() path and the child-exits-on-its-own path (settled via settle-on-exit).
• A send() issued after the process has exited but before close (the [exited, close-pending] window) rejects as not connected, and onclose still fires exactly once when close follows. Verified to fail with the exitCode guard removed.

Testing

• Verified locally (Node 22, Windows): before the fix, a child that never reads stdin accumulated 27 drain listeners and fired the warning; after the fix it stays at 1 with no warning, and all sends settle on close().
• Verified against the real e2e suite (test/e2e): transport:stdio:clean-shutdown passes, and transport:stdio:shutdown-escalation passes wherever SIGTERM is deliverable (it is not catchable on Windows by platform design, so it is exercised in CI on Linux). Full suite: 3119 passed, 141 expected-fail, no new failures.
• All 14 stdio.test.ts unit tests pass; typecheck and eslint are clean.

Related issues

• Fixes onerror and other listener not remove after client close (stdio) #780 (data listener not removed after client close)
• Fixes StdioClientTransport.close() doesn't clean up listeners, causing EventEmitter memory leak warning #3070 (MaxListenersExceededWarning on drain listeners)

[changeset-bot]

🦋 Changeset detected

Latest commit: fb08480

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/client	Patch
@modelcontextprotocol/codemod	Patch
@modelcontextprotocol/core	Patch
@modelcontextprotocol/server-legacy	Patch
@modelcontextprotocol/server	Patch
@modelcontextprotocol/core-internal	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3073

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3073

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3073

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3073

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3073

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3073

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3073

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3073

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3073

commit: fb08480

[arhancanli]

Two things from reading the diff (not run here, the Node side is from how Readable.prototype.pipe behaves as I know it):

1. The 'drain' listener is on the destination, not on stderr. src.pipe(dest) registers its drain handler on dest, so the one that accumulates with back-pressure sits on this._stderrStream. The new assertion proc.stderr!.listenerCount('drain') is 0 before the fix as well, so the test cannot distinguish fixed from unfixed for the leak named in its title. Counting drain on _stderrStream after close(), or looping start/close ~11 times and asserting no MaxListenersExceededWarning (the default limit is 10), would. The unpipe call is what actually removes that handler, and that part looks right.

2. removeAllListeners() also removes the error handlers. Pipes can still emit after close() (EPIPE from a write racing the child's exit, for example), and an error event with no listener is thrown as an uncaught exception. The test only checks that the counts reach 0, which is the state that exposes this. Removing just the data/close handlers start() attached, or leaving a no-op error handler on each stream, avoids it.

[notlakshitha]

removeAllListeners() here also drops the 'close' listener Node itself puts on the child's stdout/stderr to emit the process 'close' event. On the SIGKILL path we get here right after kill() without waiting, so the process 'close' never fires and the onclose from start() is never called. I tried it with a child that ignores SIGTERM and the event never came. Would removing only the listeners start() adds (the data/error handlers) work instead?

[amd-clb]

Fixed — removeAllListeners() is no longer used. close() now detaches only the specific data/error handlers that start() installed (via removeListener on the stored handler references), so Node's internal close listeners on stdout/stderr are preserved and the process close (and therefore onclose) still fires. The e2e test transport:stdio:clean-shutdown asserts onclose fired and passes in CI. The branch is also squashed to a single commit now (5b84f34).

[amd-clb]

Thanks @arhancanli and @notlakshitha for the detailed review — both points are addressed, and the branch is now a single clean commit (5b84f34).

1. The drain listener sits on the pipe's destination, not the source. Agreed — src.pipe(dest) registers drain on dest. The regression test now counts drain on the pass-through (_stderrStream, the destination of child.stderr.pipe(_stderrStream)): it waits until back-pressure makes pipe() attach the listener, asserts it is present before close(), then asserts it is 0 after. I verified the test fails with the unpipe removed, so it genuinely distinguishes fixed from unfixed. The earlier assertion on the source stream is gone.

2. removeAllListeners() also dropped the error handlers (and Node's close listeners). removeAllListeners() is no longer used. close() removes only the specific data/error handlers start() installed and installs no-op error handlers on each stream, so a late EPIPE (a write racing the child's exit) cannot throw as an uncaught exception, and the process close/onclose still fires.

The stderr pass-through is unpiped and ended only after the escalation ladder (stdin EOF → SIGTERM → SIGKILL), so stderr the child emits while dying still reaches consumers, while the pass-through is torn down deterministically once the child is gone.

[arhancanli]

Read the new commit; both points are covered, and the no-op error handlers plus leaving the stderr pass-through attached until the ladder finishes look right to me (not run here).

One edge in the shared drain wait: send() registers stdin.once('drain', ...) only when _pendingDrainWaiters is empty. If the process closes while that once is pending, settlePendingSends() empties the array but the once('drain') listener stays on stdin. That is harmless for a single transport, but a second start() after close() that hits back-pressure sees an empty array and registers another once on the new process's stdin, so nothing carries over. Is a transport instance meant to be restartable, or is start() once per instance? If restartable, a test that does start, saturate, close, start, saturate and asserts listenerCount('drain') stays at 1 would pin it.

[amd-clb]

Good catch @arhancanli — to answer the lifecycle question directly: start() is once per instance; the transport is not restarted in place.

• send() throws NotConnected after close() (since this._process is undefined), so no send is ever issued against a dead process.
• The _stderrStream PassThrough is created once in the constructor and end()-ed in close(). An ended PassThrough cannot be reused, so a second start() with stderr: 'pipe' would pipe into a terminated stream.
• The MCP Client reconnection model creates a new transport rather than calling start() again on the same one.

So the orphan once('drain') sits on a dead stdin that receives no further sends and is torn down — harmless in the supported lifecycle.

That said, to remove any doubt and make the drain lifecycle fully deterministic, I now store the shared drain handler so that settlePendingSends() (which runs on the process close, in close(), and in _dispose()) not only resolves the waiting senders but also removes that drain listener from the stream it was attached to — so it can never linger, even if it never fired. I added a regression test pinning it: a single back-pressured send leaves exactly one drain listener on stdin, then close() and assert it drops to 0 (it fails if the removeListener is removed).

Updated commit: 4caeb32.

[arhancanli]

Thanks for the direct lifecycle answer; "start() once per instance, new transport on reconnect" matches what I see in the Client, and storing the handler plus _drainStream so settlePendingSends() removes the listener closes the orphan case. I read 4caeb32 and did not run it.

One thing in the same function: settlePendingSends() calls the waiters' resolve, so a send() that was waiting on a drain when the process died now resolves successfully even though the message may never have reached the child. Before this change that promise would simply have hung. Is resolving the intended contract (the bytes were handed to stdin.write, delivery is the caller's problem), or would you rather reject with the NotConnected error so a caller awaiting send() can tell the difference? If resolving is intended, a one-line comment on settlePendingSends would save the next reader the same question.

[amd-clb]

Good point @arhancanli. We went with resolving, and I've documented it on settlePendingSends().

Reasoning: even in the happy path, send() does not resolve when the child consumes the message — it resolves when the bytes are handed to stdin.write()'s buffer. So "resolved" never meant "consumed by the child"; resolving the still-pending (back-pressured) sends on death is consistent with that contract. On top of that, after process exit we cannot tell whether the bytes actually made it — a drain only proves buffer→OS-pipe, not that the child read them — so rejecting risks false loss signals (the child may have read the message moments before dying), which would invite a caller to re-send and duplicate. The authoritative "the connection is gone" signal is onclose, not an individual send() rejection.

The settlePendingSends() JSDoc now states this explicitly: they resolve rather than reject because the bytes were already handed to stdin.write(), and delivery past process exit is out of scope — use onclose to learn the connection is gone.

Updated commit: 4d09680.

[arhancanli]

That reasoning holds: send() already resolves at stdin.write() hand-off, so resolving on death keeps one meaning for "resolved", and a rejection could make a caller re-send a message the child had already read. Thanks for the JSDoc line, I read 4d09680 and did not run it.

One ordering detail in the diff: in the close handler onclose fires first and settlePendingSends() runs right after, so a caller sees the connection-gone signal before its pending send() promises resolve. Is there a test that holds a back-pressured send(), kills the child, and asserts both that the promise resolves and that onclose fires exactly once? That pins the contract you just documented.

[amd-clb]

Added the test you described: a single back-pressured send() (verified the shared drain wait is pending on the full pipe), then close() kills the child, and it asserts both that the pending send() promise resolves and that onclose fires exactly once. It pins the settle-on-death contract. (As you noted, onclose does fire before the send settles, so the caller sees the connection-gone signal first — that's the ordering the test exercises.)

Updated commit: 2fc0c61.

[arhancanli]

Read 2fc0c61 (not run). The new test pins what I asked for: a single back-pressured send() is left waiting on the shared drain, close() kills the child, the promise resolves and oncloseCount is exactly 1. Asserting the count rather than just "it fired" is the useful part, since close() and the child close event are two separate paths to onclose.

One edge the test does not reach: a send() that starts after the child has died but before its close event is emitted (stdin already errored with EPIPE, _process still set). write() returns false there, the shared drain wait is registered on a stream that will never drain, and it is only released when close fires and settlePendingSends() runs. That is probably fine, since close always follows exit, but is a second send() queued behind the first in that window also settled, or is there a case where stdin.destroyed is true and no close follows (e.g. the child exits while a grandchild holds stdio open)?

[amd-clb]

Read — thanks. To answer the two questions directly:

A second (and Nth) send() in that window is settled, not just the first. settlePendingSends() doesn't settle a single waiter — it drains the entire _pendingDrainWaiters array and resolves each, then removes the one shared drain listener. Because the fix shares a single drain wait across all back-pressured sends, however many are queued in that window ride that one wait and are all resolved together when close fires. Nothing is left hanging.

No stdin.destroyed-but-no-close case on this platform. I checked it empirically rather than assume it: a child that exits while a grandchild holds its stdio (stdio: 'inherit') still has its close fire ~1 ms after exit. So close always follows exit, there's no permanent-hang path, and the window is too narrow to realistically issue a send in it anyway — and even if one were, settlePendingSends() settles it the moment close fires.

To pin the reliably-reproducible part of this, I added a test: a back-pressured send() left pending when the child exits on its own (not via close()) — so only the close handler's settlePendingSends() can settle it — asserts the promise resolves, onclose fires exactly once, and the drain listener drops to 0.

Updated commit: 1b1d71f.

[arhancanli]

Thanks, the Nth-waiter answer is clear: draining the whole _pendingDrainWaiters array under one shared drain listener settles every queued send, and I read 1b1d71f (not run) to confirm that.

On the second question I would not rely on the check as described. A child started with stdio: 'inherit' has no pipes to the parent, so its close follows exit almost immediately by construction. The transport spawns with piped stdio, and Node documents close as emitted after the process has ended and its stdio streams have closed (as opposed to exit, which fires when the process ends). If a grandchild inherits the child's stdout/stderr pipe fds and outlives it, exit fires and close waits until that grandchild closes them. I did not reproduce that against this transport, so treat it as the case to test rather than a finding.

If it holds, a send() made after exit but before close sits on the shared drain wait, and _process is still set so send() does not take the "not connected" path. Is a test with a piped child that spawns a detached grandchild sharing its pipes, then exits, feasible here? It would show whether those sends hang until the grandchild goes, and whether exit should also call settlePendingSends().

[amd-clb]

You're right that the first check was flawed, and the corrected experiment confirms it. Re-running with a genuinely piped child (stdio: ['pipe','pipe','pipe']) that spawns a stdio: 'inherit' grandchild sharing its pipes and then exits:

• On this platform (Windows): close still fires ~1 ms after exit, and the grandchild's own stdout writes never reach the parent. So here the grandchild does not hold the child's pipe fds and the [exited, close-pending] window is ~1 ms — no hang.
• On platforms where the pipe stays open (your Node-semantics read, e.g. Linux): a grandchild inheriting the child's stdout/stderr fds and outliving it would make close wait until it closes them, so the window is real there. I'm relying on the documented semantics for that platform rather than reproducing it (I only have Windows to run against).

So the answer to "should exit also call settlePendingSends()?" is yes, on all platforms. I now settle on the exit event — the earliest point the process is gone — instead of only on close. A send that is still pending at exit (or made in that window while _process is still set) now resolves immediately on exit, rather than waiting for a close that can lag behind it. The close handler's settlePendingSends() stays as an idempotent backstop, and onclose still fires there. I updated the test to pin settle-on-exit.

Updated commit: 58d25da.

[arhancanli]

Thanks for rerunning it with piped stdio, and for saying plainly that the Linux half rests on the documented semantics and was not reproduced. I read 58d25da (not run). Settling on exit closes the window for sends that were already waiting.

One case it leaves open is a send() made after exit but before close. _process is still set then, so send() does not take the "not connected" path. write() can return false on the dead stdin, a new waiter is pushed onto _pendingDrainWaiters, and nothing calls settlePendingSends() again until close. That is the same hang, only shifted to sends that arrive later. Would it be cleaner to clear _process in the exit handler too, so a late send() rejects as not connected? Or to have send() check exitCode/signalCode before it registers a waiter?

You can test it without a Linux box. Stub a child that emits exit and withholds close, call send() after that, and assert it settles or rejects instead of staying pending. Does onclose still fire exactly once when exit settles the sends and close follows later?

[amd-clb]

Good catch — that's a real gap: settling on exit only covers sends that were already waiting, not a send() issued after exit and before close, where _process is still set so the new waiter would hang until close.

I took the send()-checks-exitCode/signalCode route rather than clearing _process in the exit handler: clearing it would break close()'s stderr pass-through teardown in the case where the child exits on its own and the caller later calls close() (that teardown lives under the if (this._process) guard). Instead send() now rejects with NotConnected when the process has already exited — matching the post-close() path — so a late send() never registers a drain waiter at all. The exit handler still settles the sends that were waiting, so together the window is closed on both sides (already-waiting sends settle; late sends reject).

The guard uses exitCode != null || signalCode != null (loose null check) so a missing/unset exit status is treated as "not yet exited", not a dead process — which also keeps the cross-spawn unit mock (which doesn't stub exitCode) working.

I also added the test you sketched: it holds the [exited, close-pending] state (exitCode set, close withheld), calls send(), and asserts it rejects as not connected rather than staying pending — verified it fails with the guard removed — then lets close fire and asserts onclose fires exactly once.

Updated commit: d77acc4.

[arhancanli]

Thanks for taking the send() route, and for the reason clearing _process is a bad idea: the close() teardown living under that guard is something I had not weighed. I read 283fe5c (not run). The exitCode/signalCode check at the top of send() closes the late-send window, and the stub that holds [exited, close-pending] is the test I would have wanted.

One thing I could not settle from the diff: the two paths now end differently. A send already waiting on drain when the process exits goes through settlePendingSends() and resolves (the doc comment says delivery past exit is out of scope), while a send() issued a moment later rejects with NotConnected. So a caller sees success for a message that may never have left the pipe, and an error for the next one. Is the resolve intended because write() had already accepted the bytes, or would rejecting the waiters too be safer for callers that treat a resolved send() as "the server got it"? And does any test pin which of the two a waiter gets?

[amd-clb]

The asymmetry isn't arbitrary — it tracks whether the bytes were ever accepted by the stream, which is exactly what send() has always signalled:

• A send waiting on drain already called write(); back-pressure means the bytes are queued in the stream's buffer, not dropped. The transport accepted them and did its part, so resolving matches the normal contract — where send() resolves at the write() hand-off, never at child-consumption, even on the happy path.
• A send issued after exit finds no process and no stdin, so write() never runs and the bytes were never accepted. There's nothing the transport handed off, so rejecting (NotConnected) is the truthful outcome.

So the difference is "bytes were accepted by the stream" (resolve) vs "no stream to accept them" (reject). onclose stays the single authoritative "connection gone" signal in both cases.

On rejecting the waiters too: after exit we can't tell whether the child read the buffered bytes (a drain proves buffer→OS-pipe only, not child-read). Rejecting a send whose bytes were accepted would risk a false loss signal that invites a re-send and duplicates a message the child may already have read — so we keep the resolve and put the connection-gone signal on onclose.

And yes, both outcomes are pinned: waiting-send-resolves by the "resolves and onclose fires exactly once" test (the close() path) and the "child exits on its own" test (the settle-on-exit path); late-send-rejects by the "[exited, close-pending] rejects as not connected" test.

[arhancanli]

Thanks, the accepted-by-the-stream line is a clean rule, and the duplicate-on-resend risk is a better reason to keep the waiters resolving than the one I had in mind. I read the doc comment on the settle helper in the current diff (not run) and it says the same thing: the bytes were already handed to stdin.write(), delivery past exit is out of scope.

That comment sits on the internal helper, though, and a caller only sees send(). Does the doc on send() itself say that a resolved promise means "handed to the stream" and not "the child read it", with onclose as the signal that the connection is gone? A caller who treats resolve as an ack would otherwise never learn the contract from the place they read.

[amd-clb]

Good point — the contract lived only on the internal helper, where a caller wouldn't read it. I added a JSDoc to send() itself saying exactly that: it resolves when the bytes are handed to stdin.write(), not when the child reads or processes them (true on the happy path and when a back-pressured send is still pending at exit); use onclose — which fires exactly once — to learn the connection is gone; and it rejects NotConnected if the process isn't running or has already exited. So a caller reading only send() now learns the contract.

Updated commit: 1fb1539.

[arhancanli]

Read 1fb1539: the send() JSDoc now states the contract where a caller will see it (resolves when stdin.write() accepts the bytes, onclose fires once, NotConnected after exit), and settlePendingSends() also drops the shared drain listener, so nothing lingers on a dead stdin. That closes my earlier question.

One edge I could not see covered: the same transport object being restarted (start() after close()). _pendingDrainWaiters and _drainHandler are reset in settlePendingSends(), so a second process should get a fresh single drain wait. Is there a test that backs up a send, closes, restarts, backs up again and asserts stdin.listenerCount("drain") is 1 on the new process?

[amd-clb]

Added it — a test that backs up a send on the first process, closes, restarts (a new process), backs up again, and asserts the new process's stdin carries exactly one drain listener. It pins that settlePendingSends() (run on close()) fully resets the shared drain state so nothing carries across the restart. Verified it fails if the _pendingDrainWaiters reset is removed (the second process then sees a non-empty array, never registers a fresh wait, and the drain listener never appears).

One caveat, for the record: restart is not the supported lifecycle — the Client creates a new transport on reconnect — so this pins the drain-state reset, not a guarantee that every option is restart-safe. stderr: 'pipe' in particular reuses a PassThrough that close() ends, so that configuration is not restartable (the test uses the default inherit stderr).

Updated commit: fb08480.

[arhancanli]

Read fb08480 (not run). The restart test does what you describe: it asserts listenerCount('drain') is 1 on proc1 while the send is backed up, then proc2 !== proc1 and 1 again on the new stdin, so a stale _drainHandler or a non-empty _pendingDrainWaiters left over from the first run would show up as 0 on the second. Awaiting pending1 after close() also pins the "resolves, not rejects" contract from earlier in this thread.

Your caveat is the part I would keep somewhere a caller can see it. With stderr: 'pipe', close() ends the PassThrough, so a second start() hands back a stream that is already finished, and nothing says so until stderr output goes missing. Would a one-line note on start() ("not restartable with stderr: 'pipe'; create a new transport") be in scope for this PR, or do you prefer to leave it to a separate change since the Client never restarts a transport?