fix(dap): close tcp connect race and wake pending requests on transport end
#spawnTcp reserved a port, released it, spawned the adapter, then immediately connected. On WSL2 with networkingMode=mirrored the Windows relay keeps accepting connections to the just-released reservation port for tens of ms, so the first connect bound a ghost socket instead of js-debug. Gate the connect on the adapter announcing its listening port on stdout (as vscode-js-debug does from inside its listen callback) so we only connect once the child owns the port; this also drains stdout, which nothing else consumes in tcp mode. Separately, the message reader only rejected pending requests from its catch, so a clean stream end (the ghost socket dropped after we wrote initialize) left requests and event waiters pending until their own timeout. Route both the reader end and adapter exit through a single #failConnection that rejects pending requests and wakes event waiters, turning every transport failure into an immediate "DAP connection closed" error instead of a silent 30s timeout. Fixes #6055
This commit is contained in:
roboomp
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed JS/TS `debug` launches timing out on WSL2 with `networkingMode=mirrored` by waiting for the adapter's listening banner before connecting (avoiding the ghost-accept window on a just-released reservation port) and rejecting pending DAP requests and event waiters the moment the transport closes, so any transport failure surfaces as an immediate `DAP connection closed` error instead of a silent 30s timeout ([#6055](https://github.com/can1357/oh-my-pi/issues/6055)).
|
||||
|
||||
## [17.0.5] - 2026-07-18
|
||||
|
||||
### Added
|
||||
|
||||
@@ -75,6 +75,10 @@ export class DapClient {
|
||||
#reverseRequestHandlers = new Map<string, DapReverseRequestHandler>();
|
||||
#adapterExited = false;
|
||||
#pendingWriteExitRejectors = new Set<() => void>();
|
||||
/** Rejectors for in-flight {@link waitForEvent} calls, woken when the
|
||||
* transport closes so an event that can never arrive fails fast instead of
|
||||
* waiting out its own timeout. */
|
||||
#eventWaiterRejectors = new Set<(error: Error) => void>();
|
||||
|
||||
constructor(
|
||||
adapter: DapResolvedAdapter,
|
||||
@@ -188,12 +192,17 @@ export class DapClient {
|
||||
});
|
||||
|
||||
try {
|
||||
const { readable, writeSink, socket } = await waitForTcpTransport(
|
||||
host,
|
||||
port,
|
||||
socketReadyTimeoutMs ?? SOCKET_READY_TIMEOUT_MS,
|
||||
proc,
|
||||
);
|
||||
// Wait for the adapter to announce it is listening on `port` before
|
||||
// connecting. Without this gate the first connect can land in the
|
||||
// window where a just-released reservation port still accepts
|
||||
// connections (WSL2 mirrored networking, issue #6055): the transport
|
||||
// then binds to a ghost of the reservation listener instead of the
|
||||
// adapter. Draining stdout here also avoids a pipe-buffer deadlock —
|
||||
// in tcp mode the DAP protocol flows over the socket, so nothing else
|
||||
// consumes the adapter's stdout.
|
||||
const readyTimeoutMs = socketReadyTimeoutMs ?? SOCKET_READY_TIMEOUT_MS;
|
||||
await waitForTcpServerListening(proc, port, readyTimeoutMs);
|
||||
const { readable, writeSink, socket } = await waitForTcpTransport(host, port, readyTimeoutMs, proc);
|
||||
const client = new DapClient(adapter, cwd, proc, { readable, writeSink, socket, port });
|
||||
proc.exited.then(() => client.#handleProcessExit());
|
||||
void client.#startMessageReader();
|
||||
@@ -389,6 +398,7 @@ export class DapClient {
|
||||
let timeout: NodeJS.Timeout | undefined;
|
||||
const cleanup = () => {
|
||||
unsubscribe();
|
||||
this.#eventWaiterRejectors.delete(closeHandler);
|
||||
if (timeout) clearTimeout(timeout);
|
||||
if (signal) {
|
||||
signal.removeEventListener("abort", abortHandler);
|
||||
@@ -398,6 +408,10 @@ export class DapClient {
|
||||
cleanup();
|
||||
reject(signal?.reason instanceof Error ? signal.reason : new ToolAbortError());
|
||||
};
|
||||
const closeHandler = (error: Error) => {
|
||||
cleanup();
|
||||
reject(error);
|
||||
};
|
||||
const unsubscribe = this.onEvent(event, body => {
|
||||
const typedBody = body as TBody;
|
||||
if (predicate && !predicate(typedBody)) {
|
||||
@@ -406,6 +420,7 @@ export class DapClient {
|
||||
cleanup();
|
||||
resolve(typedBody);
|
||||
});
|
||||
this.#eventWaiterRejectors.add(closeHandler);
|
||||
if (signal) {
|
||||
signal.addEventListener("abort", abortHandler, { once: true });
|
||||
}
|
||||
@@ -580,6 +595,7 @@ export class DapClient {
|
||||
|
||||
const framer = new MessageFramer(this.#messageBuffer);
|
||||
|
||||
let closeError: Error | undefined;
|
||||
try {
|
||||
while (true) {
|
||||
const { done, value } = await reader.read();
|
||||
@@ -619,13 +635,19 @@ export class DapClient {
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
this.#rejectPendingRequests(new Error(`DAP connection closed: ${toErrorMessage(error)}`));
|
||||
closeError = new Error(`DAP connection closed: ${toErrorMessage(error)}`);
|
||||
} finally {
|
||||
// Persist any unparsed remainder so a restarted reader resumes mid-message.
|
||||
this.#messageBuffer = framer.remainder();
|
||||
reader.releaseLock();
|
||||
this.#isReading = false;
|
||||
}
|
||||
// The transport is gone once the reader loop exits — on a thrown error
|
||||
// or a clean stream end (a socket the peer dropped after we wrote, e.g.
|
||||
// the WSL2-mirrored ghost-accept race in issue #6055). Fail every
|
||||
// in-flight request and event waiter so callers see an immediate error
|
||||
// instead of waiting out their own timeout.
|
||||
this.#failConnection(closeError ?? new Error(`DAP connection closed: ${this.adapter.name} transport ended`));
|
||||
}
|
||||
|
||||
#handleResponse(message: DapResponseMessage): void {
|
||||
@@ -712,7 +734,19 @@ export class DapClient {
|
||||
? `DAP adapter exited (code ${exitCode}): ${stderr}`
|
||||
: `DAP adapter exited unexpectedly (code ${exitCode})`,
|
||||
);
|
||||
this.#failConnection(error);
|
||||
}
|
||||
|
||||
/** Reject every in-flight request and wake every event waiter with `error`.
|
||||
* Called when the transport dies (reader end, socket close, adapter exit)
|
||||
* so nothing sits pending until its own timeout. */
|
||||
#failConnection(error: Error): void {
|
||||
this.#rejectPendingRequests(error);
|
||||
const waiters = Array.from(this.#eventWaiterRejectors);
|
||||
this.#eventWaiterRejectors.clear();
|
||||
for (const reject of waiters) {
|
||||
reject(error);
|
||||
}
|
||||
}
|
||||
|
||||
#rejectPendingRequests(error: Error): void {
|
||||
@@ -826,6 +860,49 @@ async function waitForTcpTransport(
|
||||
throw new Error(`TCP port ${host}:${port} was not ready after ${timeoutMs}ms`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Give the adapter a chance to announce it is listening on `port` before the
|
||||
* first connect. vscode-js-debug prints `Debug server listening at HOST:PORT`
|
||||
* to stdout from inside its `listen()` callback; waiting for the port to appear
|
||||
* there means we only connect once the child genuinely owns the reserved port,
|
||||
* which closes the WSL2-mirrored ghost-accept window (issue #6055) at its root.
|
||||
*
|
||||
* Best-effort: resolves on the banner, on process exit, or on timeout — the
|
||||
* subsequent connect loop and `proc.exitCode` checks surface real failures, so
|
||||
* an adapter that never prints a banner still proceeds (just without the gate).
|
||||
* Also drains stdout for the wait's duration: in tcp mode the DAP protocol
|
||||
* flows over the socket, so nothing else consumes the adapter's stdout.
|
||||
*/
|
||||
async function waitForTcpServerListening(
|
||||
proc: { stdout: ReadableStream<Uint8Array>; exitCode: number | null },
|
||||
port: number,
|
||||
timeoutMs: number,
|
||||
): Promise<void> {
|
||||
const ready = Promise.withResolvers<void>();
|
||||
const portText = String(port);
|
||||
void (async () => {
|
||||
try {
|
||||
const decoder = new TextDecoder();
|
||||
let buffered = "";
|
||||
for await (const chunk of proc.stdout) {
|
||||
buffered += decoder.decode(chunk, { stream: true });
|
||||
if (buffered.includes(portText)) {
|
||||
ready.resolve();
|
||||
}
|
||||
// Keep only the tail relevant for banner matching so a chatty
|
||||
// adapter cannot grow this buffer without bound.
|
||||
if (buffered.length > 4096) {
|
||||
buffered = buffered.slice(-1024);
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
/* stdout errored — the connect loop surfaces the real failure */
|
||||
}
|
||||
ready.resolve();
|
||||
})();
|
||||
await Promise.race([ready.promise, Bun.sleep(timeoutMs)]);
|
||||
}
|
||||
|
||||
interface SocketTransport {
|
||||
readable: ReadableStream<Uint8Array>;
|
||||
writeSink: DapWriteSink;
|
||||
|
||||
@@ -487,6 +487,112 @@ describe("DAP launch failure handling", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("DAP TCP transport resilience", () => {
|
||||
const TCP_ADAPTER_BASE: DapResolvedAdapter = {
|
||||
...TEST_ADAPTER,
|
||||
name: "js-debug-adapter",
|
||||
command: process.execPath,
|
||||
resolvedCommand: process.execPath,
|
||||
connectMode: "tcp",
|
||||
};
|
||||
|
||||
// Adapter that binds the reserved port, accepts the first connection, then
|
||||
// drops it after 30ms without answering — the WSL2-mirrored ghost socket.
|
||||
const GHOST_ADAPTER = `
|
||||
const port = Number(process.argv[2]);
|
||||
const server = Bun.listen({ hostname: "127.0.0.1", port, socket: {
|
||||
open(s){ setTimeout(() => { try { s.end(); } catch {} }, 30); },
|
||||
data(){}, close(){}, error(){},
|
||||
}});
|
||||
console.log("Debug server listening at 127.0.0.1:" + port);
|
||||
await Bun.sleep(60_000);
|
||||
`;
|
||||
|
||||
// Adapter that binds only after a delay, prints the listening banner from
|
||||
// inside its listen callback, then answers initialize over the socket.
|
||||
const DELAYED_BANNER_ADAPTER = `
|
||||
const port = Number(process.argv[2]);
|
||||
await Bun.sleep(150);
|
||||
const server = Bun.listen({ hostname: "127.0.0.1", port, socket: {
|
||||
open(){},
|
||||
data(s, data){
|
||||
const text = Buffer.from(data).toString();
|
||||
const m = /Content-Length: (\\d+)\\r\\n\\r\\n([\\s\\S]*)/.exec(text);
|
||||
if (!m) return;
|
||||
const req = JSON.parse(m[2].slice(0, Number(m[1])));
|
||||
const resp = JSON.stringify({ seq: 1, type: "response", request_seq: req.seq, success: true, command: req.command, body: { supportsConfigurationDoneRequest: true } });
|
||||
s.write(\`Content-Length: \${Buffer.byteLength(resp)}\\r\\n\\r\\n\${resp}\`);
|
||||
},
|
||||
close(){}, error(){},
|
||||
}});
|
||||
console.log("Debug server listening at 127.0.0.1:" + port);
|
||||
await Bun.sleep(60_000);
|
||||
`;
|
||||
|
||||
async function withTcpAdapter(
|
||||
source: string,
|
||||
run: (adapter: DapResolvedAdapter, cwd: string) => Promise<void>,
|
||||
): Promise<void> {
|
||||
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "omp-debug-tcp-"));
|
||||
const adapterPath = path.join(cwd, "tcp-adapter.mjs");
|
||||
await fs.writeFile(adapterPath, source);
|
||||
const adapter: DapResolvedAdapter = {
|
||||
...TCP_ADAPTER_BASE,
|
||||
args: [adapterPath, "${port}", "127.0.0.1"],
|
||||
};
|
||||
try {
|
||||
await run(adapter, cwd);
|
||||
} finally {
|
||||
await removeWithRetries(cwd);
|
||||
}
|
||||
}
|
||||
|
||||
it("rejects a pending request fast when the transport closes cleanly without answering", async () => {
|
||||
await withTcpAdapter(GHOST_ADAPTER, async (adapter, cwd) => {
|
||||
const client = await DapClient.spawn({ adapter, cwd, socketReadyTimeoutMs: 5_000 });
|
||||
try {
|
||||
const start = Date.now();
|
||||
// The ghost socket ends the read stream cleanly. Before the fix the
|
||||
// request sat pending until its own 5s timeout; now the reader wakes
|
||||
// it with a descriptive connection-closed error well before then.
|
||||
await expect(client.sendRequest("initialize", {}, undefined, 5_000)).rejects.toThrow(
|
||||
/DAP connection closed/,
|
||||
);
|
||||
expect(Date.now() - start).toBeLessThan(2_000);
|
||||
} finally {
|
||||
await client.dispose();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("wakes an event waiter when the transport closes instead of waiting out its timeout", async () => {
|
||||
await withTcpAdapter(GHOST_ADAPTER, async (adapter, cwd) => {
|
||||
const client = await DapClient.spawn({ adapter, cwd, socketReadyTimeoutMs: 5_000 });
|
||||
try {
|
||||
const start = Date.now();
|
||||
await expect(client.waitForEvent("stopped", undefined, undefined, 5_000)).rejects.toThrow(
|
||||
/DAP connection closed/,
|
||||
);
|
||||
expect(Date.now() - start).toBeLessThan(2_000);
|
||||
} finally {
|
||||
await client.dispose();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("defers the first connect until the adapter announces its listening port", async () => {
|
||||
await withTcpAdapter(DELAYED_BANNER_ADAPTER, async (adapter, cwd) => {
|
||||
const client = await DapClient.spawn({ adapter, cwd, socketReadyTimeoutMs: 5_000 });
|
||||
try {
|
||||
const caps = await client.initialize({ clientID: "omp", adapterID: "js-debug-adapter" }, undefined, 5_000);
|
||||
expect(caps).toMatchObject({ supportsConfigurationDoneRequest: true });
|
||||
} finally {
|
||||
await client.dispose();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("DebugTool launch validation", () => {
|
||||
it("rejects directory programs when the selected adapter cannot debug a directory", async () => {
|
||||
const launchSpy = spyOn(dapModule, "selectLaunchAdapter").mockReturnValue({
|
||||
|
||||
Reference in New Issue
Block a user