Skip to content

Commit a0af3bf

Browse files
debugmcpdevCI Botclaudecynarlab
authored
fix(#358,#361,#366): output-buffer isolation, LLDB noise filter, child-event flush (#376)
Issue #358 — out-of-order get_output entries after restart_debugging: - Each launch's output handlers are closure-bound to their own OutputRingBuffer instance, so an in-flight event from the previous run (microtask/IPC queue) can never land in the new launch's buffer and take an early seq. noteUnboundFunctionBreakpoints writes to its launch's buffer too. No sort added to read(): seq is monotonic per buffer by construction. Issue #361 — CodeLLDB DWARF-parser stderr noise in get_output: - New optional AdapterPolicy.shouldSuppressOutputEvent(category, text) hook, consulted by the session manager's output capture after the telemetry drop; suppressed lines stay in debug logs, and policy failures never drop output. - lldbShouldSuppressOutputEvent in lldb-policy-shared matches LLDB's DWARF diagnostics (DIE/DW_AT_ranges, DW_TAG_member bounds, wrapped continuation fragments) anchored on DWARF-specific tokens; multi-line events are dropped only when EVERY non-empty line matches. Wired into the rust and cpp policies. Issue #366 — Docker JS logpoint/program output lost at run-to-completion: - js-debug delivers debuggee output as CHILD session events serialized through the CDP-bridge chain; terminated could outrun them and the SessionManager teardown dropped whatever was still queued (reliably all output under Docker's slower scheduling). - New ChildSessionManager.flushEvents() / MinimalDapClient.flushChildEvents(); the proxy worker's terminal slots (exited/terminated/connection-closed) drain the chain (two rounds with a 150ms settle for bytes still unread in the child socket, 2s backstop) before forwarding — output events hit the FIFO IPC ahead of the terminal signal. Only paid by child-session adapters. - Docker e2e now runs the exact issue repro: logpoint + run-to-completion must capture program stdout and the interpolated LOGPOINT line. Fixes #358 Fixes #361 Fixes #366 Co-authored-by: CI Bot <ci@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: JF <john.franklin@gmail.com>
1 parent 8f2983d commit a0af3bf

16 files changed

Lines changed: 398 additions & 10 deletions

‎docs/tool-reference.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,7 @@ Gets the debuggee's output (stdout/stderr/console) captured for a session. Outpu
960960

961961
**Notes:**
962962
- The buffer holds the last 1000 entries per launch; older entries are evicted and counted in `dropped`. Individual entries longer than 8192 characters are cut and flagged `"truncated": true`.
963-
- Adapter-internal `telemetry` events are filtered out at capture time; all other categories (`stdout`, `stderr`, `console`, `important`, ...) are kept. Adapters that omit a category default to `console`.
963+
- Adapter-internal `telemetry` events are filtered out at capture time, as are output events an adapter policy declares to be pure adapter noise — e.g. LLDB's harmless DWARF-parser error spew on MinGW-built rust/cpp binaries (issue #361); suppressed lines remain visible in debug logs. All other categories (`stdout`, `stderr`, `console`, `important`, ...) are kept. Adapters that omit a category default to `console`.
964964
- Works while the program is running and after it finishes — output stays readable until `close_debug_session`. Re-launching a session starts a fresh buffer (seq restarts at 1).
965965
- `hasMore: true` means more entries matched than `limit` allowed; call again with `since: nextSince`.
966966
- Incremental polling recipe: call once, remember `nextSince`, and pass it as `since` on the next call — you'll only ever see new output.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import {
2828
isLldbConnected,
2929
getLldbDapClientBehavior,
3030
isLldbInternalFrame,
31-
filterLldbStackFrames
31+
filterLldbStackFrames,
32+
lldbShouldSuppressOutputEvent
3233
} from './lldb-policy-shared.js';
3334

