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
Binary file modified src/proxy/cdp-function-breakpoint-bridge.ts
Binary file not shown.
10 changes: 9 additions & 1 deletion src/proxy/child-session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,14 @@ export class ChildSessionManager extends EventEmitter {
}

const absolutePath = path.isAbsolute(sourcePath) ? sourcePath : path.resolve(sourcePath);
this.storedBreakpoints.set(absolutePath, breakpoints);
if (breakpoints.length === 0) {
// A fresh child never saw this file's breakpoints, so replaying an
// empty set is a no-op — drop the key instead of retaining every file
// that ever had a breakpoint (issue #405).
this.storedBreakpoints.delete(absolutePath);
} else {
this.storedBreakpoints.set(absolutePath, breakpoints);
}

// Mirror to active child if present
if (this.activeChild) {
Expand Down Expand Up @@ -767,5 +774,6 @@ export class ChildSessionManager extends EventEmitter {
this.childSessions.clear();
this.activeChild = null;
this.adoptedTargets.clear();
this.storedBreakpoints.clear();
}
}
31 changes: 29 additions & 2 deletions src/proxy/dap-proxy-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ export type DapProxyWorkerHooks = {
createTraceFile?: (sessionId: string, logDir: string) => string | undefined;
};

/**
* Cap for the pre-connect and policy command queues (issue #405). Both are
* normally drained within one adapter handshake, so a queue this deep means
* the adapter is wedged — commands past the cap are rejected with an error
* response on their live requestId (silent eviction would hang the client).
*/
export const MAX_QUEUED_COMMANDS = 256;

export class DapProxyWorker {
private logger: ILogger | null = null;
private dapClient: IDapClient | null = null;
Expand Down Expand Up @@ -934,11 +942,21 @@ export class DapProxyWorker {
// Check if we're connected
if (!this.dapClient) {
if (this.state === ProxyState.INITIALIZING) {
if (this.preConnectQueue.length >= MAX_QUEUED_COMMANDS) {
// A wedged adapter never drains this queue; reject instead of
// growing without bound (issue #405). Every queued command holds a
// live requestId, so the overflow must answer, not silently drop.
this.sendDapResponse(
payload.requestId, false, undefined,
`pre-connect queue overflow (${MAX_QUEUED_COMMANDS} commands queued; adapter never became ready)`
);
return;
}
this.preConnectQueue.push(payload);
this.logger?.info(`[Worker] Queued pre-connect DAP command: ${payload.dapCommand}`);
return;
}

this.sendDapResponse(payload.requestId, false, undefined, 'DAP client not connected');
return;
}
Expand Down Expand Up @@ -970,8 +988,17 @@ export class DapProxyWorker {
);

if (handling.shouldQueue) {
if (this.commandQueue.length >= MAX_QUEUED_COMMANDS) {
// Same shape as the pre-connect overflow: reject with an error on
// the live requestId rather than queueing forever (issue #405).
this.sendDapResponse(
payload.requestId, false, undefined,
`command queue overflow (${MAX_QUEUED_COMMANDS} commands queued; adapter is not draining)`
);
return;
}
this.logger!.info(`[Worker] ${handling.reason || 'Queuing command'}`);

// Check if we need to inject configurationDone
const initBehavior = this.adapterPolicy.getInitializationBehavior();
if (handling.shouldDefer && initBehavior.deferConfigDone) {
Expand Down
20 changes: 19 additions & 1 deletion tests/proxy/cdp-function-breakpoint-bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { EventEmitter } from 'events';
import type { DebugProtocol } from '@vscode/debugprotocol';
import { CdpFunctionBreakpointBridge } from '../../src/proxy/cdp-function-breakpoint-bridge.js';
import { CdpFunctionBreakpointBridge, MAX_SCRIPT_URLS } from '../../src/proxy/cdp-function-breakpoint-bridge.js';

type CdpHandler = (params: Record<string, unknown>) => unknown;

Expand Down Expand Up @@ -640,4 +640,22 @@ describe('CdpFunctionBreakpointBridge', () => {
expect((out.body as DebugProtocol.StoppedEvent['body']).reason).toBe('breakpoint');
});
});

describe('scriptUrls cap (issue #405)', () => {
it('evicts the oldest scriptParsed entries past the cap', async () => {
await attach();

const overshoot = 10;
for (let i = 0; i < MAX_SCRIPT_URLS + overshoot; i++) {
cdp.emit('cdp-event', 'Debugger.scriptParsed', { scriptId: `s${i}`, url: `file:///f${i}.js` });
}

const urls = (bridge as unknown as { scriptUrls: Map<string, string> }).scriptUrls;
expect(urls.size).toBe(MAX_SCRIPT_URLS);
// FIFO: the earliest scripts fell out, the newest survive
expect(urls.has('s0')).toBe(false);
expect(urls.has(`s${overshoot - 1}`)).toBe(false);
expect(urls.get(`s${MAX_SCRIPT_URLS + overshoot - 1}`)).toBe(`file:///f${MAX_SCRIPT_URLS + overshoot - 1}.js`);
});
});
});
29 changes: 29 additions & 0 deletions tests/proxy/child-session-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -909,4 +909,33 @@ describe('ChildSessionManager', () => {
expect(bridge.detachCalls).toBe(2);
});
});

