Skip to content

Commit f98dd15

Browse files
committed
fix(server): re-check teardown after tryServeListen in serveStdio's processMessage
Review nit on #2494: with the transport now closing on stdin EOF, the two await tryServeListen(message) sites in processMessage were the only awaits not followed by the isTornDown() re-check, so a buffered final message racing stdin EOF on a modern-pinned connection dereferenced state.instance.channel after the state had been reassigned to closed, surfacing TypeError: Cannot read properties of undefined (reading 'channel') through options.onerror. Guard both sites like every other await in the function, and add a deterministic regression test (final 'data' chunk with 'end' on the next tick, emitted from a macrotask so the EOF handler beats the pump's microtask continuation, as it does with real stream events). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016gag9FFr6udcsF8qvMvMV8
1 parent 252556b commit f98dd15

2 files changed

Lines changed: 71 additions & 0 deletions

File tree

packages/server/src/server/serveStdio.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,11 @@ export function serveStdio(factory: McpServerFactory, options: ServeStdioOptions
598598
if (state.era === 'modern' && (await tryServeListen(message))) {
599599
return;
600600
}
601+
if (isTornDown()) {
602+
// Closed while entry listen routing was being checked (a
603+
// buffered final message can race stdin EOF): stay closed.
604+
return;
605+
}
601606
state.instance.channel.deliver(message);
602607
return;
603608
}
@@ -692,6 +697,11 @@ export function serveStdio(factory: McpServerFactory, options: ServeStdioOptions
692697
if (await tryServeListen(message)) {
693698
return;
694699
}
700+
if (isTornDown()) {
701+
// Closed while entry listen routing was being checked (a
702+
// buffered final message can race stdin EOF): stay closed.
703+
return;
704+
}
695705
state.instance.channel.deliver(message, { classification: opening.classification });
696706
return;
697707
}

packages/server/test/server/serveStdio.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
* - malformed and unsupported envelope claims are answered by the entry,
2121
* consistent with the HTTP entry's treatment, without pinning.
2222
*/
23+
import { Readable, Writable } from 'node:stream';
24+
2325
import type {
2426
JSONRPCErrorResponse,
2527
JSONRPCMessage,
@@ -47,6 +49,7 @@ import type { McpServerFactory } from '../../src/server/createMcpHandler';
4749
import { McpServer } from '../../src/server/mcp';
4850
import type { ServeStdioOptions } from '../../src/server/serveStdio';
4951
import { serveStdio } from '../../src/server/serveStdio';
52+
import { StdioServerTransport } from '../../src/server/stdio';
5053

5154
const MODERN = '2026-07-28';
5255

@@ -811,6 +814,64 @@ describe('teardown', () => {
811814
expect(closed[0]).toBe(true);
812815
expect(peerClosed).toBe(true);
813816
});
817+
818+
it('a buffered final message racing stdin EOF on a modern-pinned connection is dropped cleanly (no TypeError through onerror)', async () => {
819+
// Over the real StdioServerTransport: stdin 'end' fires on the next
820+
// tick behind a final 'data' chunk, so the transport's onclose (which
821+
// reassigns the entry state to closed) runs BEFORE the pump's
822+
// microtask continuation inside processMessage resumes from
823+
// `await tryServeListen(message)`. The continuation must re-check
824+
// teardown instead of dereferencing the pinned instance.
825+
const { factory, closed } = trackingFactory();
826+
827+
const stdin = new Readable({ read() {} });
828+
const outLines: string[] = [];
829+
const stdout = new Writable({
830+
write(chunk: Buffer, _encoding, callback) {
831+
outLines.push(chunk.toString());
832+
callback();
833+
}
834+
});
835+
const transport = new StdioServerTransport(stdin, stdout);
836+
837+
const errors: Error[] = [];
838+
const handle = serveStdio(factory, { transport, onerror: error => void errors.push(error) });
839+
840+
const line = (message: JSONRPCMessage) => Buffer.from(`${JSON.stringify(message)}\n`);
841+
842+
// Pin the modern era with a first enveloped request and wait for its answer.
843+
stdin.emit('data', line({ jsonrpc: '2.0', id: 1, method: 'tools/list', params: { _meta: envelope() } }));
844+
while (outLines.length === 0) {
845+
await new Promise(resolve => setTimeout(resolve, 5));
846+
}
847+
848+
// The race: a final enveloped request with stdin EOF right behind it.
849+
// Emitted from a timer callback so the events originate from a
850+
// macrotask exactly as real stream events do — there, the
851+
// nextTick'd 'end' runs BEFORE the pump's promise continuation
852+
// resumes inside processMessage (a test body itself is a microtask
853+
// context, where the continuation would win and mask the race).
854+
setTimeout(() => {
855+
stdin.emit(
856+
'data',
857+
line({
858+
jsonrpc: '2.0',
859+
id: 2,
860+
method: 'tools/call',
861+
params: { name: 'echo', arguments: { text: 'late' }, _meta: envelope() }
862+
})
863+
);
864+
process.nextTick(() => stdin.emit('end'));
865+
}, 0);
866+
await new Promise(resolve => setTimeout(resolve, 20));
867+
868+
// The connection tore down (EOF closed the pinned instance) and the
869+
// raced message was dropped without any error surfacing to onerror.
870+
expect(closed[0]).toBe(true);
871+
expect(errors).toEqual([]);
872+
873+
await handle.close();
874+
});
814875
});
815876

816877
describe('legacy input_required shim through the stdio entry', () => {

0 commit comments

Comments
 (0)