From bb445d8691944efcbfe012da5fea5591b99d20ae Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 21:31:11 +0800 Subject: [PATCH] fix(android): bind JADX admission release only to descriptors its record owns (#11131) --- src/android/JadxProvider.ts | 62 +++++++++++++------ .../android/jadxArchiveLifecycle.test.ts | 53 ++++++++++++++++ 2 files changed, 96 insertions(+), 19 deletions(-) diff --git a/src/android/JadxProvider.ts b/src/android/JadxProvider.ts index 5f948e4139..980eeefa35 100644 --- a/src/android/JadxProvider.ts +++ b/src/android/JadxProvider.ts @@ -135,13 +135,21 @@ interface PendingCleanup { session: JadxSession | undefined; root: JadxWorkspaceRoot | undefined; readonly fileHandles: Map; - /** Reports admission failure; bound to `descriptorRelease`. */ + /** Reports admission failure; bound to `admissionDescriptors.release` when present. */ readonly previousError: AnalysisError | undefined; - /** Confirmed once every descriptor owner from the admission close failure closes. */ - readonly descriptorRelease: AnalysisCleanupRelease; + /** + * Descriptors this record owns from the admission close failure, and the + * release confirmed once all of them close. Absent when the failure's + * descriptors belong to another owner, such as the prerequisite scope. + */ + readonly admissionDescriptors: AdmissionDescriptors | undefined; /** Confirmed once this whole retained cleanup completes; errors it reported read it. */ readonly release: AnalysisCleanupRelease; - readonly operationCause: unknown; +} + +interface AdmissionDescriptors { + readonly owners: ReadonlySet; + readonly release: AnalysisCleanupRelease; } /** Every descriptor owner nested in one rejected close, without treating a later owner as the first. */ @@ -160,6 +168,22 @@ const retainedCleanupOwners = ( return owners; }; +/** + * The descriptors in `fileHandles` that one rejected close left behind, with a + * release for them. Undefined when that close retained none of them. + */ +const admissionDescriptorsOf = ( + cause: unknown, + fileHandles: ReadonlyMap, +): AdmissionDescriptors | undefined => { + const owners = new Set( + [...retainedCleanupOwners(cause)].filter((owner) => fileHandles.has(owner)), + ); + return owners.size === 0 + ? undefined + : { owners, release: new AnalysisCleanupRelease() }; +}; + const cleanupResources = (resources: PendingCleanup): readonly string[] => [ ...(resources.session === undefined ? [] @@ -203,10 +227,7 @@ const cleanup = async ( sessionCloseFailed = true; sessionCloseCause = cause; } - const tracked = retainedCleanupOwners(resources.operationCause); - const trackedInMap = [...tracked].filter((owner) => - resources.fileHandles.has(owner), - ); + const admission = resources.admissionDescriptors; const fileCloseCauses: unknown[] = []; for (const owner of resources.fileHandles.keys()) { try { @@ -215,10 +236,12 @@ const cleanup = async ( // One descriptor's release clears only that owner. The admission error // stays incomplete until every owner from the same close failure is gone. if ( - trackedInMap.length > 0 && - trackedInMap.every((candidate) => !resources.fileHandles.has(candidate)) + admission !== undefined && + [...admission.owners].every( + (candidate) => !resources.fileHandles.has(candidate), + ) ) - resources.descriptorRelease.confirm(); + admission.release.confirm(); } catch (cause: unknown) { fileCloseCauses.push(cause); } @@ -447,9 +470,8 @@ export class JadxProvider implements AndroidAnalysisPort { root: this.#retained.root, fileHandles: new Map(), previousError: undefined, - descriptorRelease: new AnalysisCleanupRelease(), + admissionDescriptors: undefined, release: new AnalysisCleanupRelease(), - operationCause: undefined, }; this.#retained = undefined; } @@ -737,11 +759,14 @@ export class JadxProvider implements AndroidAnalysisPort { root ??= this.#retained?.root; session ??= this.#retained?.session; this.#retained = undefined; - // The admission failure reads its descriptors' release however it is - // first projected: by a cleanup failure or returned on its own. - const descriptorRelease = new AnalysisCleanupRelease(); + // The admission failure reads the release of the descriptors this record + // owns however it is first projected: by a cleanup failure or returned on + // its own. Descriptors another owner retained keep that owner's record. + const admissionDescriptors = admissionDescriptorsOf(cause, fileHandles); const admissionFailure = (): AnalysisError => - descriptorRelease.bind(failure()); + admissionDescriptors === undefined + ? failure() + : admissionDescriptors.release.bind(failure()); this.#pendingCleanup = { operation: request.operation, session, @@ -750,9 +775,8 @@ export class JadxProvider implements AndroidAnalysisPort { get previousError() { return admissionFailure(); }, - descriptorRelease, + admissionDescriptors, release: new AnalysisCleanupRelease(), - operationCause: cause, }; const retired = await this.#retire(); return retired.ok ? err(admissionFailure()) : retired; diff --git a/tests/boundary/android/jadxArchiveLifecycle.test.ts b/tests/boundary/android/jadxArchiveLifecycle.test.ts index 6974ab2d59..c04fbfcb2d 100644 --- a/tests/boundary/android/jadxArchiveLifecycle.test.ts +++ b/tests/boundary/android/jadxArchiveLifecycle.test.ts @@ -7,6 +7,7 @@ import { createJadxProtocolFixture } from "../../fixtures/android/jadx.js"; import { writeOrderedZip } from "../../fixtures/artifactEntryOrder.js"; import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; import { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; +import { parseBinaryTarget } from "../../../src/application/BinaryTargetResolver.js"; const it = test.skipIf(process.platform === "win32"); @@ -322,3 +323,55 @@ it("joins independent doctor archive finalization and retries its exact failed c if (owner !== undefined) await original.call(owner); } }); + +it("clears an analysis admission error once its prerequisite archive descriptor closes", async () => { + const f = await createJadxProtocolFixture(); + await writeFile(f.jar, "not a zip"); + const target = await parseBinaryTarget(f.apk); + if (!target.ok) throw target.error; + const identity = await stat(f.jar); + const original = OwnedFileHandle.prototype.close; + const seen = new Set(); + const handles = new Set(); + let allowClose = false; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(async function (this: OwnedFileHandle) { + if (!seen.has(this) && this.handle.fd >= 0) { + const observed = await this.handle.stat(); + if (observed.dev === identity.dev && observed.ino === identity.ino) { + // The first JAR descriptor is the provider's own source hash; the + // next is the archive inspection the prerequisite scope owns. + if (seen.size > 0) handles.add(this); + seen.add(this); + } + } + if (handles.has(this) && !allowClose) + throw new Error("injected archive descriptor close failure"); + await original.call(this); + }); + try { + // The JAR read and its descriptor close both fail. The prerequisite scope + // owns that descriptor; the analysis cleanup record owns none of it. + const result = await f.provider.execute(target.value, { + operation: "inspect_android_package", + input: { path: f.apk }, + }); + if (result.ok) throw new Error("Expected the archive admission failure"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupPrimary).toMatchObject({ + diagnostics: { failure_kind: "jar", failure_code: "version_unresolved" }, + }); + expect(handles.size).toBe(1); + allowClose = true; + await f.provider.close(); + expect([...handles].every((owner) => owner.handle.fd === -1)).toBe(true); + expect(result.error.cleanupIncomplete).toBe(false); + expect(f.launches).toEqual([]); + } finally { + allowClose = true; + close.mockRestore(); + await f.provider.close(); + for (const owner of handles) await original.call(owner); + } +});