fix: modelcontextprotocol#780 onerror and other listener not remove after client close (stdio)

Author: shellRaining

package-lock.json

@@ -94,6 +94,8 @@ export class StdioClientTransport implements Transport {
     private _readBuffer: ReadBuffer = new ReadBuffer();
     private _serverParams: StdioServerParameters;
     private _stderrStream: PassThrough | null = null;
+    private _onServerDataHandler?: (chunk: Buffer) => void;
+    private _onServerErrorHandler?: (error: Error) => void;
 
     onclose?: () => void;
     onerror?: (error: Error) => void;
@@ -129,33 +131,31 @@ export class StdioClientTransport implements Transport {
                 cwd: this._serverParams.cwd
             });
 
+            this._onServerDataHandler = (chunk: Buffer) => {
+                this._readBuffer.append(chunk);
+                this.processReadBuffer();
+            };
+            this._onServerErrorHandler = (error: Error) => {
+                this.onerror?.(error);
+            };
+
+            this._process.stdout?.on('data', this._onServerDataHandler);
+            this._process.stdout?.on('error', this._onServerErrorHandler);
+            this._process.stdin?.on('error', this._onServerErrorHandler);
+
             this._process.on('error', error => {
                 reject(error);
                 this.onerror?.(error);
             });
-
-            this._process.on('spawn', () => {
-                resolve();
-            });
-
-            this._process.on('close', _code => {
+            this._process.once('spawn', () => resolve());
+            this._process.once('close', _code => {
+                if (this._process) {
+                    this.cleanupListeners(this._process);
+                }
                 this._process = undefined;
                 this.onclose?.();
             });
 
-            this._process.stdin?.on('error', error => {
-                this.onerror?.(error);
-            });
-
-            this._process.stdout?.on('data', chunk => {
-                this._readBuffer.append(chunk);
-                this.processReadBuffer();
-            });
-
-            this._process.stdout?.on('error', error => {
-                this.onerror?.(error);
-            });
-
             if (this._stderrStream && this._process.stderr) {
                 this._process.stderr.pipe(this._stderrStream);
             }
@@ -201,8 +201,19 @@ export class StdioClientTransport implements Transport {
         }
     }
 
+    private cleanupListeners(process: ChildProcess) {
+        if (this._onServerDataHandler) {
+            process.stdout?.off('data', this._onServerDataHandler);
+        }
+        if (this._onServerErrorHandler) {
+            process.stdout?.off('error', this._onServerErrorHandler);
+            process.stdin?.off('error', this._onServerErrorHandler);
+        }
+    }
+
     async close(): Promise<void> {
         if (this._process) {
+            this.cleanupListeners(this._process);
             const processToClose = this._process;
             this._process = undefined;
 

src/client/stdio.ts

@@ -2,7 +2,7 @@ import { StdioClientTransport, getDefaultEnvironment } from '../../src/client/st
 import spawn from 'cross-spawn';
 import { JSONRPCMessage } from '../../src/types.js';
 import { ChildProcess } from 'node:child_process';
-import { Mock, MockedFunction } from 'vitest';
+import { Mock, MockedFunction, vi } from 'vitest';
 
 // mock cross-spawn
 vi.mock('cross-spawn');
@@ -14,8 +14,9 @@ describe('StdioClientTransport using cross-spawn', () => {
         mockSpawn.mockImplementation(() => {
             const mockProcess: {
                 on: Mock;
-                stdin?: { on: Mock; write: Mock };
-                stdout?: { on: Mock };
+                once: Mock;
+                stdin?: { on: Mock; write: Mock; off: Mock };
+                stdout?: { on: Mock; off: Mock };
                 stderr?: null;
             } = {
                 on: vi.fn((event: string, callback: () => void) => {
@@ -24,12 +25,20 @@ describe('StdioClientTransport using cross-spawn', () => {
                     }
                     return mockProcess;
                 }),
+                once: vi.fn((event: string, callback: () => void) => {
+                    if (event === 'spawn') {
+                        callback();
+                    }
+                    return mockProcess;
+                }),
                 stdin: {
                     on: vi.fn(),
-                    write: vi.fn().mockReturnValue(true)
+                    write: vi.fn().mockReturnValue(true),
+                    off: vi.fn()
                 },
                 stdout: {
-                    on: vi.fn()
+                    on: vi.fn(),
+                    off: vi.fn()
                 },
                 stderr: null
             };
@@ -107,13 +116,16 @@ describe('StdioClientTransport using cross-spawn', () => {
         // get the mock process object
         const mockProcess: {
             on: Mock;
+            once: Mock;
             stdin: {
                 on: Mock;
                 write: Mock;
                 once: Mock;
+                off: Mock;
             };
             stdout: {
                 on: Mock;
+                off: Mock;
             };
             stderr: null;
         } = {
@@ -123,13 +135,21 @@ describe('StdioClientTransport using cross-spawn', () => {
                 }
                 return mockProcess;
             }),
+            once: vi.fn((event: string, callback: () => void) => {
+                if (event === 'spawn') {
+                    callback();
+                }
+                return mockProcess;
+            }),
             stdin: {
                 on: vi.fn(),
                 write: vi.fn().mockReturnValue(true),
-                once: vi.fn()
+                once: vi.fn(),
+                off: vi.fn()
             },
             stdout: {
-                on: vi.fn()
+                on: vi.fn(),
+                off: vi.fn()
             },
             stderr: null
         };