describe('stored breakpoint lifecycle (issue #405)', () => {
beforeEach(() => {
manager = new ChildSessionManager({
policy: JsDebugAdapterPolicy,
host: 'localhost',
port: 9229
});
});

it('clears stored breakpoints on shutdown', async () => {
manager.storeBreakpoints('/abs/app.js', [{ line: 1 }]);
manager.storeBreakpoints('/abs/lib.js', [{ line: 2 }]);
expect((manager as any).storedBreakpoints.size).toBe(2);

await manager.shutdown();

expect((manager as any).storedBreakpoints.size).toBe(0);
});

it('deletes the entry when a file clears to zero breakpoints', () => {
manager.storeBreakpoints('/abs/app.js', [{ line: 1 }]);
expect((manager as any).storedBreakpoints.size).toBe(1);

manager.storeBreakpoints('/abs/app.js', []);

expect((manager as any).storedBreakpoints.size).toBe(0);
});
});
});
57 changes: 56 additions & 1 deletion tests/proxy/dap-proxy-worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { EventEmitter } from 'events';
import type { ChildProcess } from 'child_process';
import path from 'path';
import { describe, it, expect, beforeEach, vi, type Mock } from 'vitest';
import { DapProxyWorker } from '../../src/proxy/dap-proxy-worker.js';
import { DapProxyWorker, MAX_QUEUED_COMMANDS } from '../../src/proxy/dap-proxy-worker.js';
import { GenericAdapterManager } from '../../src/proxy/dap-proxy-adapter-manager.js';
import { DapConnectionManager } from '../../src/proxy/dap-proxy-connection-manager.js';
import type {
Expand Down Expand Up @@ -3794,4 +3794,59 @@ describe('DapProxyWorker', () => {
expect(response.error).toContain('not configured');
});
});

describe('bounded worker queues (issue #405)', () => {
const dapPayload = (id: string): DapCommandPayload => ({
cmd: 'dap',
sessionId: 'test-session',
requestId: id,
dapCommand: 'setBreakpoints',
dapArgs: {}
});

const responseFor = (requestId: string) =>
mockMessageSender.send.mock.calls
.map((c) => c[0] as { type?: string; requestId?: string; success?: boolean; error?: string })
.find((m) => m.type === 'dapResponse' && m.requestId === requestId);

it('rejects a command with an error response when the pre-connect queue is full', async () => {
(worker as any).state = ProxyState.INITIALIZING;
(worker as any).dapClient = null;
(worker as any).preConnectQueue = Array.from(
{ length: MAX_QUEUED_COMMANDS },
(_, i) => dapPayload(`preconnect-${i}`)
);

await worker.handleCommand(dapPayload('overflow-pre'));

expect((worker as any).preConnectQueue).toHaveLength(MAX_QUEUED_COMMANDS);
const response = responseFor('overflow-pre');
expect(response?.success).toBe(false);
expect(String(response?.error)).toMatch(/queue/i);
});

it('rejects a command with an error response when the policy command queue is full', async () => {
(worker as any).state = ProxyState.CONNECTED;
(worker as any).dapClient = mockDapClient;
(worker as any).adapterPolicy = {
name: 'queue-test',
shouldQueueCommand: () => ({ shouldQueue: true, shouldDefer: false }),
getInitializationBehavior: () => ({}),
getDapClientBehavior: () => ({})
};
(worker as any).commandQueue = Array.from(
{ length: MAX_QUEUED_COMMANDS },
(_, i) => dapPayload(`queued-${i}`)
);

await worker.handleCommand(dapPayload('overflow-cmd'));

// Overflow must not drain or grow the queue — the queued commands hold
// live requestIds that a silent evict would leave hanging forever.
expect((worker as any).commandQueue).toHaveLength(MAX_QUEUED_COMMANDS);
const response = responseFor('overflow-cmd');
expect(response?.success).toBe(false);
expect(String(response?.error)).toMatch(/queue/i);
});
});
});
Loading