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
6 changes: 5 additions & 1 deletion packages/shared/src/interfaces/adapter-policy-ruby.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,11 @@ export const RubyAdapterPolicy: AdapterPolicy = {
// only ever appears on the adapter's pipes (issue #222). rdbg's own
// stderr banners ("DEBUGGER: wait for debugger connection...") are
// excluded from forwarding but still logged.
forwardStdio: { excludeStderrLinePattern: /^DEBUGGER: / }
forwardStdio: { excludeStderrLinePattern: /^DEBUGGER: / },
// For the same reason the adapter process's exit status is the
// debuggee's, and rdbg never sends a DAP exited event — let the
// worker synthesize one (issue #258).
adapterExitCodeIsDebuggeeExitCode: true
};
}
};
10 changes: 10 additions & 0 deletions packages/shared/src/interfaces/adapter-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,16 @@ export type AdapterSpawnConfig =
*/
excludeStderrLinePattern?: RegExp;
};
/**
* The adapter process's exit code IS the debuggee's exit code (issue
* #258): rdbg -c runs the debuggee under the adapter process, so its
* exit status propagates. Lets the worker synthesize a DAP 'exited'
* event for adapters that never send one, giving the session a
* recorded debuggee exit code like Python/js. Deliberately separate
* from forwardStdio: CodeLLDB forwards stdio too, but its exit code
* is its own, not the debuggee's.
*/
adapterExitCodeIsDebuggeeExitCode?: boolean;
}
| {
mode: 'connect';
Expand Down
11 changes: 7 additions & 4 deletions src/dap-core/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,13 @@ function handleStatusMessage(
case 'terminated':
commands.push(
{ type: 'log', level: 'info', message: `[ProxyManager] Status: ${message.status}` },
{
type: 'emitEvent',
event: 'exit',
args: [message.code || 1, message.signal || undefined]
{
type: 'emitEvent',
event: 'exit',
// Pass the code through untouched (issue #258): only adapter_exited
// carries one, and fabricating 1 for the codeless closure statuses
// turned every clean rdbg run into a session error.
args: [message.code ?? null, message.signal || undefined, message.expected]
}
);
break;
Expand Down
2 changes: 1 addition & 1 deletion src/dap-core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export type ProxyStatusMessage =
| { type: 'status'; sessionId: string; status: 'adapter_connected'; data?: unknown }
| { type: 'status'; sessionId: string; status: 'adapter_configured_and_launched'; data?: unknown }
| { type: 'status'; sessionId: string; status: 'adapter_capabilities'; capabilities: DebugProtocol.Capabilities; data?: unknown }
| { type: 'status'; sessionId: string; status: 'adapter_exited' | 'dap_connection_closed' | 'terminated'; code?: number | null; signal?: NodeJS.Signals | null; data?: unknown };
| { type: 'status'; sessionId: string; status: 'adapter_exited' | 'dap_connection_closed' | 'terminated'; code?: number | null; signal?: NodeJS.Signals | null; expected?: boolean; data?: unknown };

export type ProxyDapEventMessage = {
type: 'dapEvent';
Expand Down
7 changes: 7 additions & 0 deletions src/proxy/dap-proxy-interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ export interface StatusMessage extends ProxyMessage {
script?: string;
/** Adapter initialize response body, on 'adapter_capabilities' (issue #243) */
capabilities?: DebugProtocol.Capabilities;
/**
* On terminal statuses (issue #258): true when the worker had already seen
* orderly debuggee termination (a terminated/exited DAP event was forwarded
* or shutdown was underway), so the parent can distinguish a normal
* teardown from an adapter dying or dropping the socket mid-run.
*/
expected?: boolean;
}

export interface DapResponseMessage extends ProxyMessage {
Expand Down
148 changes: 131 additions & 17 deletions src/proxy/dap-proxy-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ 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;
// 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
// codeless dap_connection_closed outrunning terminated makes the parent
// end the session as an error (issue #258).
private terminalSignalQueue: Promise<void> = Promise.resolve();
/** True once an exited/terminated DAP event was forwarded to the parent (issue #258). */
private terminalDapEventForwarded: boolean = false;
// Adapter-exit exitCode synthesis (issue #258): armed by policies whose
// adapter process exit status IS the debuggee's (rdbg -c). The exit is
// recorded synchronously so the terminated slot can synthesize a DAP
// exited event even though rdbg never sends one.
private adapterExitCodeIsDebuggeeExitCode: boolean = false;
private adapterExitCode: number | null | undefined = undefined;
private adapterExitSynthesisAttempted: boolean = false;
private initializedEventPending: boolean = false;
private deferInitializedHandling: boolean = false;
private initializedEventHandled: boolean = false;
Expand Down Expand Up @@ -386,6 +401,7 @@ export class DapProxyWorker {
if (spawnConfig.forwardStdio) {
this.adapterStdioDrained = this.createStdioDrainBarrier(spawnResult.process);
}
this.adapterExitCodeIsDebuggeeExitCode = spawnConfig.adapterExitCodeIsDebuggeeExitCode === true;
this.logger!.info(`[Worker] Adapter spawned with PID: ${spawnResult.pid}`);

this.adapterProcess.on('error', (err) => {
Expand All @@ -395,7 +411,13 @@ export class DapProxyWorker {

this.adapterProcess.on('exit', (code, signal) => {
this.logger!.info(`[Worker] Adapter process exited. Code: ${code}, Signal: ${signal}`);
this.sendStatus('adapter_exited', { code, signal });
// Recorded synchronously: the terminated slot's synthesis wait may
// resolve on this very event (issue #258)
this.adapterExitCode = code;
this.enqueueTerminalSignal('adapter_exited', async () => {
await this.waitForAdapterStdioDrain();
this.sendStatus('adapter_exited', { code, signal, expected: this.isTerminationExpected() });
});
});
} else {
// connect mode: an external DAP server is already listening (e.g. remote
Expand Down Expand Up @@ -650,42 +672,51 @@ export class DapProxyWorker {
this.logger!.debug('[Worker] DAP event: thread', body);
this.sendDapEvent('thread', body);
},
onExited: async (body) => {
onExited: (body) => {
this.logger!.info(`[Worker] DAP event: exited exitCode=${body.exitCode}`);
// A real exited event stays authoritative - suppress synthesis (issue #247).
// Set synchronously, before any await, so a racing terminated sees it.
this.exitedEventSeen = true;
await this.waitForAdapterStdioDrain();
this.sendDapEvent('exited', body);
return this.enqueueTerminalSignal('exited', async () => {
await this.waitForAdapterStdioDrain();
this.terminalDapEventForwarded = true;
this.sendDapEvent('exited', body);
});
},
onTerminated: async (body) => {
onTerminated: (body) => {
this.logger!.info(`[Worker] DAP event: terminated body=${JSON.stringify(body)}`);
// When adapter stdio is forwarded as debuggee output, the exit-time
// flush of a block-buffered pipe arrives milliseconds AFTER the DAP
// terminated event — but the SessionManager reacts to terminated by
// stopping the proxy, which drops late messages. Hold terminated
// until the streams have drained so the output wins the race.
await this.waitForAdapterStdioDrain();
// Must complete before terminated is forwarded: whichever of
// exited/terminated reaches the SessionManager first strips the
// other's handler, and shutdown() below tears down the client
await this.maybeSynthesizeExitedEvent();
this.sendDapEvent('terminated', body);
this.shutdown();
return this.enqueueTerminalSignal('terminated', async () => {
await this.waitForAdapterStdioDrain();
// Must complete before terminated is forwarded: whichever of
// exited/terminated reaches the SessionManager first strips the
// other's handler, and shutdown() below tears down the client
await this.maybeSynthesizeExitedEvent();
await this.maybeSynthesizeExitedFromAdapterExit();
this.terminalDapEventForwarded = true;
this.sendDapEvent('terminated', body);
this.shutdown();
});
},
onError: (err) => {
this.logger!.error('[Worker] DAP client error:', err);
this.sendError(`DAP client error: ${err.message}`);
},
onClose: async () => {
onClose: () => {
this.logger!.info('[Worker] DAP client connection closed.');
// Adapters that close the DAP socket at debuggee exit (rdbg) race the
// exit-time stdio flush exactly like terminated does — the parent
// reacts to dap_connection_closed by tearing down its listeners, so
// hold this path behind the same drain barrier (issue #222).
await this.waitForAdapterStdioDrain();
this.sendStatus('dap_connection_closed');
this.shutdown();
return this.enqueueTerminalSignal('dap_connection_closed', async () => {
await this.waitForAdapterStdioDrain();
this.sendStatus('dap_connection_closed', { expected: this.isTerminationExpected() });
this.shutdown();
});
}
});
}
Expand Down Expand Up @@ -1042,7 +1073,7 @@ export class DapProxyWorker {
}

await this.shutdown();
this.sendStatus('terminated');
this.sendStatus('terminated', { expected: true });
}

/**
Expand Down Expand Up @@ -1191,6 +1222,60 @@ export class DapProxyWorker {
}
}

/**
* Synthesize a DAP 'exited' event from the adapter process's exit status
* (issue #258). Only for policies that declare
* adapterExitCodeIsDebuggeeExitCode — rdbg -c runs the debuggee under the
* adapter process, so its exit status propagates, and rdbg never sends an
* exited event of its own. The process usually dies moments after the
* terminated event, so wait briefly for its exit if it hasn't landed yet.
* On signal kill there is no code and the exitCode simply stays unknown,
* matching the js synthesis path.
*/
private async maybeSynthesizeExitedFromAdapterExit(): Promise<void> {
if (
this.exitedEventSeen ||
this.adapterExitSynthesisAttempted ||
!this.adapterExitCodeIsDebuggeeExitCode ||
!this.adapterProcess
) {
return;
}
this.adapterExitSynthesisAttempted = true;

if (this.adapterExitCode === undefined) {
const proc = this.adapterProcess;
if (typeof proc.exitCode === 'number') {
this.adapterExitCode = proc.exitCode;
} else if (proc.signalCode) {
this.adapterExitCode = null;
} else {
await new Promise<void>((resolve) => {
const onExit = () => {
clearTimeout(timer);
resolve();
};
const timer = setTimeout(() => {
proc.removeListener('exit', onExit);
resolve();
}, 500);
// The recording 'exit' listener was registered first, so
// adapterExitCode is already set when this one resolves
proc.once('exit', onExit);
});
}
}

const exitCode = this.adapterExitCode;
if (typeof exitCode === 'number') {
this.exitedEventSeen = true;
this.logger?.info?.(`[Worker] Synthesizing 'exited' from adapter process exit code ${exitCode} (issue #258)`);
this.sendDapEvent('exited', { exitCode });
} else {
this.logger?.info?.('[Worker] Adapter exit code unavailable (signal kill or still running); exitCode stays unknown');
}
}

/**
* Resolves when both adapter stdio streams have closed — i.e. every byte
* the debuggee flushed on exit has been read and forwarded (issue #222).
Expand Down Expand Up @@ -1233,6 +1318,35 @@ export class DapProxyWorker {
}
}

/**
* Chain a terminal signal onto the FIFO forwarding queue (issue #258).
* Every producer awaits the drain barrier, so without serialization the
* signal with the fewest awaits after the barrier wins — not the one that
* arrived first. Each slot is bounded (drain backstop 2s), and the chain
* swallows rejections so one failed slot cannot block the next.
*/
private enqueueTerminalSignal(label: string, task: () => Promise<void>): Promise<void> {
const tail = this.terminalSignalQueue.then(task).catch((err) => {
this.logger?.error(`[Worker] Terminal signal '${label}' failed:`, err);
});
this.terminalSignalQueue = tail;
return tail;
}

/**
* Whether a terminal status sent now reflects orderly debuggee termination
* (issue #258): a terminated/exited DAP event was already forwarded, or the
* worker itself initiated shutdown. False means the adapter died or dropped
* the socket mid-run — the parent maps that to a session error.
*/
private isTerminationExpected(): boolean {
return (
this.terminalDapEventForwarded ||
this.state === ProxyState.SHUTTING_DOWN ||
this.state === ProxyState.TERMINATED
);
}

/**
* Build the raw-stdio → DAP 'output' forwarder for adapters whose debuggee
* inherits the adapter process's stdio (issue #222: rdbg -c on all
Expand Down
15 changes: 13 additions & 2 deletions src/proxy/proxy-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ export interface ProxyManagerEvents {
'initialized': () => void;
'init-received': () => void;
'error': (error: Error) => void;
'exit': (code: number | null, signal?: string) => void;
/**
* Proxy or adapter teardown. `expected` is set on status-driven exits
* (issue #258): true = the worker saw orderly debuggee termination first;
* false = the adapter died or dropped the socket mid-run; undefined = the
* proxy process itself exited (legacy path).
*/
'exit': (code: number | null, signal?: string, expected?: boolean) => void;

// Status events
'dry-run-complete': (command: string, script: string) => void;
Expand Down Expand Up @@ -888,6 +894,9 @@ export class ProxyManager extends EventEmitter implements IProxyManager {
// Skip emitEvent commands for DAP events — they are already handled
// by the fast-path handleDapEvent() call above to avoid double emission.
if (message.type === 'dapEvent') break;
// Terminal statuses are emitted (and latched) by
// handleStatusMessage above — suppress the duplicate (issue #258).
if (command.event === 'exit' && this.exitEmitted) break;
const args = (command.args as unknown[]) ?? [];
this.emit(command.event as keyof ProxyManagerEvents, ...(args as never[]));
}
Expand Down Expand Up @@ -1078,7 +1087,9 @@ export class ProxyManager extends EventEmitter implements IProxyManager {
this.logger.info(`[ProxyManager] Status: ${message.status}`);
if (!this.exitEmitted) {
this.exitEmitted = true;
this.emit('exit', message.code ?? 1, message.signal || undefined);
// No fabricated code (issue #258): the closure statuses are
// codeless, and inventing 1 made every clean rdbg run an error.
this.emit('exit', message.code ?? null, message.signal || undefined, message.expected);
}
break;
}
Expand Down
33 changes: 26 additions & 7 deletions src/session/session-manager-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -537,18 +537,37 @@ export abstract class SessionManagerCore extends EventEmitter {
handlers.set('error', handleError);

// Named function for exit event
const handleExit = (code: number | null, signal?: string) => {
this.logger.debug(`[SessionManager] handleExit: session=${sessionId} currentState=${session.state} code=${code} signal=${signal}`);
this.logger.info(`[ProxyManager ${sessionId}] Exit: code=${code}, signal=${signal}`);
const handleExit = (code: number | null, signal?: string, expected?: boolean) => {
this.logger.debug(`[SessionManager] handleExit: session=${sessionId} currentState=${session.state} code=${code} signal=${signal} expected=${expected}`);
this.logger.info(`[ProxyManager ${sessionId}] Exit: code=${code}, signal=${signal}, expected=${expected}`);
if (session.state !== SessionState.STOPPED && session.state !== SessionState.ERROR) {
// Clean exit (code 0 or null with no signal) is normal termination, not an error
if (code === 0 || (code === null && !signal)) {
if (expected === true) {
// Orderly debuggee termination (issue #258): the worker saw a
// terminated/exited DAP event or was already shutting down. A
// non-zero code here is the debuggee's own exit status (rdbg -c
// propagates it) — a normal debugging outcome, not an error.
if (typeof code === 'number' && session.exitCode === undefined) {
session.exitCode = code;
}
this._updateSessionState(session, SessionState.STOPPED);
} else if (expected === false) {
// The adapter died or dropped the socket with no preceding
// terminal DAP event. Only a clean code 0 counts as normal.
this._updateSessionState(
session,
code === 0 ? SessionState.STOPPED : SessionState.ERROR
);
} else {
this._updateSessionState(session, SessionState.ERROR);
// Legacy path (real proxy-process exit): clean exit is code 0 or
// null with no signal; anything else is an infrastructure error.
if (code === 0 || (code === null && !signal)) {
this._updateSessionState(session, SessionState.STOPPED);
} else {
this._updateSessionState(session, SessionState.ERROR);
}
}
}

// Clean up listeners since proxy is gone
this.cleanupProxyEventHandlers(session, proxyManager);
session.proxyManager = undefined;
Expand Down
3 changes: 2 additions & 1 deletion tests/adapters/ruby/unit/adapter-policy-ruby.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ describe('RubyAdapterPolicy.getAdapterSpawnConfig', () => {
port: 4711,
logDir: '/tmp/logs',
env: { FOO: 'bar' },
forwardStdio: { excludeStderrLinePattern: /^DEBUGGER: / }
forwardStdio: { excludeStderrLinePattern: /^DEBUGGER: / },
adapterExitCodeIsDebuggeeExitCode: true
});
});

Expand Down
Loading
Loading