Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/856.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +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)
12 changes: 12 additions & 0 deletions packages/shared/src/interfaces/adapter-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,18 @@ export type AdapterSpawnConfig =
* debuggee output.
*/
excludeStderrLinePattern?: RegExp;
/**
* The adapter process stays alive after the debuggee has ended, and
* keeps these pipes open with it (CodeLLDB; issue #856). The worker
* holds the debuggee's exit until the forwarded output has drained,
* and by default "drained" means the pipes closed — true of rdbg -c,
* which exits with its debuggee, and never true of an adapter like
* this one, for which the wait would run to its backstop every time.
* With this set, the pipes going quiet counts as drained: when the
* adapter reports the exit the process is gone and everything it
* wrote is already in the pipe.
*/
adapterOutlivesDebuggee?: boolean;
};
/**
* The adapter process's exit code IS the debuggee's exit code (issue
Expand Down
5 changes: 4 additions & 1 deletion packages/shared/src/interfaces/lldb-policy-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,16 @@ export function matchesLldbAdapterCommand(adapterCommand: { command: string; arg
* STDOUT/STDERR process broadcasts), LLDB on Windows lets the debuggee
* inherit the adapter process's pipes. Forward those as output events there;
* on POSIX the channels are exclusive, so this stays off to avoid noise.
* CodeLLDB outlives the debuggee and holds those pipes open until the session
* ends, which the forwarding is told so that a program's exit is not held
* back waiting for a close that only teardown brings (issue #856).
*/
export function buildLldbSpawnConfig(
payload: AdapterSpawnPayload,
platform: NodeJS.Platform = process.platform,
arch: NodeJS.Architecture = process.arch
): AdapterSpawnConfig {
const forwardStdio = platform === 'win32' ? {} : undefined;
const forwardStdio = platform === 'win32' ? { adapterOutlivesDebuggee: true } : undefined;

// If a custom adapter command was provided, use it directly
if (payload.adapterCommand) {
Expand Down
3 changes: 2 additions & 1 deletion packages/shared/tests/unit/adapter-policy-cpp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ describe('CppAdapterPolicy', () => {
args: ['--port', '4711'],
port: 4711
});
expect((config as { forwardStdio?: unknown }).forwardStdio).toEqual({});
// CodeLLDB outlives the debuggee, so its pipes never close with it (issue #856).
expect((config as { forwardStdio?: unknown }).forwardStdio).toEqual({ adapterOutlivesDebuggee: true });
});

