diff --git a/src/client/stdio.ts b/src/client/stdio.ts index 9e35293d..31fca510 100644 --- a/src/client/stdio.ts +++ b/src/client/stdio.ts @@ -184,6 +184,13 @@ export class StdioClientTransport implements Transport { return this._process?.stderr ?? null; } + /** + * The process id of the child process, if it has been started. + */ + get pid(): number | undefined { + return this._process?.pid; + } + private processReadBuffer() { while (true) { try { @@ -199,7 +206,23 @@ export class StdioClientTransport implements Transport { } } + private forceExitProcess(pid: number): void { + try { + process.kill(pid, "SIGKILL"); + + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code === "ESRCH") { + // Ignoring ESRCH error, which means the process is already dead. + return; + } + + console.warn(`Failed to kill process ${pid}: ${err}`); + } + } + async close(): Promise { + const pid = this.pid; + this._abortController.signal.onabort = () => setTimeout(() => pid && this.forceExitProcess(pid), 3000); this._abortController.abort(); this._process = undefined; this._readBuffer.clear(); diff --git a/src/integration-tests/process-cleanup.test.ts b/src/integration-tests/process-cleanup.test.ts index 0dd7861a..f6fd4d59 100644 --- a/src/integration-tests/process-cleanup.test.ts +++ b/src/integration-tests/process-cleanup.test.ts @@ -1,10 +1,13 @@ +import { execSync } from "node:child_process"; import { Server } from "../server/index.js"; import { StdioServerTransport } from "../server/stdio.js"; +import { Client } from "../client/index.js"; +import { StdioClientTransport } from "../client/stdio.js"; describe("Process cleanup", () => { - jest.setTimeout(5000); // 5 second timeout + jest.setTimeout(10 * 1000); // 10 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", @@ -25,4 +28,36 @@ describe("Process cleanup", () => { // The test runner will fail if the process hangs expect(true).toBe(true); }); -}); \ No newline at end of file + + it("client should exit cleanly after closing transport", async () => { + const isProcessRunning = (pid: number) => { + try { + execSync(`ps -p ${pid}`, { stdio: 'ignore' }); + return true; + + /* eslint-disable @typescript-eslint/no-unused-vars */ + } catch (error) { + return false; + } + } + + const client = new Client({ + name: "test-client", + version: "1.0.0", + }); + + const transport = new StdioClientTransport({ + command: "node", + args: ["server-that-hangs.js"], + cwd: __dirname + }); + + await client.connect(transport); + const pid = transport.pid; + + await client.close(); + await new Promise(resolve => setTimeout(resolve, 5000)); + + expect(isProcessRunning(pid!)).toBe(false); + }); +}); diff --git a/src/integration-tests/server-that-hangs.js b/src/integration-tests/server-that-hangs.js new file mode 100644 index 00000000..72ef77bf --- /dev/null +++ b/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);