From 61fd6336bf31741a77f51e8ef84a596ce8d94d41 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 10:20:08 +0800 Subject: [PATCH] refactor(hopper): build Hopper cancellations with a required reason (#10601) * refactor(errors): require a reason for every analysis cancellation Add cancelledBy, the one constructor for AnalysisCancelledError. Its reason argument is required: an aborted signal, an earlier cancellation, a reason carried from an inner failure, or an OwnerCancellation when REA closed the work's owner itself. Closed adb, apktool, firmware, sqlite, gdb, web-bundle analyzer and binary-session record owners now report OwnerCancellation.ownerClosed instead of an absent reason. Firmware's predecessor wait returns its cancellation as a Result, removing an unreachable reason-less fallback. An architecture guard rejects new direct constructions, with a counted per-file baseline that can only shrink. * refactor(errors): share one aborted-signal predicate with cancelledBy * refactor(process): return a departing caller's reason from a shared startup wait * refactor(providers): build decompiler-provider cancellations with a required reason (#10587) Migrate every AnalysisCancelledError construction under src/ghidra, src/ida, src/cutter, src/jeb and src/kuna to cancelledBy and lower the guard baseline from 671 to 572. Closed Kuna target commands report OwnerCancellation.ownerClosed instead of an absent reason, and Kuna and JEB close abort their lifetimes with that typed reason. * fix(errors): read the reason of any signal-shaped cancellation source * test(ci): cover the counted-baseline comparison behind the architecture guard * refactor(hopper): build Hopper cancellations with a required reason Migrate the remaining AnalysisCancelledError constructions under src/hopper to cancelledBy. Regex-search close aborts its workers with OwnerCancellation.ownerClosed. HopperCleanup and HopperClient are not touched. * test(hopper): cover cancellation during search shutdown --- .../verify/architecture-guard-baselines.json | 5 -- src/hopper/HopperAnalysisProfile.ts | 6 +- src/hopper/HopperCallerReconciliation.ts | 15 ++-- src/hopper/HopperFileOffset.ts | 19 +++-- src/hopper/HopperMachOImage.ts | 69 +++++++------------ src/hopper/HopperRegexSearch.test.ts | 44 +++++++++++- src/hopper/HopperRegexSearch.ts | 17 ++--- 7 files changed, 98 insertions(+), 77 deletions(-) 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))); }