it('omits stdio forwarding on POSIX', () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/shared/tests/unit/adapter-policy-rust.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,13 +494,14 @@ describe('RustAdapterPolicy', () => {
const win = asSpawn(
RustAdapterPolicy.getAdapterSpawnConfig(basePayload, 'win32', 'x64')
);
expect(win.forwardStdio).toEqual({});
// CodeLLDB outlives the debuggee, so its pipes never close with it (issue #856).
expect(win.forwardStdio).toEqual({ adapterOutlivesDebuggee: true });

const winCustom = asSpawn(RustAdapterPolicy.getAdapterSpawnConfig({
...basePayload,
adapterCommand: { command: 'custom', args: [] }
}, 'win32', 'x64'));
expect(winCustom.forwardStdio).toEqual({});
expect(winCustom.forwardStdio).toEqual({ adapterOutlivesDebuggee: true });

// POSIX: CodeLLDB emits DAP output events itself (LLDB holds the pipes)
const linux = asSpawn(
Expand Down
111 changes: 106 additions & 5 deletions src/proxy/dap-proxy-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ export const MAX_QUEUED_COMMANDS = 256;
export const INITIALIZE_RESPONSE_GRACE_MS = 2000;
/** Bound on each terminal-signal wait (stdio drain, child flush, the debugger-off launch outcome). */
const TERMINAL_SIGNAL_BACKSTOP_MS = 2000;
/**
* How long the stdio pipes of an adapter that outlives its debuggee must stay
* silent, after the debuggee's end was reported, to count as drained (issue
* #856). Sized from CodeLLDB on Windows printing 9 MB as it exits: the
* longest pause between two consecutive chunks of that output was 14 ms on an
* idle machine and 39 ms with all 16 cores busy (and the proxy logging every
* line at debug level). The clock is also not trusted alone — see
* waitForAdapterStdioQuiet.
*/
const ADAPTER_STDIO_QUIET_MS = 100;
/**
* How long the pre-detach resume (issue #763) may wait for the adapter's
* continue response before the disconnect goes out regardless. Budgeted
Expand Down Expand Up @@ -115,6 +125,14 @@ export class DapProxyWorker {
private exitSynthesisAttempted: boolean = false;
/** Armed when adapter stdio forwarding is active (issue #222): resolves once both stdio streams close. */
private adapterStdioDrained: Promise<void> | null = null;
/**
* Armed as well when the adapter outlives its debuggee (issue #856), whose
* pipes do not close when the program ends: every chunk either stream
* delivers is counted and timed, so the drain can settle on silence.
*/
private adapterStdioActivity: { chunks: number; lastChunkAt: number } | null = null;
/** When the first terminal signal arrived: the silence that counts is measured from here. */
private firstTerminalSignalAt: number | null = null;
// Terminal signals (exited/terminated DAP events, socket close, adapter
// process exit) all await the stdio drain barrier; without a queue their
// continuations resume in an order set by await counts, not arrival — a
Expand Down Expand Up @@ -529,6 +547,9 @@ export class DapProxyWorker {
this.adapterProcess = spawnResult.process;
if (spawnConfig.forwardStdio) {
this.adapterStdioDrained = this.createStdioDrainBarrier(spawnResult.process);
if (spawnConfig.forwardStdio.adapterOutlivesDebuggee) {
this.adapterStdioActivity = this.trackStdioActivity(spawnResult.process);
}
}
this.adapterExitCodeIsDebuggeeExitCode = spawnConfig.adapterExitCodeIsDebuggeeExitCode === true;
this.logger!.info(`[Worker] Adapter spawned with PID: ${spawnResult.pid}`);
Expand Down Expand Up @@ -2412,14 +2433,43 @@ export class DapProxyWorker {
]).then(() => undefined);
}

/**
* Count and time every chunk the adapter's stdio delivers (issue #856).
* Registered after the adapter manager's own 'data' listeners, so by the
* time a chunk is counted the complete lines in it have been forwarded. A
* trailing fragment with no newline is another matter: it stays in the
* manager's line buffer until the pipe closes, which for an adapter that
* outlives its debuggee is teardown — too late, now as before (issue #860).
*/
private trackStdioActivity(adapterProcess: ChildProcess): { chunks: number; lastChunkAt: number } {
const activity = { chunks: 0, lastChunkAt: 0 };
const note = () => {
activity.chunks += 1;
activity.lastChunkAt = Date.now();
};
adapterProcess.stdout?.on('data', note);
adapterProcess.stderr?.on('data', note);
return activity;
}

/**
* Hold exited/terminated forwarding until adapter stdio has drained: a
* debuggee printing to a block-buffered pipe flushes everything at exit,
* milliseconds after the adapter's terminated event, and the SessionManager
* stops the proxy on terminated — dropping late messages. Stream 'data'
* fires before 'close' and IPC is FIFO, so waiting here guarantees the
* forwarded output reaches the session buffer first. 2s backstop for
* adapters that never close their pipes. No-op when forwarding is off.
* stops the proxy on terminated — dropping late messages. When the wait
* ends on the pipes closing, the order is guaranteed: stream 'data' fires
* before 'close' and IPC is FIFO, so the forwarded output reaches the
* session buffer first. When it ends on silence the order rests on a
* measurement instead (ADAPTER_STDIO_QUIET_MS), and at the backstop on
* nothing.
*
* Drained means the pipes closed — or, for an adapter that outlives its
* debuggee and so never closes them when the program ends, that they went
* quiet (issue #856: CodeLLDB on Windows used to sit out the backstop at
* the end of every program, reporting each exit 2 s late). The 2 s
* backstop still bounds both: a pipe that neither closes nor falls silent
* (something the program started is still writing to it) is not waited
* for longer than before. No-op when forwarding is off.
*/
private async waitForAdapterStdioDrain(): Promise<void> {
if (!this.adapterStdioDrained) {
Expand All @@ -2429,15 +2479,65 @@ export class DapProxyWorker {
const backstop = new Promise<void>(resolve => {
timer = setTimeout(resolve, TERMINAL_SIGNAL_BACKSTOP_MS);
});
const quiet = this.adapterStdioActivity ? this.waitForAdapterStdioQuiet(this.adapterStdioActivity) : undefined;
try {
await Promise.race([this.adapterStdioDrained, backstop]);
await Promise.race([this.adapterStdioDrained, backstop, ...(quiet ? [quiet.settled] : [])]);
} finally {
if (timer) {
clearTimeout(timer);
}
quiet?.cancel();
}
}

/**
* Settles when the adapter's stdio has delivered nothing for
* ADAPTER_STDIO_QUIET_MS, counted from the later of the last chunk and the
* first terminal signal — so a second signal queued behind the first
* (terminated after exited) does not wait a window of its own.
*
* The clock alone is not believed. A worker that was descheduled can find
* its timer due while output that arrived in the meantime is still unread:
* timers run before the poll phase that reads it. So once the window has
* passed, one full turn of the event loop is taken first (setImmediate
* runs after that poll phase), and the window starts over if the turn
* brought a chunk.
*/
private waitForAdapterStdioQuiet(
activity: { chunks: number; lastChunkAt: number }
): { settled: Promise<void>; cancel: () => void } {
const since = this.firstTerminalSignalAt ?? Date.now();
// At most one of the two is pending at any moment, so clearing both
// stops the wait for good.
let timer: NodeJS.Timeout | undefined;
let turn: NodeJS.Immediate | undefined;
const settled = new Promise<void>((resolve) => {
const check = (): void => {
const quietFor = Date.now() - Math.max(activity.lastChunkAt, since);
if (quietFor < ADAPTER_STDIO_QUIET_MS) {
timer = setTimeout(check, ADAPTER_STDIO_QUIET_MS - quietFor);
return;
}
const chunksBefore = activity.chunks;
turn = setImmediate(() => {
if (activity.chunks !== chunksBefore) {
check();
return;
}
resolve();
});
};
check();
});
return {
settled,
cancel: () => {
clearTimeout(timer);
clearImmediate(turn);
}
};
}

/**
* Drain the DAP client's child-session event chain before forwarding a
* terminal signal (issue #366): js-debug delivers debuggee output as CHILD
Expand Down Expand Up @@ -2498,6 +2598,7 @@ export class DapProxyWorker {
// point must not open a configuration phase against a program that
// has ended (#746).
this.terminalSignalArrived = true;
this.firstTerminalSignalAt ??= Date.now();
const tail = this.terminalSignalQueue
.then(() => this.settleDebuggerOffLaunch())
.then(task)
Expand Down
43 changes: 43 additions & 0 deletions tests/e2e/mcp-server-smoke-cpp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,49 @@ describe.skipIf(SKIP_CPP)('MCP Server C/C++ Debugging Smoke Test @requires-cpp',
90000
);

it(
"answers a short program's launch with its end, its exit code and all of its output (issue #856)",
async (ctx) => {
const { binaryPath } = prepareCppExample('hello_world');

const createResponse = parseSdkToolResult(await mcpClient!.callTool({
name: 'create_debug_session',
arguments: { language: 'cpp', name: 'cpp-short-program' }
}));
expect(createResponse.success).toBe(true);
sessionId = createResponse.sessionId as string;

const startResponse = parseSdkToolResult(await mcpClient!.callTool({
name: 'start_debugging',
arguments: { sessionId, scriptPath: binaryPath, dapLaunchArgs: { stopOnEntry: false } }
}));
if (!startResponse.success) {
skipIfSpawnBlocked(ctx, startResponse, 'C/C++');
throw new Error(`start_debugging failed: ${JSON.stringify(startResponse, null, 2)}`);
}

// hello_world is over in milliseconds, so its end is the launch's
// answer. On Windows the proxy used to hold CodeLLDB's `exited` until
// the adapter's stdio pipes closed — which they never do while CodeLLDB
// lives — so every exit was reported 2 s late and this launch answered
// `running` with pending: true for a program that had already ended.
expect(startResponse.state, JSON.stringify(startResponse)).toBe('stopped');
expect(startResponse.pending).toBeUndefined();
expect((startResponse.data as { exitCode?: number } | undefined)?.exitCode).toBe(0);

// Reporting the exit sooner must not cost the program's last words:
// they are still read off the pipe before the exit is forwarded.
const outputResult = await callToolSafely(mcpClient!, 'get_output', { sessionId });
const stdout = ((outputResult.entries ?? []) as Array<{ category?: string; output?: string }>)
.filter(e => e.category === 'stdout')
.map(e => e.output ?? '')
.join('');
expect(stdout).toContain('CPP_DEBUG_MARKER: Hello from C++ answer=42');
expect(stdout).toContain('values has 3 elements');
},
90000
);

it(
'completes a noDebug launch although CodeLLDB refuses the configuration requests (issue #746)',
async (ctx) => {
Expand Down
Loading
Loading