diff --git a/scripts/verify/architecture-guard-baselines.json b/scripts/verify/architecture-guard-baselines.json index 6e5d9c2955..85a9ae47a3 100644 --- a/scripts/verify/architecture-guard-baselines.json +++ b/scripts/verify/architecture-guard-baselines.json @@ -330,9 +330,7 @@ "src/kuna/KunaProjectCache.ts": 1, "src/native/CommandRunner.ts": 1, "src/native/LldbCallTracer.ts": 1, - "src/process/OwnedDebuggerSessionRegistry.ts": 1, "src/providers/AnalysisClientFailure.ts": 1, - "src/server/environment/serveEnvironment.ts": 1, "src/sqlite/SqliteDatabaseProvider.ts": 2 }, "cleanup-dependency-binding": { diff --git a/src/gdb/GdbSessionManager.ts b/src/gdb/GdbSessionManager.ts index ddf9d49882..c54e9e78c7 100644 --- a/src/gdb/GdbSessionManager.ts +++ b/src/gdb/GdbSessionManager.ts @@ -455,7 +455,6 @@ export class GdbSessionManager { this.#sessions = new OwnedDebuggerSessionRegistry({ provider: "gdb", displayName: "GDB", - closeOperation: "close_gdb_session", sealSession: (session) => { session.cleanupStarted = true; }, diff --git a/src/process/OwnedDebuggerSessionRegistry.ts b/src/process/OwnedDebuggerSessionRegistry.ts index ba207f3219..a6df2c1844 100644 --- a/src/process/OwnedDebuggerSessionRegistry.ts +++ b/src/process/OwnedDebuggerSessionRegistry.ts @@ -3,6 +3,7 @@ import type { AnalysisErrorOptions, } from "../domain/analysisErrorBase.js"; import { err, ok, type Result } from "../domain/result.js"; +import { AnalysisCleanupRelease } from "../domain/analysisCleanupRelease.js"; import { closeOwnedDebuggerSessions, retainedDebuggerCleanupFailure, @@ -23,8 +24,6 @@ export interface OwnedDebuggerSessionRegistryOptions { readonly provider: string; /** Debugger name shown in cleanup failures, such as "GDB". */ readonly displayName: string; - /** Operation a retained failure names when no caller operation applies. */ - readonly closeOperation: string; /** Seal the session against new commands; runs on every stop request. */ readonly sealSession: (session: Session) => void; /** Stop the owned debugger process; protocol hooks stay with the adapter. */ @@ -45,7 +44,14 @@ export class OwnedDebuggerSessionRegistry< > { readonly #options: OwnedDebuggerSessionRegistryOptions; readonly #sessions = new Map(); - readonly #cleanupFailures = new WeakMap>(); + /** + * Each session's unconfirmed stop: the release its retained failures read, + * and the latest of them. + */ + readonly #cleanupFailures = new WeakMap< + Session, + { readonly release: AnalysisCleanupRelease; latest: AnalysisError } + >(); readonly #stopAttempts = new WeakMap< Session, Promise @@ -116,9 +122,8 @@ export class OwnedDebuggerSessionRegistry< })) .then((stopped) => { if (stopped.status !== "incomplete") { - const retained = this.#cleanupFailures.get(session); - for (const failure of retained ?? []) failure.resolveCleanup(); - retained?.clear(); + this.#cleanupFailures.get(session)?.release.confirm(); + this.#cleanupFailures.delete(session); } return stopped; }) @@ -132,13 +137,13 @@ export class OwnedDebuggerSessionRegistry< /** Keep a failure whose cleanup is incomplete until a stop is confirmed. */ retain(session: Session, failure: AnalysisError): AnalysisError { - let retained = this.#cleanupFailures.get(session); - if (retained === undefined) { - retained = new Set(); - this.#cleanupFailures.set(session, retained); - } - retained.add(failure); - return failure; + const retained = this.#cleanupFailures.get(session) ?? { + release: new AnalysisCleanupRelease(), + latest: failure, + }; + retained.latest = failure; + this.#cleanupFailures.set(session, retained); + return retained.release.bind(failure); } /** Retain the failure of an operation that could not confirm the stop. */ @@ -174,17 +179,9 @@ export class OwnedDebuggerSessionRegistry< ): Promise> { for (const session of this.#sessions.values()) { const retained = this.#cleanupFailures.get(session); - if (retained === undefined || retained.size === 0) continue; + if (retained === undefined) continue; const stopped = await this.stop(session); - if (stopped.status === "incomplete") - return err( - [...retained].at(-1) ?? - this.retainCleanupFailure( - session, - this.#options.closeOperation, - stopped.reason, - ), - ); + if (stopped.status === "incomplete") return err(retained.latest); if (!keepAddressable(session)) this.remove(session); } return ok(null); diff --git a/src/rizin/RizinDebugSessionManager.ts b/src/rizin/RizinDebugSessionManager.ts index 2621354205..dd630f1755 100644 --- a/src/rizin/RizinDebugSessionManager.ts +++ b/src/rizin/RizinDebugSessionManager.ts @@ -312,7 +312,6 @@ export class RizinDebugSessionManager { this.#sessions = new OwnedDebuggerSessionRegistry({ provider: "rizin", displayName: "Rizin", - closeOperation: "close_rizin_debug_session", sealSession: (session) => { session.closing = true; }, diff --git a/src/server/environment/serveEnvironment.ts b/src/server/environment/serveEnvironment.ts index 1332ddab81..dcaf960948 100644 --- a/src/server/environment/serveEnvironment.ts +++ b/src/server/environment/serveEnvironment.ts @@ -29,6 +29,7 @@ import { type EnvironmentToolDefinition, } from "../../application/environment/environmentTools.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; +import { AnalysisCleanupRelease } from "../../domain/analysisCleanupRelease.js"; import { safeParseJson } from "../../domain/safeJson.js"; import { AnalysisOutputError, @@ -519,7 +520,8 @@ export const serveEnvironment = async ( let responseClosePending = false; let settledSuccessful = false; let closing: Promise | undefined; - const shutdownFailures = new Set(); + // Every shutdown failure reports this release until a close succeeds. + let shutdownRelease: AnalysisCleanupRelease | undefined; const owner: EnvironmentShutdownOwner = { get cleanupResources() { const stateResources = stateFiles.cleanupResources; @@ -598,12 +600,13 @@ export const serveEnvironment = async ( "Keep this REA process available for cleanup. Confirm the reported resources are released before starting a replacement environment server.", }, ).bindCleanupResourceOwner(owner); - shutdownFailures.add(failure); - throw failure; + throw (shutdownRelease ??= new AnalysisCleanupRelease()).bind( + failure, + ); } retainedEnvironmentShutdowns.delete(owner); - for (const failure of shutdownFailures) failure.resolveCleanup(); - shutdownFailures.clear(); + shutdownRelease?.confirm(); + shutdownRelease = undefined; settledSuccessful = true; })(); closing = attempt; @@ -625,7 +628,7 @@ export const serveEnvironment = async ( closing = undefined; } } else if ( - shutdownFailures.size === 0 && + shutdownRelease === undefined && !httpClosePending && !stateClosePending ) { diff --git a/src/server/environment/serveEnvironmentState.test.ts b/src/server/environment/serveEnvironmentState.test.ts index 5a2fd433bd..8559a1efac 100644 --- a/src/server/environment/serveEnvironmentState.test.ts +++ b/src/server/environment/serveEnvironmentState.test.ts @@ -1229,7 +1229,8 @@ describe("serveEnvironment state file cleanup", () => { await expect( add({ kind: "data", data: renamedDocument() }), ).resolves.toMatchObject({ result: {} }); - await expect(environment.close()).rejects.toMatchObject({ + const reported = await environment.close().catch((cause: unknown) => cause); + expect(reported).toMatchObject({ _tag: "AnalysisOutputError", diagnostics: { cleanup_stages: ["state-files"] }, cause: expect.any(ArtifactResourceRetryFailure), @@ -1239,6 +1240,8 @@ describe("serveEnvironment state file cleanup", () => { failing = false; await expect(environment.close()).resolves.toBeUndefined(); expect(owners[0]?.handle.fd).toBe(-1); + // The earlier report reads the confirmed release instead of staying open. + expect(reported).toMatchObject({ cleanupIncomplete: false }); }); });