fix(android): retain a source descriptor when admission close fails

openRegularFile can reject after the descriptor opens. If that close also
fails, keep the owner on the snapshot cleanup map instead of dropping it
when adoption never runs.

Co-authored-by: morluto <morluto@users.noreply.github.com>
This commit is contained in:
Cursor Agentandmorluto committed 2026-10-22 18:13:21 +00:00
1 parent 0e1b07957a
commit 832be4b82e
2 files changed
+108 -3

No files matched your search

+41 -2
View File
@@ -16,6 +16,7 @@ import {
} from "../filesystem/OwnedRegularFile.js";
import {
openRegularFile,
RegularFileAdmissionFailure,
assertRegularFileActive,
sameRegularFileState,
} from "../filesystem/RegularFile.js";
@@ -173,7 +174,7 @@ const copyAndroidSnapshot = async (
// copy outcome and stays retained until that owner is retried; the
// partial file is removed only after those close attempts.
const source = adoptSnapshotHandle(
await openRegularFile(sourcePath, { symlinks: "follow", signal }),
await openSnapshotSource(sourcePath, signal, retain(sourcePath)),
);
return await withOwnedFileHandle(
sourcePath,
@@ -275,6 +276,44 @@ const copyAndroidSnapshot = async (
}
};
/**
* Admission can reject after the descriptor opens. Keep that owner when its
* own close also rejects; the success path still adopts one close owner.
*/
const openSnapshotSource = async (
path: string,
signal: AbortSignal | undefined,
retention: {
readonly retainFailedClose?: (owner: OwnedFileHandle) => void;
},
): Promise<FileHandle> => {
try {
return await openRegularFile(path, { symlinks: "follow", signal });
} catch (cause: unknown) {
if (!(cause instanceof RegularFileAdmissionFailure)) throw cause;
throw new RegularFileCleanupFailure(
path,
cause.owner,
{ kind: "failed", cause: cause.cause },
cause.cleanupCause,
retainAdmissionOwner(retention.retainFailedClose, cause.owner),
);
}
};
const retainAdmissionOwner = (
retain: ((owner: OwnedFileHandle) => void) | undefined,
owner: OwnedFileHandle,
): RegularFileCleanupFailure["retention"] => {
if (retain === undefined) return { kind: "not_requested" };
try {
retain(owner);
return { kind: "callback_returned" };
} catch (cause: unknown) {
return { kind: "callback_threw", cause };
}
};
/** Keep a stable-file diagnosis when descriptor cleanup fails around it. */
const projectSnapshotFailure = (
cause: unknown,
@@ -300,7 +339,7 @@ const projectSnapshotFailure = (
};
const verifySourceState = async (
handle: Awaited<ReturnType<typeof openRegularFile>>,
handle: FileHandle,
path: string,
initial: Stats,
observedBytes: number,
@@ -24,6 +24,7 @@ const state = vi.hoisted(() => ({
change: false,
fail: false,
failSource: false,
failAdmission: false,
source: "",
closeCalls: 0,
sourceCloseCalls: 0,
@@ -54,13 +55,29 @@ vi.mock("node:fs/promises", async (importOriginal) => {
) {
state.sourceOpens += 1;
const close = handle.close.bind(handle);
const record = { handle, close, snapshot: false };
const record = {
handle,
close,
snapshot: false,
admit: state.failAdmission && state.sourceOpens > 1,
};
state.latestSource = record;
state.sourceHandles.push(record);
if (record.admit) {
handle.stat = async () => {
throw Object.assign(new Error("controlled admission stat EIO"), {
code: "EIO",
});
};
}
handle.close = async () => {
state.sourceCloseCalls += 1;
// Only the descriptor still open when the snapshot destination is
// created belongs to the copy. Later admission reads must close.
if (record.admit && state.failAdmission)
throw Object.assign(new Error("controlled admission close EIO"), {
code: "EIO",
});
if (record.snapshot && state.failSource)
throw Object.assign(new Error("controlled source close EIO"), {
code: "EIO",
@@ -408,6 +425,55 @@ test("keeps the changed-file diagnosis when both snapshot closes fail", async ()
}
});
test("retains the source descriptor when snapshot admission cleanup fails", async () => {
const fixture = await createJadxProtocolFixture();
state.source = fixture.apk;
state.change = false;
state.fail = false;
state.failSource = false;
state.failAdmission = true;
state.enabled = true;
state.closeCalls = 0;
state.sourceCloseCalls = 0;
state.opens = 0;
state.sourceOpens = 0;
state.handles = [];
state.sourceHandles = [];
state.latestSource = undefined;
try {
const result = await fixture.service.execute("inspect_android_package", {
path: fixture.apk,
});
expect(result.ok).toBe(false);
if (result.ok) return;
expect(projectAnalysisError(result.error).code).toBe("cleanup_incomplete");
expect(result.error.cleanupIncomplete).toBe(true);
expect(result.error.cleanupResources).toContain(fixture.apk);
expect(state.opens).toBe(0);
const source = state.sourceHandles.find(
(candidate) => candidate.handle.fd >= 0,
);
if (source === undefined) throw new Error("missing source descriptor");
expect(source.handle.fd).toBeGreaterThanOrEqual(0);
await expect(fixture.provider.close()).rejects.toMatchObject({
cleanupIncomplete: true,
});
expect(source.handle.fd).toBeGreaterThanOrEqual(0);
state.failAdmission = false;
await fixture.provider.close();
expect(source.handle.fd).toBe(-1);
expect(result.error.cleanupIncomplete).toBe(false);
} finally {
state.enabled = false;
state.failAdmission = false;
await fixture.provider.close().catch(() => undefined);
for (const owned of [...state.handles, ...state.sourceHandles])
if (owned.handle.fd >= 0) await owned.close().catch(() => undefined);
await verifyJadxFixtureCleanup(fixture.launches);
}
});
test("classifies a nested snapshot close as cancellation", () => {
const reason = { requested: "snapshot-close" };
const copy = {