Skip to content

Commit d64bd02

Browse files
debugmcpdevcynarlabclaude
authored
fix(windows): report the end of a Rust/C++/COBOL program when it happens, not 2 s later (#856) (#859)
* fix(windows): report the end of a Rust/C++/COBOL program when it happens, not 2 s later The worker holds exited/terminated until the adapter's forwarded stdio has drained, and took "the pipes closed" as the sign. CodeLLDB outlives its debuggee and keeps the pipes open until teardown, so on Windows (where the debuggee writes straight into them) the wait ran to its 2 s backstop at the end of every program. - AdapterSpawnConfig.forwardStdio gains adapterOutlivesDebuggee; buildLldbSpawnConfig sets it on win32 (rust, cpp, cobol). - For such an adapter the drain also settles when the pipes have been quiet for 100 ms, counted from the later of the last chunk and the first terminal signal, and re-checked after one full turn of the event loop so a descheduled worker cannot take unread output for silence. Output still arriving keeps the wait open; the 2 s backstop is unchanged. - rdbg, whose pipes close with the program, still waits for the close. Measured: end reported 110-134 ms after launch (was 2009-2030 ms); the longest gap inside 9 MB of exit-time output was 14 ms idle, 39 ms with every core busy. Closes #856. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(proxy): say what the stdio drain guarantees in each of its three endings Review follow-up on #859. The comment on trackStdioActivity said a counted chunk's lines have been forwarded; that holds for complete lines only (a trailing fragment waits in the line buffer for the pipe to close, #860). The drain's doc stated the output-before-exit order as a guarantee; it is one when the wait ends on the close, a measured expectation when it ends on silence, and neither at the backstop. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(proxy): the quiet window's event-loop turn is pinned, and its dead cancel flag goes Output that lands in the same instant the quiet window closes must restart the window: a test now fails if the turn after the timer stops checking for it (verified by removing the check). The cancelled flag in waitForAdapterStdioQuiet could never be read — at most one of the timer and the immediate is pending, and cancel clears both — so it is removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: JF <john.franklin@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 20e809b commit d64bd02

8 files changed

Lines changed: 412 additions & 9 deletions

File tree

‎changelog.d/856.fixed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
**On Windows, the end of a Rust, C/C++ or COBOL program is reported when it happens, not 2 s later** — the proxy holds a program's exit until the output it printed on the way out has been forwarded, and took "the adapter's stdio pipes closed" as the sign of that. CodeLLDB outlives its debuggee and keeps those pipes open until the session is torn down, so on Windows (where the debuggee writes straight into them) the wait ran to its 2 s backstop at the end of every program: `list_debug_sessions` kept saying `running`, `wait_for_stop` kept blocking, and a `start_debugging` of a short program answered `running` with `pending: true` for a program that had already ended. For an adapter that outlives its debuggee the proxy now takes the pipes going quiet as the sign — 100 ms without output, counted from the adapter's `exited` and re-checked after a full turn of the event loop; the longest pause measured inside 9 MB of exit-time output was 14 ms idle and 39 ms with every core busy — so the end is reported about 100 ms after the fact and output that is still arriving keeps the wait open. The 2 s backstop is unchanged, as is rdbg, whose pipes do close with the program (#856)

‎packages/shared/src/interfaces/adapter-policy.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,18 @@ export type AdapterSpawnConfig =
837837
* debuggee output.
838838
*/
839839
excludeStderrLinePattern?: RegExp;
840+
/**
841+
* The adapter process stays alive after the debuggee has ended, and
842+
* keeps these pipes open with it (CodeLLDB; issue #856). The worker
843+
* holds the debuggee's exit until the forwarded output has drained,
844+
* and by default "drained" means the pipes closed — true of rdbg -c,
845+
* which exits with its debuggee, and never true of an adapter like
846+
* this one, for which the wait would run to its backstop every time.
847+
* With this set, the pipes going quiet counts as drained: when the
848+
* adapter reports the exit the process is gone and everything it
849+
* wrote is already in the pipe.
850+
*/
851+
adapterOutlivesDebuggee?: boolean;
840852
};
841853
/**
842854
* The adapter process's exit code IS the debuggee's exit code (issue

‎packages/shared/src/interfaces/lldb-policy-shared.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,13 +452,16 @@ export function matchesLldbAdapterCommand(adapterCommand: { command: string; arg
452452
* STDOUT/STDERR process broadcasts), LLDB on Windows lets the debuggee
453453
* inherit the adapter process's pipes. Forward those as output events there;
454454
* on POSIX the channels are exclusive, so this stays off to avoid noise.
455+
* CodeLLDB outlives the debuggee and holds those pipes open until the session
456+
* ends, which the forwarding is told so that a program's exit is not held
457+
* back waiting for a close that only teardown brings (issue #856).
455458
*/
456459
export function buildLldbSpawnConfig(
457460
payload: AdapterSpawnPayload,
458461
platform: NodeJS.Platform = process.platform,
459462
arch: NodeJS.Architecture = process.arch
460463
): AdapterSpawnConfig {
461-
const forwardStdio = platform === 'win32' ? {} : undefined;
464+
const forwardStdio = platform === 'win32' ? { adapterOutlivesDebuggee: true } : undefined;
462465

463466
// If a custom adapter command was provided, use it directly
464467
if (payload.adapterCommand) {

‎packages/shared/tests/unit/adapter-policy-cpp.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,8 @@ describe('CppAdapterPolicy', () => {
213213
args: ['--port', '4711'],
214214
port: 4711
215215
});
216-
expect((config as { forwardStdio?: unknown }).forwardStdio).toEqual({});
216+
// CodeLLDB outlives the debuggee, so its pipes never close with it (issue #856).
217+
expect((config as { forwardStdio?: unknown }).forwardStdio).toEqual({ adapterOutlivesDebuggee: true });
217218
});
218219

219220
it('omits stdio forwarding on POSIX', () => {

‎packages/shared/tests/unit/adapter-policy-rust.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,13 +494,14 @@ describe('RustAdapterPolicy', () => {
494494
const win = asSpawn(
495495
RustAdapterPolicy.getAdapterSpawnConfig(basePayload, 'win32', 'x64')
496496
);
497-
expect(win.forwardStdio).toEqual({});
497+
// CodeLLDB outlives the debuggee, so its pipes never close with it (issue #856).
498+
expect(win.forwardStdio).toEqual({ adapterOutlivesDebuggee: true });
498499

499500
const winCustom = asSpawn(RustAdapterPolicy.getAdapterSpawnConfig({
500501
...basePayload,
501502
adapterCommand: { command: 'custom', args: [] }
502503
}, 'win32', 'x64'));
503-
expect(winCustom.forwardStdio).toEqual({});
504+
expect(winCustom.forwardStdio).toEqual({ adapterOutlivesDebuggee: true });
504505

505506
// POSIX: CodeLLDB emits DAP output events itself (LLDB holds the pipes)
506507
const linux = asSpawn(

‎src/proxy/dap-proxy-worker.ts‎

Lines changed: 106 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ export const MAX_QUEUED_COMMANDS = 256;
8181
export const INITIALIZE_RESPONSE_GRACE_MS = 2000;
8282
/** Bound on each terminal-signal wait (stdio drain, child flush, the debugger-off launch outcome). */
8383
const TERMINAL_SIGNAL_BACKSTOP_MS = 2000;
84+
/**
85+
* How long the stdio pipes of an adapter that outlives its debuggee must stay
86+
* silent, after the debuggee's end was reported, to count as drained (issue
87+
* #856). Sized from CodeLLDB on Windows printing 9 MB as it exits: the
88+
* longest pause between two consecutive chunks of that output was 14 ms on an
89+
* idle machine and 39 ms with all 16 cores busy (and the proxy logging every
90+
* line at debug level). The clock is also not trusted alone — see
91+
* waitForAdapterStdioQuiet.
92+
*/
93+
const ADAPTER_STDIO_QUIET_MS = 100;
8494
/**
8595
* How long the pre-detach resume (issue #763) may wait for the adapter's
8696
* continue response before the disconnect goes out regardless. Budgeted
@@ -115,6 +125,14 @@ export class DapProxyWorker {
115125
private exitSynthesisAttempted: boolean = false;
116126
/** Armed when adapter stdio forwarding is active (issue #222): resolves once both stdio streams close. */
117127
private adapterStdioDrained: Promise<void> | null = null;
128+
/**
129+
* Armed as well when the adapter outlives its debuggee (issue #856), whose
130+
* pipes do not close when the program ends: every chunk either stream
131+
* delivers is counted and timed, so the drain can settle on silence.
132+
*/
133+
private adapterStdioActivity: { chunks: number; lastChunkAt: number } | null = null;
134+
/** When the first terminal signal arrived: the silence that counts is measured from here. */
135+
private firstTerminalSignalAt: number | null = null;
118136
// Terminal signals (exited/terminated DAP events, socket close, adapter
119137
// process exit) all await the stdio drain barrier; without a queue their
120138
// continuations resume in an order set by await counts, not arrival — a
@@ -529,6 +547,9 @@ export class DapProxyWorker {
529547
this.adapterProcess = spawnResult.process;
530548
if (spawnConfig.forwardStdio) {
531549
this.adapterStdioDrained = this.createStdioDrainBarrier(spawnResult.process);
550+
if (spawnConfig.forwardStdio.adapterOutlivesDebuggee) {
551+
this.adapterStdioActivity = this.trackStdioActivity(spawnResult.process);
552+
}
532553
}
533554
this.adapterExitCodeIsDebuggeeExitCode = spawnConfig.adapterExitCodeIsDebuggeeExitCode === true;
534555
this.logger!.info(`[Worker] Adapter spawned with PID: ${spawnResult.pid}`);
@@ -2412,14 +2433,43 @@ export class DapProxyWorker {
24122433
]).then(() => undefined);
24132434
}
24142435

2436+
/**
2437+
* Count and time every chunk the adapter's stdio delivers (issue #856).
2438+
* Registered after the adapter manager's own 'data' listeners, so by the
2439+
* time a chunk is counted the complete lines in it have been forwarded. A
2440+
* trailing fragment with no newline is another matter: it stays in the
2441+
* manager's line buffer until the pipe closes, which for an adapter that
2442+
* outlives its debuggee is teardown — too late, now as before (issue #860).
2443+
*/
2444+
private trackStdioActivity(adapterProcess: ChildProcess): { chunks: number; lastChunkAt: number } {
2445+
const activity = { chunks: 0, lastChunkAt: 0 };
2446+
const note = () => {
2447+
activity.chunks += 1;
2448+
activity.lastChunkAt = Date.now();
2449+
};
2450+
adapterProcess.stdout?.on('data', note);
2451+
adapterProcess.stderr?.on('data', note);
2452+
return activity;
2453+
}
2454+
24152455
/**
24162456
* Hold exited/terminated forwarding until adapter stdio has drained: a
24172457
* debuggee printing to a block-buffered pipe flushes everything at exit,
24182458
* milliseconds after the adapter's terminated event, and the SessionManager
2419-
* stops the proxy on terminated — dropping late messages. Stream 'data'
2420-
* fires before 'close' and IPC is FIFO, so waiting here guarantees the
2421-
* forwarded output reaches the session buffer first. 2s backstop for
2422-
* adapters that never close their pipes. No-op when forwarding is off.
2459+
* stops the proxy on terminated — dropping late messages. When the wait
2460+
* ends on the pipes closing, the order is guaranteed: stream 'data' fires
2461+
* before 'close' and IPC is FIFO, so the forwarded output reaches the
2462+
* session buffer first. When it ends on silence the order rests on a
2463+
* measurement instead (ADAPTER_STDIO_QUIET_MS), and at the backstop on
2464+
* nothing.
2465+
*
2466+
* Drained means the pipes closed — or, for an adapter that outlives its
2467+
* debuggee and so never closes them when the program ends, that they went
2468+
* quiet (issue #856: CodeLLDB on Windows used to sit out the backstop at
2469+
* the end of every program, reporting each exit 2 s late). The 2 s
2470+
* backstop still bounds both: a pipe that neither closes nor falls silent
2471+
* (something the program started is still writing to it) is not waited
2472+
* for longer than before. No-op when forwarding is off.
24232473
*/
24242474
private async waitForAdapterStdioDrain(): Promise<void> {
24252475
if (!this.adapterStdioDrained) {
@@ -2429,15 +2479,65 @@ export class DapProxyWorker {
24292479
const backstop = new Promise<void>(resolve => {
24302480
timer = setTimeout(resolve, TERMINAL_SIGNAL_BACKSTOP_MS);
24312481
});
2482+
const quiet = this.adapterStdioActivity ? this.waitForAdapterStdioQuiet(this.adapterStdioActivity) : undefined;
24322483
try {
2433-
await Promise.race([this.adapterStdioDrained, backstop]);
2484+
await Promise.race([this.adapterStdioDrained, backstop, ...(quiet ? [quiet.settled] : [])]);
24342485
} finally {
24352486
if (timer) {
24362487
clearTimeout(timer);
24372488
}
2489+
quiet?.cancel();
24382490
}
24392491
}
24402492

2493+
/**
2494+
* Settles when the adapter's stdio has delivered nothing for
2495+
* ADAPTER_STDIO_QUIET_MS, counted from the later of the last chunk and the
2496+
* first terminal signal — so a second signal queued behind the first
2497+
* (terminated after exited) does not wait a window of its own.
2498+
*
2499+
* The clock alone is not believed. A worker that was descheduled can find
2500+
* its timer due while output that arrived in the meantime is still unread:
2501+
* timers run before the poll phase that reads it. So once the window has
2502+
* passed, one full turn of the event loop is taken first (setImmediate
2503+
* runs after that poll phase), and the window starts over if the turn
2504+
* brought a chunk.
2505+
*/
2506+
private waitForAdapterStdioQuiet(
2507+
activity: { chunks: number; lastChunkAt: number }
2508+
): { settled: Promise<void>; cancel: () => void } {
2509+
const since = this.firstTerminalSignalAt ?? Date.now();
2510+
// At most one of the two is pending at any moment, so clearing both
2511+
// stops the wait for good.
2512+
let timer: NodeJS.Timeout | undefined;
2513+
let turn: NodeJS.Immediate | undefined;
2514+
const settled = new Promise<void>((resolve) => {
2515+
const check = (): void => {
2516+
const quietFor = Date.now() - Math.max(activity.lastChunkAt, since);
2517+
if (quietFor < ADAPTER_STDIO_QUIET_MS) {
2518+
timer = setTimeout(check, ADAPTER_STDIO_QUIET_MS - quietFor);
2519+
return;
2520+
}
2521+
const chunksBefore = activity.chunks;
2522+
turn = setImmediate(() => {
2523+
if (activity.chunks !== chunksBefore) {
2524+
check();
2525+
return;
2526+
}
2527+
resolve();
2528+
});
2529+
};
2530+
check();
2531+
});
2532+
return {
2533+
settled,
2534+
cancel: () => {
2535+
clearTimeout(timer);
2536+
clearImmediate(turn);
2537+
}
2538+
};
2539+
}
2540+
24412541
/**
24422542
* Drain the DAP client's child-session event chain before forwarding a
24432543
* terminal signal (issue #366): js-debug delivers debuggee output as CHILD
@@ -2498,6 +2598,7 @@ export class DapProxyWorker {
24982598
// point must not open a configuration phase against a program that
24992599
// has ended (#746).
25002600
this.terminalSignalArrived = true;
2601+
this.firstTerminalSignalAt ??= Date.now();
25012602
const tail = this.terminalSignalQueue
25022603
.then(() => this.settleDebuggerOffLaunch())
25032604
.then(task)

‎tests/e2e/mcp-server-smoke-cpp.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,49 @@ describe.skipIf(SKIP_CPP)('MCP Server C/C++ Debugging Smoke Test @requires-cpp',
348348
90000
349349
);
350350

351+
it(
352+
"answers a short program's launch with its end, its exit code and all of its output (issue #856)",
353+
async (ctx) => {
354+
const { binaryPath } = prepareCppExample('hello_world');
355+
356+
const createResponse = parseSdkToolResult(await mcpClient!.callTool({
357+
name: 'create_debug_session',
358+
arguments: { language: 'cpp', name: 'cpp-short-program' }
359+
}));
360+
expect(createResponse.success).toBe(true);
361+
sessionId = createResponse.sessionId as string;
362+
363+
const startResponse = parseSdkToolResult(await mcpClient!.callTool({
364+
name: 'start_debugging',
365+
arguments: { sessionId, scriptPath: binaryPath, dapLaunchArgs: { stopOnEntry: false } }
366+
}));
367+
if (!startResponse.success) {
368+
skipIfSpawnBlocked(ctx, startResponse, 'C/C++');
369+
throw new Error(`start_debugging failed: ${JSON.stringify(startResponse, null, 2)}`);
370+
}
371+
372+
// hello_world is over in milliseconds, so its end is the launch's
373+
// answer. On Windows the proxy used to hold CodeLLDB's `exited` until
374+
// the adapter's stdio pipes closed — which they never do while CodeLLDB
375+
// lives — so every exit was reported 2 s late and this launch answered
376+
// `running` with pending: true for a program that had already ended.
377+
expect(startResponse.state, JSON.stringify(startResponse)).toBe('stopped');
378+
expect(startResponse.pending).toBeUndefined();
379+
expect((startResponse.data as { exitCode?: number } | undefined)?.exitCode).toBe(0);
380+
381+
// Reporting the exit sooner must not cost the program's last words:
382+
// they are still read off the pipe before the exit is forwarded.
383+
const outputResult = await callToolSafely(mcpClient!, 'get_output', { sessionId });
384+
const stdout = ((outputResult.entries ?? []) as Array<{ category?: string; output?: string }>)
385+
.filter(e => e.category === 'stdout')
386+
.map(e => e.output ?? '')
387+
.join('');
388+
expect(stdout).toContain('CPP_DEBUG_MARKER: Hello from C++ answer=42');
389+
expect(stdout).toContain('values has 3 elements');
390+
},
391+
90000
392+
);
393+
351394
it(
352395
'completes a noDebug launch although CodeLLDB refuses the configuration requests (issue #746)',
353396
async (ctx) => {

0 commit comments

Comments
 (0)