diff --git a/src/gdb/GdbSessionManager.ts b/src/gdb/GdbSessionManager.ts index 49e4207751..5695044ba2 100644 --- a/src/gdb/GdbSessionManager.ts +++ b/src/gdb/GdbSessionManager.ts @@ -464,22 +464,6 @@ export class GdbSessionManager { }, ]), ); - if (this.#closing || signal?.aborted) - return err( - signal?.aborted - ? cancelledWithCallerReason("start_gdb_session", signal) - : cancelledBy("start_gdb_session", OwnerCancellation.ownerClosed), - ); - if (!persistentDebuggerHostSupported(this.#platform)) - return err( - new AnalysisCapabilityUnavailableError( - "gdb", - "start_gdb_session", - this.#platform === "win32" - ? "Persistent Windows GDB sessions are unavailable because owned Job Object cleanup can terminate debugger inferiors." - : "Persistent GDB sessions are available only on Linux because REA verifies live inferior output descriptors through /proc.", - ), - ); return this.#sessions.start("start_gdb_session", signal, (started) => this.#startWithReservation(started, options), ); @@ -495,6 +479,16 @@ export class GdbSessionManager { > > { if (signal?.aborted) return err(cancelledBy("start_gdb_session", signal)); + if (!persistentDebuggerHostSupported(this.#platform)) + return err( + new AnalysisCapabilityUnavailableError( + "gdb", + "start_gdb_session", + this.#platform === "win32" + ? "Persistent Windows GDB sessions are unavailable because owned Job Object cleanup can terminate debugger inferiors." + : "Persistent GDB sessions are available only on Linux because REA verifies live inferior output descriptors through /proc.", + ), + ); const occupied = this.#sessions.occupied(); if (occupied >= this.#maxSessions.value) return err( @@ -1360,7 +1354,7 @@ export class GdbSessionManager { } async #drainStartsAndCloseSessions(): Promise { - await this.#sessions.abortStarts(); + await this.#sessions.closeStarts(); await Promise.all( [...this.#sessions.values()].map((session) => session.commandTail), ); diff --git a/src/process/OwnedDebuggerSessionRegistry.ts b/src/process/OwnedDebuggerSessionRegistry.ts index 5907b00f9e..98e770a2e9 100644 --- a/src/process/OwnedDebuggerSessionRegistry.ts +++ b/src/process/OwnedDebuggerSessionRegistry.ts @@ -65,6 +65,8 @@ export class OwnedDebuggerSessionRegistry< #reservations = 0; /** In-flight starts; closing aborts each as owner-closed and awaits it. */ readonly #starts = new Map>(); + /** Set once closing begins; later starts are refused as owner-closed. */ + #closed = false; constructor(options: OwnedDebuggerSessionRegistryOptions) { this.#options = options; @@ -110,15 +112,25 @@ export class OwnedDebuggerSessionRegistry< } /** - * Run one session start that closing aborts as owner-closed. The caller's - * abort is relayed; when it came first, a cancellation reports the - * caller's reason. Register before awaiting, so a close cannot miss it. + * Run one session start that closing aborts as owner-closed. A start made + * after closing began is refused the same way, and a caller that already + * aborted gets its own reason. The caller's abort is relayed; when it came + * first, a cancellation reports the caller's reason. Register before + * awaiting, so a close cannot miss it. */ start( operation: string, callerSignal: AbortSignal | undefined, run: (signal: AbortSignal) => Promise>, ): Promise> { + if (callerSignal?.aborted) + return Promise.resolve( + err(cancelledWithCallerReason(operation, callerSignal)), + ); + if (this.#closed) + return Promise.resolve( + err(cancelledBy(operation, OwnerCancellation.ownerClosed)), + ); const controller = new AbortController(); let callerAbortWon = false; const relayAbort = (): void => { @@ -127,7 +139,6 @@ export class OwnedDebuggerSessionRegistry< controller.abort(callerSignal?.reason); }; callerSignal?.addEventListener("abort", relayAbort, { once: true }); - if (callerSignal?.aborted) relayAbort(); const started = Promise.resolve() .then(() => run(controller.signal)) .finally(() => { @@ -145,8 +156,12 @@ export class OwnedDebuggerSessionRegistry< ); } - /** Abort every in-flight start as owner-closed and wait for each to settle. */ - async abortStarts(): Promise { + /** + * Refuse new starts, abort every in-flight start as owner-closed and wait + * for each to settle. + */ + async closeStarts(): Promise { + this.#closed = true; const active = [...this.#starts.entries()]; for (const [controller] of active) controller.abort(OwnerCancellation.ownerClosed); @@ -241,7 +256,7 @@ export class OwnedDebuggerSessionRegistry< /** * Stop every session; sessions whose stop is incomplete stay registered for - * retry. Callers block new starts and abort in-flight ones first. + * retry. Callers close starts first. */ closeAll(): Promise { return closeOwnedDebuggerSessions( diff --git a/src/rizin/RizinDebugSessionManager.ts b/src/rizin/RizinDebugSessionManager.ts index e6fc9bbd4a..364194f875 100644 --- a/src/rizin/RizinDebugSessionManager.ts +++ b/src/rizin/RizinDebugSessionManager.ts @@ -371,17 +371,28 @@ export class RizinDebugSessionManager { AnalysisError > > { - if (this.#closingAll) - return err( - new AnalysisInputError("start_rizin_debug_session", undefined, [ - { - path: ["inferior_tty"], - reason: "missing_argument", - message: - "Select a persistent readable and writable Linux terminal for inferior input and output.", - }, - ]), - ); + return this.#sessions.start( + "start_rizin_debug_session", + signal, + (started) => this.#startWithReservation(input, started), + ); + } + + async #startWithReservation( + input: { + readonly path: string; + readonly inferior_tty: string; + readonly backend?: string; + }, + signal: AbortSignal, + ): Promise< + Result< + { readonly session_id: string; readonly backend: string | null }, + AnalysisError + > + > { + if (signal.aborted) + return err(cancelledBy("start_rizin_debug_session", signal)); const occupied = this.#sessions.occupied(); if (occupied >= this.#maxSessions.value) return err( @@ -398,11 +409,11 @@ export class RizinDebugSessionManager { ), ); const releaseReservation = this.#sessions.reserve(); - return this.#sessions - .start("start_rizin_debug_session", signal, (started) => - this.#start(input, started, releaseReservation), - ) - .finally(releaseReservation); + try { + return await this.#start(input, signal, releaseReservation); + } finally { + releaseReservation(); + } } async #start( @@ -411,7 +422,7 @@ export class RizinDebugSessionManager { readonly inferior_tty: string; readonly backend?: string; }, - signal: AbortSignal | undefined, + signal: AbortSignal, releaseReservation: () => void, ): Promise< Result< @@ -419,8 +430,6 @@ export class RizinDebugSessionManager { AnalysisError > > { - if (signal?.aborted) - return err(cancelledBy("start_rizin_debug_session", signal)); if (!persistentDebuggerHostSupported(this.#platform)) return err( new AnalysisCapabilityUnavailableError( @@ -1184,7 +1193,7 @@ export class RizinDebugSessionManager { async closeAll(): Promise { this.#closingAll = true; - await this.#sessions.abortStarts(); + await this.#sessions.closeStarts(); const outcomes = await Promise.allSettled([ this.#sessions.closeAll(), this.#ttyResources.close(), diff --git a/tests/boundary/providers/RizinDebugSessionReviewRegressions.test.ts b/tests/boundary/providers/RizinDebugSessionReviewRegressions.test.ts index 72ecd1a6f6..cc663d6dca 100644 --- a/tests/boundary/providers/RizinDebugSessionReviewRegressions.test.ts +++ b/tests/boundary/providers/RizinDebugSessionReviewRegressions.test.ts @@ -184,6 +184,32 @@ it.skipIf(process.platform === "win32")( }, ); +it("refuses a Rizin start made after closeAll began as owner-closed, keeping a caller's own abort reason", async () => { + const manager = new RizinDebugSessionManager(process.env, { + platform: "linux", + spawnProcess: () => { + throw new Error("a refused start must not spawn Rizin"); + }, + }); + managers.push(manager); + await manager.closeAll(); + const input = { path: "/tmp/fixture.bin", inferior_tty: "/dev/null" }; + await expect(manager.start(input)).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + cause: OwnerCancellation.ownerClosed, + }, + }); + const caller = new AbortController(); + const reason = new Error("caller stopped waiting"); + caller.abort(reason); + await expect(manager.start(input, caller.signal)).resolves.toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError", cause: reason }, + }); +}); + it("keeps persistent Rizin sessions unavailable on Windows", async () => { const rizin = await createRizinTestManager( { REA_RIZIN_COMMAND: "/missing/rizin" },