diff --git a/src/android/AndroidTargetSnapshot.ts b/src/android/AndroidTargetSnapshot.ts index 1f6cf26435..28c685c811 100644 --- a/src/android/AndroidTargetSnapshot.ts +++ b/src/android/AndroidTargetSnapshot.ts @@ -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 => { + 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>, + handle: FileHandle, path: string, initial: Stats, observedBytes: number, diff --git a/tests/boundary/process/androidSnapshotClose.test.ts b/tests/boundary/process/androidSnapshotClose.test.ts index bebb0d40dc..ac0545a9a9 100644 --- a/tests/boundary/process/androidSnapshotClose.test.ts +++ b/tests/boundary/process/androidSnapshotClose.test.ts @@ -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 = {