diff --git a/scripts/verify/architecture-guard-baselines.json b/scripts/verify/architecture-guard-baselines.json index 4ae34e22a1..5c52c8650e 100644 --- a/scripts/verify/architecture-guard-baselines.json +++ b/scripts/verify/architecture-guard-baselines.json @@ -130,11 +130,6 @@ "src/firmware/FirmwareProvider.ts": 1, "src/gdb/GdbSessionManager.ts": 14, "src/gpu/GpuCodeObjectsProvider.ts": 1, - "src/hopper/HopperAnalysisProfile.ts": 1, - "src/hopper/HopperCallerReconciliation.ts": 2, - "src/hopper/HopperFileOffset.ts": 1, - "src/hopper/HopperMachOImage.ts": 10, - "src/hopper/HopperRegexSearch.ts": 3, "src/inspector/JavaScriptRuntimeScope.ts": 1, "src/inspector/V8InspectorProvider.ts": 2, "src/javascript/analysis/JavaScriptAnalysisWorker.ts": 5, diff --git a/src/hopper/HopperAnalysisProfile.ts b/src/hopper/HopperAnalysisProfile.ts index 7d3247685d..509d6e0737 100644 --- a/src/hopper/HopperAnalysisProfile.ts +++ b/src/hopper/HopperAnalysisProfile.ts @@ -31,8 +31,9 @@ import { type HeaderArchitecture, } from "../domain/binaryTargetTypes.js"; import { - AnalysisCancelledError, AnalysisCapabilityUnavailableError, + cancelledBy, + carriedReason, } from "../domain/analysisErrorCore.js"; import { BinaryTargetError } from "../domain/configurationErrors.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; @@ -391,8 +392,7 @@ const readHopperProfileFile = async ( const diagnostics = { profile_file: { kind, path }, ...completed }; const failure = cancellation !== undefined - ? new AnalysisCancelledError("open_binary", { - cause: cancellation.reason, + ? cancelledBy("open_binary", carriedReason(cancellation.reason), { cleanup, diagnostics, }) diff --git a/src/hopper/HopperCallerReconciliation.ts b/src/hopper/HopperCallerReconciliation.ts index a0c0aaa589..f1eb500b9c 100644 --- a/src/hopper/HopperCallerReconciliation.ts +++ b/src/hopper/HopperCallerReconciliation.ts @@ -5,6 +5,8 @@ import { AnalysisCancelledError, AnalysisOutputError, AnalysisResourceConstraintError, + cancelledBy, + carriedReason, } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { JsonValue } from "../domain/jsonValue.js"; @@ -129,8 +131,9 @@ async function buildIndex( bytes = 0; const began = performance.now(); let stop: HopperCallerScanStop = { code: "complete" }; + // Called only after the caller's signal aborted. const cancelled = (): AnalysisCancelledError => - new AnalysisCancelledError(operation, { cause: options?.signal?.reason }); + cancelledBy(operation, carriedReason(options?.signal?.reason)); const fail = ( error: AnalysisError, failureStop: HopperCallerScanStop, @@ -428,12 +431,10 @@ export const reconcileHopperCallers = async ( ); if (options?.signal?.aborted) return err( - retainNative( - new AnalysisCancelledError(operation, { - cause: options.signal.reason, - }), - { scan: null, destinations: null }, - ), + retainNative(cancelledBy(operation, options.signal), { + scan: null, + destinations: null, + }), ); const selected = base !== undefined diff --git a/src/hopper/HopperFileOffset.ts b/src/hopper/HopperFileOffset.ts index 0b1014da8f..e915ac7736 100644 --- a/src/hopper/HopperFileOffset.ts +++ b/src/hopper/HopperFileOffset.ts @@ -9,6 +9,9 @@ import { AnalysisArtifactChangedError, AnalysisCancelledError, AnalysisCapabilityUnavailableError, + cancelledBy, + carriedReason, + type CancellationReason, } from "../domain/analysisErrorCore.js"; import { BinaryTargetError } from "../domain/configurationErrors.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; @@ -71,7 +74,7 @@ export async function mapHopperFileOffset( "The selected database digest does not identify its original executable bytes. Open that executable to verify original-file coordinates; read_bytes still exposes the database's native bytes.", ); const signal = ownership.signal; - if (signal?.aborted) return err(cancelled(partial, signal.reason)); + if (signal?.aborted) return err(cancelled(partial, signal)); const publish = (marker: ProviderCleanupError): ProviderCleanupError => { ownership.onCleanupMarker(marker); return marker; @@ -117,7 +120,7 @@ export async function mapHopperFileOffset( ); return err( retainSnapshot( - cancelled(partial, cause.reason), + cancelled(partial, carriedReason(cause.reason)), captured.value, verification.observation, verified.fileOffset, @@ -161,10 +164,9 @@ const isSourceRead = ( const cancelled = ( partial: (reason: string) => FileOffsetPartialObservation, - cause?: unknown, + reason: CancellationReason, ): AnalysisCancelledError => - new AnalysisCancelledError(OPERATION, { - ...(cause === undefined ? {} : { cause }), + cancelledBy(OPERATION, reason, { partialObservation: partial("Source verification was cancelled"), }); @@ -184,7 +186,7 @@ const sourceAccessFailure = ( partial: (reason: string) => FileOffsetPartialObservation, ): AnalysisError => { if (cause instanceof RegularFileOperationCancelled) - return cancelled(partial, cause.reason); + return cancelled(partial, carriedReason(cause.reason)); const reason = `Cannot verify original source bytes: ${cause instanceof Error ? cause.message : String(cause)}`; const code = typeof cause === "object" && cause !== null && "code" in cause @@ -266,7 +268,10 @@ const cleanupAfterSnapshot = ( return err( withCleanup( retainSnapshot( - cancelled(partial, failure.completionCancellation.reason), + cancelled( + partial, + carriedReason(failure.completionCancellation.reason), + ), captured.value, verification.observation, verified.fileOffset, diff --git a/src/hopper/HopperMachOImage.ts b/src/hopper/HopperMachOImage.ts index ef9d79855a..b1513eedd6 100644 --- a/src/hopper/HopperMachOImage.ts +++ b/src/hopper/HopperMachOImage.ts @@ -11,7 +11,11 @@ import type { RetainedResourceScope } from "../application/AnalysisProvider.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { BinaryTargetError } from "../domain/configurationErrors.js"; -import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + cancelledBy, + carriedReason, +} from "../domain/analysisErrorCore.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { findOverlappingFatSlices, @@ -95,11 +99,7 @@ export const resolveHopperMachOImage = async ( ), ); if (options.signal?.aborted) - return err( - new AnalysisCancelledError("open_binary", { - cause: options.signal.reason, - }), - ); + return err(cancelledBy("open_binary", options.signal)); if (target.format !== "mach-o" && target.format !== "mach-o-universal") return invalid("Hopper image preparation requires a Mach-O target"); let opened: FileHandle; @@ -158,11 +158,7 @@ const inspectAndFinalizeHopperMachOImage = async ( } if (closeOutcome.kind === "closed") { if (result.ok && options.signal?.aborted) - return err( - new AnalysisCancelledError("open_binary", { - cause: options.signal.reason, - }), - ); + return err(cancelledBy("open_binary", options.signal)); return result; } return retainHopperPreflightCloseFailure({ @@ -225,15 +221,20 @@ const retainHopperPreflightCloseFailure = (options: { cleanup, }, ) - : new AnalysisCancelledError("open_binary", { - cause: options.result.error.cause, - cleanup, - }); + : cancelledBy( + "open_binary", + carriedReason(options.result.error.cause), + { cleanup }, + ); } else if (options.cancellationBeforeClose) { - failure = new AnalysisCancelledError("open_binary", { - cause: options.signal?.reason, - cleanup, - }); + // cancellationBeforeClose records that the caller's signal had aborted. + failure = cancelledBy( + "open_binary", + carriedReason(options.signal?.reason), + { + cleanup, + }, + ); } else { failure = new BinaryTargetError( options.target.path, @@ -256,11 +257,7 @@ const retainHopperPreflightCloseFailure = (options: { failure.retainSecondaryFailure(cleanupError); } if (options.signal?.aborted && !cancellationIsPrimary) - failure.retainSecondaryFailure( - new AnalysisCancelledError("open_binary", { - cause: options.signal.reason, - }), - ); + failure.retainSecondaryFailure(cancelledBy("open_binary", options.signal)); new AnalysisCleanupLeaseGroup(failure, [lease]); return err(failure); }; @@ -280,10 +277,7 @@ const inspectHopperMachOImage = async ( ); const read = (offset: number, length: number): Promise => readFileHandleRange(file, offset, length, () => { - if (signal?.aborted) - throw new AnalysisCancelledError("open_binary", { - cause: signal.reason, - }); + if (signal?.aborted) throw cancelledBy("open_binary", signal); }); const header = await read(0, 8); const fatPrefix = readFatHeaderPrefix(header); @@ -298,14 +292,9 @@ const inspectHopperMachOImage = async ( for await (const chunk of readFileHandleChunks(file, { ...(signal === undefined ? {} : { signal }), cancellationFailure: (reason) => - new AnalysisCancelledError("open_binary", { cause: reason }), + cancelledBy("open_binary", carriedReason(reason)), })) { - if (signal?.aborted) - return err( - new AnalysisCancelledError("open_binary", { - cause: signal.reason, - }), - ); + if (signal?.aborted) return err(cancelledBy("open_binary", signal)); const bytes = Buffer.from(chunk); hash.update(bytes); size += bytes.length; @@ -317,10 +306,7 @@ const inspectHopperMachOImage = async ( prefixLength += copied; } } - if (signal?.aborted) - return err( - new AnalysisCancelledError("open_binary", { cause: signal.reason }), - ); + if (signal?.aborted) return err(cancelledBy("open_binary", signal)); if (prefixLength !== prefix.length) return invalid("Thin Mach-O header is truncated"); const observedSha256 = hash.digest("hex"); @@ -372,10 +358,7 @@ const inspectHopperMachOImage = async ( const admitted = target.universalSlice; const matches: FatSliceDeclaration[] = []; for (const slice of parsed.slices) { - if (signal?.aborted) - return err( - new AnalysisCancelledError("open_binary", { cause: signal.reason }), - ); + if (signal?.aborted) return err(cancelledBy("open_binary", signal)); const rangeError = validateFatSlice(slice, size, tableEnd); if (rangeError !== null) return invalid(rangeError); if (slice.reserved !== null && slice.reserved !== 0) diff --git a/src/hopper/HopperRegexSearch.test.ts b/src/hopper/HopperRegexSearch.test.ts index 61674f8e3f..2dc4a23fc5 100644 --- a/src/hopper/HopperRegexSearch.test.ts +++ b/src/hopper/HopperRegexSearch.test.ts @@ -1,7 +1,10 @@ import { expect, it, vi } from "vitest"; import { HopperRegexSearch } from "./HopperRegexSearch.js"; import { err, ok } from "../domain/result.js"; -import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; +import { + AnalysisCapabilityUnavailableError, + OwnerCancellation, +} from "../domain/analysisErrorCore.js"; import { HopperRemoteError, HopperTimeoutError, @@ -247,6 +250,45 @@ it.each( }, ); +it.each([false, true])( + "joins an active search on close and preserves earlier caller cancellation: %s", + async (callerFirst) => { + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const controller = new AbortController(); + const search = new HopperRegexSearch({ + callTool: async () => { + await gate; + return ok([]); + }, + }); + const pending = search.execute( + "search_strings", + { pattern: "fixture", mode: "regex", case_sensitive: false }, + { signal: controller.signal }, + ); + if (callerFirst) controller.abort("caller stopped"); + let finished = false; + const closing = search.close().then(() => { + finished = true; + }); + await Promise.resolve(); + expect(finished).toBe(false); + release(); + expect(await pending).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + cause: callerFirst ? "caller stopped" : OwnerCancellation.ownerClosed, + }, + }); + await closing; + expect(finished).toBe(true); + }, +); + it.each( [null, "caller stopped", { requested: "stop" }].flatMap((reason) => (["inventory", "progress"] as const).map((stage) => ({ reason, stage })), diff --git a/src/hopper/HopperRegexSearch.ts b/src/hopper/HopperRegexSearch.ts index c830fd88a8..ed56f6d484 100644 --- a/src/hopper/HopperRegexSearch.ts +++ b/src/hopper/HopperRegexSearch.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import type { ExecutionOptions } from "../application/AnalysisProvider.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; -import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { cancelledBy, OwnerCancellation } from "../domain/analysisErrorCore.js"; import { HopperRemoteError } from "../domain/hopperErrors.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -61,7 +61,8 @@ export class HopperRegexSearch { /** Abort matching workers before their provider session closes. */ async close(): Promise { - for (const controller of this.#active.keys()) controller.abort(); + for (const controller of this.#active.keys()) + controller.abort(OwnerCancellation.ownerClosed); await Promise.all(this.#active.values()); } @@ -115,9 +116,7 @@ export class HopperRegexSearch { if (!parsed.ok) return parsed; const rows: readonly (typeof parsed.value)[number][] = parsed.value; if (options.signal?.aborted) - return err( - new AnalysisCancelledError(operation, { cause: options.signal.reason }), - ); + return err(cancelledBy(operation, options.signal)); await options.progress ?.report({ phase: "hopper_regex", @@ -127,9 +126,7 @@ export class HopperRegexSearch { }) .catch(() => undefined); if (options.signal?.aborted) - return err( - new AnalysisCancelledError(operation, { cause: options.signal.reason }), - ); + return err(cancelledBy(operation, options.signal)); const matched = await matchRegexValues({ operation, pattern: input.data.pattern, @@ -141,9 +138,7 @@ export class HopperRegexSearch { }); if (!matched.ok) return matched; if (options.signal?.aborted) - return err( - new AnalysisCancelledError(operation, { cause: options.signal.reason }), - ); + return err(cancelledBy(operation, options.signal)); const selected = new Set(matched.value); return ok(rows.filter((_, index) => selected.has(index))); }