fix(android): bind JADX admission release only to descriptors its record owns (#11131)

src/android/JadxProvider.ts

@@ -135,13 +135,21 @@ interface PendingCleanup {
   session: JadxSession | undefined;
   root: JadxWorkspaceRoot | undefined;
   readonly fileHandles: Map<OwnedFileHandle, string>;
-  /** 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<OwnedFileHandle>;
+  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<OwnedFileHandle, string>,
+): 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;

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<OwnedFileHandle>();
+  const handles = new Set<OwnedFileHandle>();
+  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);
+  }
+});