3435
export const CppAdapterPolicy: AdapterPolicy = {
@@ -47,6 +48,7 @@ export const CppAdapterPolicy: AdapterPolicy = {
4748
},
4849

4950
normalizeStopReason: normalizeLldbStopReason,
51+
shouldSuppressOutputEvent: lldbShouldSuppressOutputEvent,
5052

5153
/**
5254
* Hide LLDB-synthesized symbols and libc/runtime plumbing from stack

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import {
2121
isLldbConnected,
2222
getLldbDapClientBehavior,
2323
isLldbInternalFrame,
24-
filterLldbStackFrames
24+
filterLldbStackFrames,
25+
lldbShouldSuppressOutputEvent
2526
} from './lldb-policy-shared.js';
2627

2728
export const RustAdapterPolicy: AdapterPolicy = {
@@ -55,6 +56,7 @@ export const RustAdapterPolicy: AdapterPolicy = {
5556
* LLDB-backed policy; see normalizeLldbStopReason for the full rules.
5657
*/
5758
normalizeStopReason: normalizeLldbStopReason,
59+
shouldSuppressOutputEvent: lldbShouldSuppressOutputEvent,
5860

5961
/**
6062
* Hide LLDB-synthesized symbols and libc/runtime plumbing from stack

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,20 @@ export interface AdapterPolicy {
166166
*/
167167
isInternalFrame?(frame: StackFrame): boolean;
168168

169+
/**
170+
* Return true to drop a DAP 'output' event that is known adapter-internal
171+
* noise, not debuggee output (issue #361 — e.g. LLDB's DWARF-parsing
172+
* error spew on MinGW binaries). Consulted by the session manager's output
173+
* capture AFTER the standard 'telemetry' category drop; suppressed events
174+
* are still logged at debug level. Optional — absent means capture
175+
* everything. Implementations must be conservative: when in doubt, keep
176+
* the output.
177+
*
178+
* @param category The DAP output category ('stdout', 'stderr', 'console', ...)
179+
* @param text The event's output text (may span multiple lines)
180+
*/
181+
shouldSuppressOutputEvent?(category: string, text: string): boolean;
182+
169183
/**
170184
* Normalize an adapter-specific DAP stop reason into a canonical reason
171185
* ('pause', 'breakpoint', 'step', 'exception', ...). Some adapters report

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,52 @@ export function filterLldbStackFrames(frames: StackFrame[], includeInternals: bo
266266
return frames.filter((frame) => !isLldbInternalFrame(frame));
267267
}
268268

269+
/**
270+
* LLDB's own DWARF-parser diagnostics (issue #361). MinGW/g++-generated
271+
* debug info routinely trips LLDB's DWARF parser into printing multi-line
272+
* "error: ..." spew on stderr ("DIE has DW_AT_ranges ... range extraction
273+
* failed", "DW_TAG_member '...' refers to type ... which extends beyond the
274+
* bounds ..."), even though debugging then works perfectly. These are
275+
* adapter internals, not debuggee output — filter them out of the captured
276+
* output buffer the same way 'telemetry' events are.
277+
*
278+
* Patterns are anchored on DWARF-specific tokens (DIE / DW_AT_ / DW_TAG_ /
279+
* "please file a bug") so genuine program stderr containing the word
280+
* "error:" is never eaten.
281+
*/
282+
const LLDB_DWARF_NOISE_PATTERNS: RegExp[] = [
283+
// "error: <module> [0x...]: DIE has DW_AT_ranges(...) attribute, but ..."
284+
/^error:\s+.*\bDIE\b.*\bDW_AT_/,
285+
// "error: <module> 0x...: DW_TAG_member '...' refers to type ... which extends beyond the bounds ..."
286+
/^error:\s+.*\bDW_TAG_\w+\b/,
287+
// Generic DWARF diagnostic shape: "error: <module> 0x...: ... DW_AT_..."
288+
/^error:\s+\S+.*\bDW_(AT|TAG|FORM)_\w+/,
289+
// Continuation lines of the multi-line DIE diagnostic (LLDB wraps it; the
290+
// pieces can arrive trimmed or even as separate line-buffered events)
291+
/\bplease file a bug and attach the file\b/,
292+
/^start of this error message$/,
293+
/\brange extraction failed\b/,
294+
/\binvalid range list offset\b/,
295+
/\bextends beyond the bounds of 0x[0-9a-f]+\b/
296+
];
297+
298+
/**
299+
* Suppress a DAP output event when it is purely LLDB DWARF-parser noise.
300+
* Multi-line events are dropped only when EVERY non-empty line matches a
301+
* noise pattern — mixed content always passes through untouched (never
302+
* partially rewritten).
303+
*/
304+
export function lldbShouldSuppressOutputEvent(category: string, text: string): boolean {
305+
if (category !== 'stderr' && category !== 'console') {
306+
return false;
307+
}
308+
const lines = text.split('\n').map((line) => line.trim()).filter((line) => line.length > 0);
309+
if (lines.length === 0) {
310+
return false;
311+
}
312+
return lines.every((line) => LLDB_DWARF_NOISE_PATTERNS.some((pattern) => pattern.test(line)));
313+
}
314+
269315
/**
270316
* Validate that a CodeLLDB binary exists and answers --version with output
271317
* containing 'codelldb'.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* LLDB DWARF-parser noise suppression (issue #361).
3+
*/
4+
import { describe, it, expect } from 'vitest';
5+
import { lldbShouldSuppressOutputEvent } from '../../src/interfaces/lldb-policy-shared.js';
6+
import { CppAdapterPolicy } from '../../src/interfaces/adapter-policy-cpp.js';
7+
import { RustAdapterPolicy } from '../../src/interfaces/adapter-policy-rust.js';
8+
9+
const DIE_ERROR_MULTILINE =
10+
"error: hello_world.exe [0x0000000000002293]: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x000000000000000c) attribute,\n" +
11+
' but range extraction failed (invalid range list offset 0xc), please file a bug and attach the file at the\n' +
12+
' start of this error message\n';
13+
14+
const MEMBER_ERROR =
15+
"error: hello_world.exe 0x00002b54: DW_TAG_member '_M_local_buf' refers to type 0x0000000000010ac0 which extends beyond the bounds of 0x00002b4b\n";
16+
17+
describe('lldbShouldSuppressOutputEvent', () => {
18+
it('suppresses the multi-line DIE/DW_AT_ranges diagnostic', () => {
19+
expect(lldbShouldSuppressOutputEvent('stderr', DIE_ERROR_MULTILINE)).toBe(true);
20+
});
21+
22+
it('suppresses the DW_TAG_member bounds diagnostic', () => {
23+
expect(lldbShouldSuppressOutputEvent('stderr', MEMBER_ERROR)).toBe(true);
24+
});
25+
26+
it('suppresses line-buffered fragments of the DIE diagnostic delivered as separate events', () => {
27+
for (const line of DIE_ERROR_MULTILINE.split('\n').filter(l => l.trim())) {
28+
expect(lldbShouldSuppressOutputEvent('stderr', line + '\n'), line).toBe(true);
29+
}
30+
});
31+
32+
it('keeps genuine program stderr, even when it contains the word error:', () => {
33+
expect(lldbShouldSuppressOutputEvent('stderr', 'error: could not open config file\n')).toBe(false);
34+
expect(lldbShouldSuppressOutputEvent('stderr', 'panicked at src/main.rs:10\n')).toBe(false);
35+
});
36+
37+
it('keeps mixed events where any line is not noise (never partially rewrites)', () => {
38+
const mixed = MEMBER_ERROR + 'real program output\n';
39+
expect(lldbShouldSuppressOutputEvent('stderr', mixed)).toBe(false);
40+
});
41+
42+
it('never suppresses stdout or empty events', () => {
43+
expect(lldbShouldSuppressOutputEvent('stdout', MEMBER_ERROR)).toBe(false);
44+
expect(lldbShouldSuppressOutputEvent('stderr', '\n')).toBe(false);
45+
expect(lldbShouldSuppressOutputEvent('stderr', '')).toBe(false);
46+
});
47+
48+
it('is wired into both the rust and cpp policies', () => {
49+
expect(RustAdapterPolicy.shouldSuppressOutputEvent?.('stderr', MEMBER_ERROR)).toBe(true);
50+
expect(CppAdapterPolicy.shouldSuppressOutputEvent?.('stderr', MEMBER_ERROR)).toBe(true);
51+
expect(RustAdapterPolicy.shouldSuppressOutputEvent?.('stdout', 'hello\n')).toBe(false);
52+
});
53+
});

‎src/proxy/child-session-manager.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,16 @@ export class ChildSessionManager extends EventEmitter {
147147
// (correlation/bind window) cannot be overtaken by later events
148148
private childEventChain: Promise<void> = Promise.resolve();
149149

150+
/**
151+
* Resolves when every child event enqueued so far has been forwarded
152+
* (issue #366): output events ride the childEventChain when the CDP bridge
153+
* is active, so a terminal event must not be forwarded upstream until the
154+
* chain has drained or queued output is silently dropped at teardown.
155+
*/
156+
flushEvents(): Promise<void> {
157+
return this.childEventChain;
158+
}
159+
150160
constructor(options: ChildSessionOptions) {
151161
super();
152162
this.policy = options.policy;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,12 @@ export interface IDapClient {
173173
* MinimalDapClient (js-debug child sessions) implements it.
174174
*/
175175
setExceptionBreakMode?(mode: 'uncaught' | 'all' | 'none'): void;
176+
/**
177+
* Resolves once all child-session events enqueued so far (output included)
178+
* have been forwarded (issue #366). Optional: only MinimalDapClient
179+
* (js-debug child sessions) implements it; absent means nothing to flush.
180+
*/
181+
flushChildEvents?(): Promise<void>;
176182
}
177183

178184
/**

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,7 @@ export class DapProxyWorker {
733733
this.exitedEventSeen = true;
734734
return this.enqueueTerminalSignal('exited', async () => {
735735
await this.waitForAdapterStdioDrain();
736+
await this.waitForChildEventFlush();
736737
this.terminalDapEventForwarded = true;
737738
this.sendDapEvent('exited', body);
738739
});
@@ -746,6 +747,7 @@ export class DapProxyWorker {
746747
// until the streams have drained so the output wins the race.
747748
return this.enqueueTerminalSignal('terminated', async () => {
748749
await this.waitForAdapterStdioDrain();
750+
await this.waitForChildEventFlush();
749751
// Must complete before terminated is forwarded: whichever of
750752
// exited/terminated reaches the SessionManager first strips the
751753
// other's handler, and shutdown() below tears down the client
@@ -768,6 +770,7 @@ export class DapProxyWorker {
768770
// hold this path behind the same drain barrier (issue #222).
769771
return this.enqueueTerminalSignal('dap_connection_closed', async () => {
770772
await this.waitForAdapterStdioDrain();
773+
await this.waitForChildEventFlush();
771774
this.sendStatus('dap_connection_closed', { expected: this.isTerminationExpected() });
772775
this.shutdown();
773776
});
@@ -1593,6 +1596,46 @@ export class DapProxyWorker {
15931596
}
15941597
}
15951598

1599+
/**
1600+
* Drain the DAP client's child-session event chain before forwarding a
1601+
* terminal signal (issue #366): js-debug delivers debuggee output as CHILD
1602+
* session events, which are microtask-deferred through the CDP-bridge
1603+
* serialization chain. Forwarding terminated/exited ahead of that chain
1604+
* lets the SessionManager tear down its listeners while output is still
1605+
* queued — silently losing it (reliably so under Docker's slower
1606+
* scheduling). Bounded by the same 2s backstop as the stdio drain so a
1607+
* wedged chain cannot hang shutdown.
1608+
*/
1609+
private async waitForChildEventFlush(): Promise<void> {
1610+
if (typeof this.dapClient?.flushChildEvents !== 'function') {
1611+
return;
1612+
}
1613+
const boundedFlush = async (): Promise<void> => {
1614+
const flush = this.dapClient?.flushChildEvents?.();
1615+
if (!flush) {
1616+
return;
1617+
}
1618+
let timer: NodeJS.Timeout | undefined;
1619+
const backstop = new Promise<void>(resolve => {
1620+
timer = setTimeout(resolve, 2000);
1621+
});
1622+
try {
1623+
await Promise.race([flush, backstop]);
1624+
} finally {
1625+
if (timer) {
1626+
clearTimeout(timer);
1627+
}
1628+
}
1629+
};
1630+
// Two rounds with a short settle: the parent session's terminated can
1631+
// arrive while child output is still UNREAD in the child socket (not on
1632+
// the chain yet). The settle lets those bytes be read and enqueued; the
1633+
// second flush drains them. Only paid by child-session adapters.
1634+
await boundedFlush();
1635+
await new Promise(resolve => setTimeout(resolve, 150));
1636+
await boundedFlush();
1637+
}
1638+
15961639
/**
15971640
* Chain a terminal signal onto the FIFO forwarding queue (issue #258).
15981641
* Every producer awaits the drain barrier, so without serialization the

‎src/proxy/minimal-dap.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,15 @@ export class MinimalDapClient extends EventEmitter {
127127
this.childSessionManager?.setExceptionBreakMode(mode);
128128
}
129129

130+
/**
131+
* Resolves once all child events enqueued so far (output included) have
132+
* been forwarded to this client's listeners (issue #366). No-op for
133+
* adapters without child sessions.
134+
*/
135+
flushChildEvents(): Promise<void> {
136+
return this.childSessionManager?.flushEvents() ?? Promise.resolve();
137+
}
138+
130139
/**
131140
* Handle raw data using the same algorithm as vscode's ProtocolServer
132141
* (extracted to DapFrameDecoder). This ensures compatibility and proper

0 commit comments

Comments
 (0)