Ensure stdio client closes underlying process

src/client/stdio.ts

@@ -134,7 +134,6 @@ export class StdioClientTransport implements Transport {
             this._process.on('error', error => {
                 if (error.name === 'AbortError') {
                     // Expected when close() is called.
-                    this.onclose?.();
                     return;
                 }
 
@@ -210,8 +209,36 @@ export class StdioClientTransport implements Transport {
     }
 
     async close(): Promise<void> {
-        this._abortController.abort();
-        this._process = undefined;
+        if (this._process) {
+            const processToClose = this._process;
+            this._process = undefined;
+
+            const closePromise = new Promise<void>(resolve => {
+                processToClose.once('close', () => {
+                    resolve();
+                });
+            });
+
+            this._abortController.abort();
+
+            // waits the underlying process to exit cleanly otherwise after 1s kills it
+            await Promise.race([closePromise, new Promise(resolve => setTimeout(resolve, 1_000).unref())]);
+
+            if (processToClose.exitCode === null) {
+                try {
+                    processToClose.stdin?.end();
+                } catch {
+                    // ignore errors in trying to close stdin
+                }
+
+                try {
+                    processToClose.kill('SIGKILL');
+                } catch {
+                    // we did our best
+                }
+            }
+        }
+
         this._readBuffer.clear();
     }
 

src/integration-tests/process-cleanup.test.ts

@@ -1,10 +1,13 @@
+import { Readable, Writable } from 'stream';
+import { Client } from '../client/index.js';
+import { StdioClientTransport } from '../client/stdio.js';
 import { Server } from '../server/index.js';
 import { StdioServerTransport } from '../server/stdio.js';
 
 describe('Process cleanup', () => {
     jest.setTimeout(5000); // 5 second timeout
 
-    it('should exit cleanly after closing transport', async () => {
+    it('server should exit cleanly after closing transport', async () => {
         const server = new Server(
             {
                 name: 'test-server',
@@ -15,14 +18,56 @@ describe('Process cleanup', () => {
             }
         );
 
-        const transport = new StdioServerTransport();
+        const mockReadable = new Readable({
+                read() {
+                    this.push(null); // signal EOF
+                }
+            }),
+            mockWritable = new Writable({
+                write(chunk, encoding, callback) {
+                    callback();
+                }
+            });
+
+        // Attach mock streams to process for the server transport
+        const transport = new StdioServerTransport(mockReadable, mockWritable);
         await server.connect(transport);
 
         // Close the transport
         await transport.close();
 
+        // ensure a proper disposal mock streams
+        mockReadable.destroy();
+        mockWritable.destroy();
+
         // If we reach here without hanging, the test passes
         // The test runner will fail if the process hangs
         expect(true).toBe(true);
     });
+
+    it('onclose should be called exactly once', async () => {
+        const client = new Client({
+            name: 'test-client',
+            version: '1.0.0'
+        });
+
+        const transport = new StdioClientTransport({
+            command: process.argv0,
+            args: ['test-server.js'],
+            cwd: __dirname
+        });
+
+        let onCloseWasCalled = 0;
+        client.onclose = () => {
+            onCloseWasCalled++;
+        };
+
+        await client.connect(transport);
+        await client.close();
+
+        // A short delay to allow the close event to propagate
+        await new Promise(resolve => setTimeout(resolve, 50));
+
+        expect(onCloseWasCalled).toBe(1);
+    });
 });

src/integration-tests/server-that-hangs.js

@@ -0,0 +1,21 @@
+import { setTimeout } from 'node:timers'
+import process from 'node:process'
+import { McpServer } from "../../dist/esm/server/mcp.js";
+import { StdioServerTransport } from "../../dist/esm/server/stdio.js";
+
+const transport = new StdioServerTransport();
+
+const server = new McpServer({
+  name: "server-that-hangs",
+  version: "1.0.0"
+});
+
+await server.connect(transport);
+
+const doNotExitImmediately = async () => {
+  setTimeout(() => process.exit(0), 30 * 1000);
+};
+
+process.stdin.on('close', doNotExitImmediately);
+process.on('SIGINT', doNotExitImmediately);
+process.on('SIGTERM', doNotExitImmediately);

src/integration-tests/test-server.js

@@ -0,0 +1,11 @@
+import { McpServer } from "../../dist/esm/server/mcp.js";
+import { StdioServerTransport } from "../../dist/esm/server/stdio.js";
+
+const transport = new StdioServerTransport();
+
+const server = new McpServer({
+  name: "test-server",
+  version: "1.0.0",
+});
+
+await server.connect(transport);