Skip to content

Commit ec29787

Browse files
committed
Merge fix/exit-on-stdio-close: exit on MCP stdio disconnect (upstream PR ktnyt#53)
2 parents 2bafa53 + d206f43 commit ec29787

3 files changed

Lines changed: 181 additions & 11 deletions

File tree

index.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Server } from '@modelcontextprotocol/sdk/server/index.js';
44
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';
55
import { logger } from './src/logger.js';
66
import { LSPClient } from './src/lsp-client.js';
7+
import { installShutdownHandlers } from './src/shutdown.js';
78
import { diagnosticsTools } from './src/tools/diagnostics.js';
89
import { hoverTools } from './src/tools/hover.js';
910
import { navigationTools } from './src/tools/navigation.js';
@@ -57,15 +58,8 @@ const allTools = [
5758

5859
registerTools(server, allTools, lspClient);
5960

60-
process.on('SIGINT', () => {
61-
lspClient.dispose();
62-
process.exit(0);
63-
});
64-
65-
process.on('SIGTERM', () => {
66-
lspClient.dispose();
67-
process.exit(0);
68-
});
61+
// Covers SIGINT/SIGTERM/SIGHUP and the MCP client closing its end of stdio.
62+
const shutdown = installShutdownHandlers(lspClient);
6963

7064
async function main() {
7165
const transport = new StdioServerTransport();
@@ -82,6 +76,5 @@ async function main() {
8276

8377
main().catch((error) => {
8478
logger.error(`Server error: ${error}\n`);
85-
lspClient.dispose();
86-
process.exit(1);
79+
shutdown(1);
8780
});

src/shutdown.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { describe, expect, it, jest } from 'bun:test';
2+
import { installShutdownHandlers } from './shutdown.js';
3+
import type { ShutdownProcess } from './shutdown.js';
4+
5+
/** A stand-in for `process` that records handlers so tests can fire them. */
6+
function createFakeProcess() {
7+
const signalHandlers = new Map<string, Array<() => void>>();
8+
const stdinHandlers = new Map<string, Array<() => void>>();
9+
const exit = jest.fn();
10+
11+
const record = (map: Map<string, Array<() => void>>, event: string, listener: () => void) => {
12+
const existing = map.get(event) ?? [];
13+
existing.push(listener);
14+
map.set(event, existing);
15+
};
16+
17+
const proc: ShutdownProcess = {
18+
on(event: string, listener: () => void) {
19+
record(signalHandlers, event, listener);
20+
return proc;
21+
},
22+
stdin: {
23+
on(event: string, listener: () => void) {
24+
record(stdinHandlers, event, listener);
25+
return proc.stdin;
26+
},
27+
},
28+
exit,
29+
};
30+
31+
return {
32+
proc,
33+
exit,
34+
fireSignal: (event: string) => {
35+
for (const handler of signalHandlers.get(event) ?? []) handler();
36+
},
37+
fireStdin: (event: string) => {
38+
for (const handler of stdinHandlers.get(event) ?? []) handler();
39+
},
40+
hasSignal: (event: string) => signalHandlers.has(event),
41+
hasStdin: (event: string) => stdinHandlers.has(event),
42+
};
43+
}
44+
45+
describe('installShutdownHandlers', () => {
46+
it('subscribes to the termination signals', () => {
47+
const fake = createFakeProcess();
48+
installShutdownHandlers({ dispose: jest.fn() }, fake.proc);
49+
50+
expect(fake.hasSignal('SIGINT')).toBe(true);
51+
expect(fake.hasSignal('SIGTERM')).toBe(true);
52+
expect(fake.hasSignal('SIGHUP')).toBe(true);
53+
});
54+
55+
// The regression: an MCP client that exits closes the pipe without
56+
// signalling, and StdioServerTransport never surfaces EOF.
57+
it('disposes and exits when stdin ends', () => {
58+
const fake = createFakeProcess();
59+
const dispose = jest.fn();
60+
installShutdownHandlers({ dispose }, fake.proc);
61+
62+
expect(fake.hasStdin('end')).toBe(true);
63+
fake.fireStdin('end');
64+
65+
expect(dispose).toHaveBeenCalledTimes(1);
66+
expect(fake.exit).toHaveBeenCalledWith(0);
67+
});
68+
69+
it('disposes and exits when stdin closes', () => {
70+
const fake = createFakeProcess();
71+
const dispose = jest.fn();
72+
installShutdownHandlers({ dispose }, fake.proc);
73+
74+
fake.fireStdin('close');
75+
76+
expect(dispose).toHaveBeenCalledTimes(1);
77+
expect(fake.exit).toHaveBeenCalledWith(0);
78+
});
79+
80+
it.each(['SIGINT', 'SIGTERM', 'SIGHUP'])('disposes and exits on %s', (signal) => {
81+
const fake = createFakeProcess();
82+
const dispose = jest.fn();
83+
installShutdownHandlers({ dispose }, fake.proc);
84+
85+
fake.fireSignal(signal);
86+
87+
expect(dispose).toHaveBeenCalledTimes(1);
88+
expect(fake.exit).toHaveBeenCalledWith(0);
89+
});
90+
91+
// stdin emits both 'end' and 'close', and signals can race each other.
92+
it('disposes once even when several shutdown triggers fire', () => {
93+
const fake = createFakeProcess();
94+
const dispose = jest.fn();
95+
installShutdownHandlers({ dispose }, fake.proc);
96+
97+
fake.fireStdin('end');
98+
fake.fireStdin('close');
99+
fake.fireSignal('SIGTERM');
100+
101+
expect(dispose).toHaveBeenCalledTimes(1);
102+
expect(fake.exit).toHaveBeenCalledTimes(1);
103+
});
104+
105+
it('returns a shutdown function that follows the same path', () => {
106+
const fake = createFakeProcess();
107+
const dispose = jest.fn();
108+
const shutdown = installShutdownHandlers({ dispose }, fake.proc);
109+
110+
shutdown(1);
111+
112+
expect(dispose).toHaveBeenCalledTimes(1);
113+
expect(fake.exit).toHaveBeenCalledWith(1);
114+
});
115+
});

src/shutdown.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/** Anything holding resources that must be released before exit. */
2+
export interface Disposable {
3+
dispose(): void;
4+
}
5+
6+
/**
7+
* The parts of `process` this module touches, named structurally so tests can
8+
* pass a fake.
9+
*/
10+
export interface ShutdownProcess {
11+
on(event: string, listener: () => void): unknown;
12+
stdin: { on(event: string, listener: () => void): unknown };
13+
exit(code?: number): unknown;
14+
}
15+
16+
/** Signals that mean "stop now". */
17+
const SIGNALS = ['SIGINT', 'SIGTERM', 'SIGHUP'] as const;
18+
19+
/**
20+
* stdin reaching EOF means the MCP client has gone away and nothing further can
21+
* arrive.
22+
*/
23+
const STDIN_CLOSED = ['end', 'close'] as const;
24+
25+
/**
26+
* Exit cleanly when the MCP client disconnects or we are signalled, releasing
27+
* the LSP servers we spawned.
28+
*
29+
* The stdio half matters as much as the signals. An MCP client that exits
30+
* simply closes its end of the pipe, often without signalling us, and
31+
* `StdioServerTransport` subscribes only to stdin's `data` and `error` events —
32+
* never `end` or `close` — so `Server.onclose` does not fire on EOF. Without
33+
* this, cclsp keeps running after its client is gone, and every LSP server it
34+
* spawned stays resident with it. Long-lived language servers are not cheap
35+
* (pylsp holds a few hundred MB), so across repeated client invocations the
36+
* orphans accumulate until the machine is under real memory pressure.
37+
*
38+
* Returns the shutdown function, so a caller can trigger the same path itself.
39+
*/
40+
export function installShutdownHandlers(
41+
client: Disposable,
42+
proc: ShutdownProcess = process
43+
): (code?: number) => void {
44+
let shuttingDown = false;
45+
46+
const shutdown = (code = 0): void => {
47+
// Signals can arrive together, and stdin emits both `end` and `close`.
48+
if (shuttingDown) return;
49+
shuttingDown = true;
50+
client.dispose();
51+
proc.exit(code);
52+
};
53+
54+
for (const signal of SIGNALS) {
55+
proc.on(signal, () => shutdown(0));
56+
}
57+
for (const event of STDIN_CLOSED) {
58+
proc.stdin.on(event, () => shutdown(0));
59+
}
60+
61+
return shutdown;
62+
}

0 commit comments

Comments
 (0)