mirror of
https://github.com/morluto/rea.git
synced 2026-11-15 19:23:24 +00:00
fix(rizin): refuse a start after shutdown began as an owner-closed cancellation (#11041)
OwnedDebuggerSessionRegistry owns the rule: closeStarts refuses later starts as owner-closed (a caller that already aborted keeps its own reason) and aborts in-flight ones. Rizin no longer reports an inferior_tty input error for a start made after closeAll began; GDB's local copy of the rule is gone. User-visible: start_rizin_debug_session called while REA is shutting down now fails as a cancellation (owner closed) instead of an input error naming inferior_tty, matching start_gdb_session.
This commit is contained in:
1 parent
423489a761
commit
bcb068cc04
4 files changed
+88
-44
No files matched your search
@@ -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<void> {
|
||||
await this.#sessions.abortStarts();
|
||||
await this.#sessions.closeStarts();
|
||||
await Promise.all(
|
||||
[...this.#sessions.values()].map((session) => session.commandTail),
|
||||
);
|
||||
|
||||
@@ -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<AbortController, Promise<unknown>>();
|
||||
/** Set once closing begins; later starts are refused as owner-closed. */
|
||||
#closed = false;
|
||||
|
||||
constructor(options: OwnedDebuggerSessionRegistryOptions<Session>) {
|
||||
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<Value>(
|
||||
operation: string,
|
||||
callerSignal: AbortSignal | undefined,
|
||||
run: (signal: AbortSignal) => Promise<Result<Value, AnalysisError>>,
|
||||
): Promise<Result<Value, AnalysisError>> {
|
||||
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<void> {
|
||||
/**
|
||||
* Refuse new starts, abort every in-flight start as owner-closed and wait
|
||||
* for each to settle.
|
||||
*/
|
||||
async closeStarts(): Promise<void> {
|
||||
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<void> {
|
||||
return closeOwnedDebuggerSessions(
|
||||
|
||||
@@ -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<void> {
|
||||
this.#closingAll = true;
|
||||
await this.#sessions.abortStarts();
|
||||
await this.#sessions.closeStarts();
|
||||
const outcomes = await Promise.allSettled([
|
||||
this.#sessions.closeAll(),
|
||||
this.#ttyResources.close(),
|
||||
|
||||
@@ -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" },
|
||||
|
||||
Reference in new issue
Block a user