fix(dap): reject and bound the unix socket connect on Linux
connectSocket captured only resolve from Promise.withResolvers and its error handler merely errored the stream, so a Bun.connect failure (ECONNREFUSED/ENOENT/EACCES on a stat-ready-but-dead socket) never settled the promise. The unbounded await in #spawnSocketUnix then hung the launch forever, and the surrounding kill/reap guard could not fire. Capture reject, track an opened flag, reject on error/close-before-open and on the Bun.connect promise rejection, and bound the connect with a timeout matching the launch deadline (cleared on settle). Thread timeoutMs from #spawnSocketUnix. Export connectSocket for a deterministic reject-path test. Fixes #4087
This commit is contained in:
@@ -260,7 +260,7 @@ export class DapClient {
|
||||
// socket connect fails, we must not leak the detached adapter process.
|
||||
try {
|
||||
await waitForCondition(() => isUnixSocketReady(socketPath), timeoutMs, proc);
|
||||
const { readable, writeSink, socket } = await connectSocket({ unix: socketPath });
|
||||
const { readable, writeSink, socket } = await connectSocket({ unix: socketPath }, timeoutMs);
|
||||
const client = new DapClient(adapter, cwd, proc, { readable, writeSink, socket });
|
||||
proc.exited.then(() => client.#handleProcessExit());
|
||||
void client.#startMessageReader();
|
||||
@@ -924,10 +924,20 @@ function socketToSink(socket: Bun.Socket<undefined>): DapWriteSink {
|
||||
};
|
||||
}
|
||||
|
||||
/** Connect to a unix domain socket and return DAP transport streams. */
|
||||
async function connectSocket(options: { unix: string }): Promise<SocketTransport> {
|
||||
const { promise, resolve } = Promise.withResolvers<SocketTransport>();
|
||||
/**
|
||||
* Connect to a unix domain socket and return DAP transport streams.
|
||||
*
|
||||
* Rejects (rather than hanging) when the connect fails — a stat-ready but dead
|
||||
* socket returns ECONNREFUSED, a socket removed between the readiness stat and
|
||||
* the connect returns ENOENT, a permission mismatch returns EACCES — and when
|
||||
* neither `open` nor an error arrives within `timeoutMs` (e.g. a TOCTOU stall).
|
||||
* `#spawnSocketUnix`'s catch then kills the detached adapter instead of leaking
|
||||
* it. Exported so tests can drive the reject path deterministically.
|
||||
*/
|
||||
export async function connectSocket(options: { unix: string }, timeoutMs: number): Promise<SocketTransport> {
|
||||
const { promise, resolve, reject } = Promise.withResolvers<SocketTransport>();
|
||||
let streamController: ReadableStreamDefaultController<Uint8Array>;
|
||||
let opened = false;
|
||||
|
||||
const readable = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
@@ -935,10 +945,21 @@ async function connectSocket(options: { unix: string }): Promise<SocketTransport
|
||||
},
|
||||
});
|
||||
|
||||
const timer = setTimeout(() => {
|
||||
reject(new Error(`Timed out connecting to unix socket ${options.unix} after ${timeoutMs}ms`));
|
||||
}, timeoutMs);
|
||||
// A late socket callback after settle is a no-op; clearing the timer just
|
||||
// stops it from keeping the event loop alive past the connect.
|
||||
void promise.then(
|
||||
() => clearTimeout(timer),
|
||||
() => clearTimeout(timer),
|
||||
);
|
||||
|
||||
Bun.connect({
|
||||
unix: options.unix,
|
||||
socket: {
|
||||
open(socket) {
|
||||
opened = true;
|
||||
resolve({
|
||||
readable,
|
||||
writeSink: socketToSink(socket),
|
||||
@@ -949,6 +970,9 @@ async function connectSocket(options: { unix: string }): Promise<SocketTransport
|
||||
streamController.enqueue(new Uint8Array(data));
|
||||
},
|
||||
close() {
|
||||
if (!opened) {
|
||||
reject(new Error(`Unix socket ${options.unix} closed before opening`));
|
||||
}
|
||||
try {
|
||||
streamController.close();
|
||||
} catch {
|
||||
@@ -956,6 +980,9 @@ async function connectSocket(options: { unix: string }): Promise<SocketTransport
|
||||
}
|
||||
},
|
||||
error(_socket, err) {
|
||||
if (!opened) {
|
||||
reject(err);
|
||||
}
|
||||
try {
|
||||
streamController.error(err);
|
||||
} catch {
|
||||
@@ -963,6 +990,12 @@ async function connectSocket(options: { unix: string }): Promise<SocketTransport
|
||||
}
|
||||
},
|
||||
},
|
||||
}).catch(err => {
|
||||
// Bun.connect rejects the returned promise on synchronous connect
|
||||
// failures (e.g. ENOENT) without always firing the `error` handler.
|
||||
if (!opened) {
|
||||
reject(err);
|
||||
}
|
||||
});
|
||||
|
||||
return promise;
|
||||
|
||||
@@ -4,7 +4,7 @@ import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import * as dapModule from "@oh-my-pi/pi-coding-agent/dap";
|
||||
import { DapClient, waitForTcpServerListening } from "@oh-my-pi/pi-coding-agent/dap/client";
|
||||
import { connectSocket, DapClient, waitForTcpServerListening } from "@oh-my-pi/pi-coding-agent/dap/client";
|
||||
import { DapSessionManager } from "@oh-my-pi/pi-coding-agent/dap/session";
|
||||
import type {
|
||||
DapCapabilities,
|
||||
@@ -487,6 +487,20 @@ describe("DAP launch failure handling", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("connectSocket unix transport", () => {
|
||||
it("rejects instead of hanging when the unix socket cannot be connected", async () => {
|
||||
// A path that stat would report as a socket but that no one listens on
|
||||
// yields ECONNREFUSED/ENOENT from Bun.connect. Before the fix the error
|
||||
// handler only errored the stream and the returned promise never settled,
|
||||
// so `await connectSocket(...)` hung the launch forever.
|
||||
const deadSocket = path.join(os.tmpdir(), `omp-dap-dead-${Date.now()}-${Math.random().toString(36).slice(2)}.sock`);
|
||||
const start = Date.now();
|
||||
await expect(connectSocket({ unix: deadSocket }, 5_000)).rejects.toThrow();
|
||||
// Must settle on the connect error, not linger until the timeout bound.
|
||||
expect(Date.now() - start).toBeLessThan(2_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("DAP TCP transport resilience", () => {
|
||||
const TCP_ADAPTER_BASE: DapResolvedAdapter = {
|
||||
...TEST_ADAPTER,
|
||||
|
||||
Reference in New Issue
Block a user