diff --git a/src/proxy/cdp-function-breakpoint-bridge.ts b/src/proxy/cdp-function-breakpoint-bridge.ts index 6ae4004e..e685d0c7 100644 Binary files a/src/proxy/cdp-function-breakpoint-bridge.ts and b/src/proxy/cdp-function-breakpoint-bridge.ts differ diff --git a/src/proxy/child-session-manager.ts b/src/proxy/child-session-manager.ts index c770deae..10fed33f 100644 --- a/src/proxy/child-session-manager.ts +++ b/src/proxy/child-session-manager.ts @@ -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) { @@ -767,5 +774,6 @@ export class ChildSessionManager extends EventEmitter { this.childSessions.clear(); this.activeChild = null; this.adoptedTargets.clear(); + this.storedBreakpoints.clear(); } } diff --git a/src/proxy/dap-proxy-worker.ts b/src/proxy/dap-proxy-worker.ts index 19193424..310fe8f7 100644 --- a/src/proxy/dap-proxy-worker.ts +++ b/src/proxy/dap-proxy-worker.ts @@ -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; @@ -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; } @@ -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) { diff --git a/tests/proxy/cdp-function-breakpoint-bridge.test.ts b/tests/proxy/cdp-function-breakpoint-bridge.test.ts index 62d5615d..2a4a967c 100644 --- a/tests/proxy/cdp-function-breakpoint-bridge.test.ts +++ b/tests/proxy/cdp-function-breakpoint-bridge.test.ts @@ -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) => unknown; @@ -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 }).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`); + }); + }); }); diff --git a/tests/proxy/child-session-manager.test.ts b/tests/proxy/child-session-manager.test.ts index 220017d9..e5010b98 100644 --- a/tests/proxy/child-session-manager.test.ts +++ b/tests/proxy/child-session-manager.test.ts @@ -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); + }); + }); }); diff --git a/tests/proxy/dap-proxy-worker.test.ts b/tests/proxy/dap-proxy-worker.test.ts index dce15dad..cc438e3c 100644 --- a/tests/proxy/dap-proxy-worker.test.ts +++ b/tests/proxy/dap-proxy-worker.test.ts @@ -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 { @@ -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); + }); + }); });