diff --git a/src/artifacts/AsarArtifactReader.ts b/src/artifacts/AsarArtifactReader.ts index 9b33d03ea..1408bce8a 100644 --- a/src/artifacts/AsarArtifactReader.ts +++ b/src/artifacts/AsarArtifactReader.ts @@ -41,6 +41,11 @@ export class AsarArtifactReader implements ArtifactReader { #archiveSize: number | undefined; #headerSize: number | undefined; readonly #entries = new Map(); + readonly #unpackedHandles = new Map(); + readonly #unpackedHandleClosePromises = new Map< + UnpackedFileHandle, + Promise + >(); #snapshot: { readonly path: string; readonly sha256: string } | undefined; #snapshotRoot: string | undefined; @@ -156,32 +161,8 @@ export class AsarArtifactReader implements ArtifactReader { ); const { metadata } = state; const producedEntry = state.entry; - if (producedEntry.unpacked) { - const handle = await this.#openUnpackedEntry(producedEntry); - try { - const observed = await handle.stat(); - abortIfNeeded(signal); - if (!observed.isFile()) - throw new ArtifactReaderFailure( - "path", - `ASAR unpacked entry is not a regular file: ${producedEntry.path}`, - ); - } catch (cause: unknown) { - await handle.close().catch(() => undefined); - throw asarFailure(this.path, `read ${producedEntry.path}`, cause); - } - const source = handle.createReadStream({ - start: 0, - autoClose: true, - }); - return readValidatedAsarEntry( - source, - undefined, - producedEntry.path, - this.path, - signal, - ); - } + if (producedEntry.unpacked) + return this.#openUnpackedStream(producedEntry, signal); const archiveSize = this.#archiveSize; const headerSize = this.#headerSize; const offset = @@ -315,22 +296,39 @@ export class AsarArtifactReader implements ArtifactReader { } async close(): Promise { + let cleanupFailure: ArtifactReaderFailure | undefined; + for (const [handle, path] of this.#unpackedHandles) { + try { + await this.#closeUnpackedHandle(handle); + } catch (cause: unknown) { + cleanupFailure = ArtifactReaderFailure.withCleanup( + cleanupFailure ?? cause, + ArtifactReaderFailure.cleanupObservation(cause, path), + ); + } + } const root = this.#snapshotRoot; if (root !== undefined) { try { await this.removeSnapshot(root); + if (this.#snapshotRoot === root) this.#snapshotRoot = undefined; } catch (cause: unknown) { - throw ArtifactReaderFailure.withCleanup(cause, { - reason: `Could not remove ASAR snapshot: ${root}`, - resources: [root], - }); + cleanupFailure = ArtifactReaderFailure.withCleanup( + cleanupFailure ?? cause, + { + reason: `Could not remove ASAR snapshot: ${root}`, + resources: [root], + }, + ); } - this.#snapshotRoot = undefined; } - if (this.#snapshot !== undefined) uncache(this.#snapshot.path); - this.#snapshot = undefined; - this.#resetArchiveState(); - this.#entries.clear(); + if (root === undefined || this.#snapshotRoot === undefined) { + if (this.#snapshot !== undefined) uncache(this.#snapshot.path); + this.#snapshot = undefined; + this.#resetArchiveState(); + this.#entries.clear(); + } + if (cleanupFailure !== undefined) throw cleanupFailure; } #snapshotPath(): string { @@ -346,8 +344,12 @@ export class AsarArtifactReader implements ArtifactReader { return []; } - async #openUnpackedEntry(entry: ArtifactEntry): Promise { + async #openUnpackedStream( + entry: ArtifactEntry, + signal?: AbortSignal, + ): Promise { const unpackedRoot = `${this.path}.unpacked`; + let handle: UnpackedFileHandle | undefined; try { const rootMetadata = await lstat(unpackedRoot); if (!rootMetadata.isDirectory() || rootMetadata.isSymbolicLink()) @@ -374,22 +376,18 @@ export class AsarArtifactReader implements ArtifactReader { "path", `ASAR unpacked entry is not a regular file: ${entry.path}`, ); - const handle = await this.openUnpackedFile( + handle = await this.openUnpackedFile( canonical, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK, ); - const openedMetadata = await handle - .stat() - .catch(async (cause: unknown) => { - await handle.close().catch(() => undefined); - throw cause; - }); + const openedHandle = handle; + this.#unpackedHandles.set(openedHandle, canonical); + const openedMetadata = await openedHandle.stat(); if ( !openedMetadata.isFile() || openedMetadata.dev !== pathMetadata.dev || openedMetadata.ino !== pathMetadata.ino ) { - await handle.close(); throw new ArtifactReaderFailure( "integrity", `ASAR unpacked entry changed before read: ${entry.path}`, @@ -402,25 +400,54 @@ export class AsarArtifactReader implements ArtifactReader { }, ); } - return handle; + abortIfNeeded(signal); + const source = openedHandle.createReadStream({ + start: 0, + autoClose: false, + }); + source.once("close", () => { + void this.#closeUnpackedHandle(openedHandle).catch(() => undefined); + }); + return readValidatedAsarEntry( + source, + undefined, + entry.path, + this.path, + signal, + ); } catch (cause: unknown) { - if (cause instanceof ArtifactReaderFailure) throw cause; - if (entry.unpacked && isMissingFile(cause)) - throw new ArtifactReaderFailure( - "unavailable", - `ASAR unpacked entry bytes are unavailable: ${entry.path}`, - { cause }, - { - logicalPath: entry.path, - declaredSha256: entry.declaredSha256, - calculatedSha256: null, - unpacked: true, - }, - ); - throw asarFailure(this.path, `read ${entry.path}`, cause); + let failure = asarUnpackedEntryFailure(this.path, entry, cause); + if (handle !== undefined && this.#unpackedHandles.has(handle)) { + try { + await this.#closeUnpackedHandle(handle); + } catch (cleanupCause: unknown) { + failure = ArtifactReaderFailure.withCleanup( + failure, + ArtifactReaderFailure.cleanupObservation( + cleanupCause, + this.#unpackedHandles.get(handle) ?? entry.path, + ), + ); + } + } + throw failure; } } + async #closeUnpackedHandle(handle: UnpackedFileHandle): Promise { + if (!this.#unpackedHandles.has(handle)) return; + const pending = this.#unpackedHandleClosePromises.get(handle); + if (pending !== undefined) return pending; + const closing = Promise.resolve() + .then(() => handle.close()) + .then(() => { + this.#unpackedHandles.delete(handle); + }) + .finally(() => this.#unpackedHandleClosePromises.delete(handle)); + this.#unpackedHandleClosePromises.set(handle, closing); + return closing; + } + #resetArchiveState(): void { this.#archiveSize = undefined; this.#headerSize = undefined; @@ -442,6 +469,27 @@ type OpenAsarUnpackedFile = ( flags: number, ) => Promise; +const asarUnpackedEntryFailure = ( + archivePath: string, + entry: ArtifactEntry, + cause: unknown, +): ArtifactReaderFailure => { + if (cause instanceof ArtifactReaderFailure) return cause; + if (entry.unpacked && isMissingFile(cause)) + return new ArtifactReaderFailure( + "unavailable", + `ASAR unpacked entry bytes are unavailable: ${entry.path}`, + { cause }, + { + logicalPath: entry.path, + declaredSha256: entry.declaredSha256, + calculatedSha256: null, + unpacked: true, + }, + ); + return asarFailure(archivePath, `read ${entry.path}`, cause); +}; + const copyArtifactEntry = (entry: ArtifactEntry): ArtifactEntry => ({ ...entry, limitations: [...entry.limitations], diff --git a/tests/boundary/filesystem/asarStreamingAllocation.test.ts b/tests/boundary/filesystem/asarStreamingAllocation.test.ts index cc9438179..24584063d 100644 --- a/tests/boundary/filesystem/asarStreamingAllocation.test.ts +++ b/tests/boundary/filesystem/asarStreamingAllocation.test.ts @@ -305,19 +305,22 @@ describe("ASAR read failures", () => { errno: -5, syscall: "fstat", }); + const closeError = Object.assign(new Error("device close failed"), { + code: "EIO", + errno: -5, + syscall: "close", + }); let closeCount = 0; const reader = new AsarArtifactReader(archive, async (path, flags) => { const handle = await openFile(path, flags); expect(await matchingDescriptorCount(identity)).toBeGreaterThan(0); - let statCount = 0; return { async stat() { - statCount += 1; - if (statCount === 2) throw ioError; - return handle.stat(); + throw ioError; }, async close() { closeCount += 1; + if (closeCount === 1) throw closeError; await handle.close(); }, createReadStream: (options) => handle.createReadStream(options), @@ -337,73 +340,320 @@ describe("ASAR read failures", () => { message: expect.stringContaining( `Could not read ${entry.path} ASAR at ${archive}: device stat failed`, ), - cause: { code: "EIO", errno: -5, syscall: "fstat" }, + cause: { + reason: "io", + cause: { code: "EIO", errno: -5, syscall: "fstat" }, + }, + cleanup: { + resources: [unpackedPath], + reason: "device close failed", + }, }); expect(closeCount).toBe(1); + expect(await matchingDescriptorCount(identity)).toBeGreaterThan(0); + await reader.close(); + expect(closeCount).toBe(2); await waitForNoMatchingDescriptor(identity); } finally { await reader.close(); } }, ); +}); - it.each(["missing", "replaced"] as const)( - "reads a captured zero-length member when the original container is %s", - async (change) => { - const root = await createTestTempDirectory("rea-asar-empty-member-"); - const source = join(root, "source"); - const archive = join(root, "fixture.asar"); - await mkdir(source); - await writeFile(join(source, "empty.js"), ""); - await createPackageWithOptions(source, archive, {}); - const reader = new AsarArtifactReader(archive); +it.skipIf(process.platform === "win32")( + "retries cleanup when cancellation arrives immediately after unpacked stat", + async () => { + const root = await createTestTempDirectory("rea-asar-unpacked-abort-stat-"); + const source = join(root, "source"); + const archive = join(root, "fixture.asar"); + await mkdir(source); + await writeFile(join(source, "small.js"), "module.exports = 1;\n"); + await createPackageWithOptions(source, archive, { unpack: "small.js" }); + const unpackedPath = join(`${archive}.unpacked`, "small.js"); + const identity = await lstat(unpackedPath); + const controller = new AbortController(); + let closeCount = 0; + const reader = new AsarArtifactReader(archive, async (path, flags) => { + const handle = await openFile(path, flags); + return { + async stat() { + const observed = await handle.stat(); + controller.abort(); + return observed; + }, + async close() { + closeCount += 1; + if (closeCount === 1) throw new Error("first close failed"); + await handle.close(); + }, + createReadStream: (options) => handle.createReadStream(options), + }; + }); - try { - const entries = []; - for await (const entry of reader.entries()) entries.push(entry); - const entry = entries.find(({ path }) => path === "empty.js"); - if (entry === undefined) throw new Error("Expected empty ASAR member"); - if (change === "missing") await rm(archive); - else await writeFile(archive, "replacement container"); + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries.find(({ path }) => path === "small.js"); + if (entry === undefined) throw new Error("Expected unpacked ASAR member"); - expect(await buffer(await reader.open(entry))).toHaveLength(0); - } finally { - await reader.close(); - } - }, + await expect(reader.open(entry, controller.signal)).rejects.toMatchObject( + { + reason: "cancelled", + cleanup: { + resources: [unpackedPath], + reason: "first close failed", + }, + }, + ); + expect(closeCount).toBe(1); + expect(await matchingDescriptorCount(identity)).toBeGreaterThan(0); + await reader.close(); + expect(closeCount).toBe(2); + await waitForNoMatchingDescriptor(identity); + } finally { + await reader.close(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "preserves an unpacked identity failure when closing its handle fails", + async () => { + const root = await createTestTempDirectory("rea-asar-unpacked-identity-"); + const source = join(root, "source"); + const archive = join(root, "fixture.asar"); + await mkdir(source); + await writeFile(join(source, "small.js"), "module.exports = 1;\n"); + await createPackageWithOptions(source, archive, { unpack: "small.js" }); + const unpackedPath = join(`${archive}.unpacked`, "small.js"); + const identity = await lstat(unpackedPath); + const closeError = new Error("first close failed"); + let closeCount = 0; + const reader = new AsarArtifactReader(archive, async (path, flags) => { + const handle = await openFile(path, flags); + return { + async stat() { + const observed = await handle.stat(); + return Object.assign(Object.create(observed), { + ino: observed.ino + 1, + }); + }, + async close() { + closeCount += 1; + if (closeCount === 1) throw closeError; + await handle.close(); + }, + createReadStream: (options) => handle.createReadStream(options), + }; + }); + + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries.find(({ path }) => path === "small.js"); + if (entry === undefined) throw new Error("Expected unpacked ASAR member"); + + await expect(reader.open(entry)).rejects.toMatchObject({ + name: "ArtifactReaderFailure", + reason: "integrity", + message: expect.stringContaining( + `ASAR unpacked entry changed before read: ${entry.path}`, + ), + cleanup: { + resources: [unpackedPath], + reason: "first close failed", + }, + }); + expect(closeCount).toBe(1); + expect(await matchingDescriptorCount(identity)).toBeGreaterThan(0); + await reader.close(); + expect(closeCount).toBe(2); + await waitForNoMatchingDescriptor(identity); + } finally { + await reader.close(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "retains unpacked handles and snapshots across combined cleanup failures", + async () => { + const root = await createTestTempDirectory("rea-asar-combined-cleanup-"); + const source = join(root, "source"); + const archive = join(root, "fixture.asar"); + await mkdir(source); + await writeFile(join(source, "small.js"), "module.exports = 1;\n"); + await createPackageWithOptions(source, archive, { unpack: "small.js" }); + const unpackedPath = join(`${archive}.unpacked`, "small.js"); + const identity = await lstat(unpackedPath); + const closeError = new Error("unpacked close denied"); + const snapshotError = new Error("snapshot removal denied"); + let closeCount = 0; + const snapshotPaths: string[] = []; + const reader = new AsarArtifactReader( + archive, + async (path, flags) => { + const handle = await openFile(path, flags); + return { + async stat() { + throw Object.assign(new Error("device stat failed"), { + code: "EIO", + errno: -5, + syscall: "fstat", + }); + }, + async close() { + closeCount += 1; + if (closeCount < 3) throw closeError; + await handle.close(); + }, + createReadStream: (options) => handle.createReadStream(options), + }; + }, + async (path) => { + snapshotPaths.push(path); + if (snapshotPaths.length === 1) throw snapshotError; + await rm(path, { recursive: true, force: true }); + }, + ); + + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries.find(({ path }) => path === "small.js"); + if (entry === undefined) throw new Error("Expected unpacked ASAR member"); + + await expect(reader.open(entry)).rejects.toMatchObject({ + reason: "io", + cleanup: { resources: [unpackedPath] }, + }); + expect(closeCount).toBe(1); + + const cleanupFailure = await reader.close().then( + () => undefined, + (cause: unknown) => cause, + ); + expect(cleanupFailure).toMatchObject({ + cleanup: { + resources: [unpackedPath, expect.any(String)], + reason: expect.stringContaining("unpacked close denied"), + }, + }); + const snapshotRoot = snapshotPaths[0]; + if (snapshotRoot === undefined) + throw new Error("Expected owned ASAR snapshot root"); + expect(snapshotPaths).toEqual([snapshotRoot]); + expect(cleanupFailure).toMatchObject({ + cleanup: { resources: [unpackedPath, snapshotRoot] }, + }); + expect(closeCount).toBe(2); + expect(await matchingDescriptorCount(identity)).toBeGreaterThan(0); + expect((await lstat(snapshotRoot)).isDirectory()).toBe(true); + + await reader.close(); + expect(closeCount).toBe(3); + expect(snapshotPaths).toEqual([snapshotRoot, snapshotRoot]); + await waitForNoMatchingDescriptor(identity); + await expect(lstat(snapshotRoot)).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + await reader.close(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "releases unpacked handles after stream completion and cancellation", + async () => { + const root = await createTestTempDirectory("rea-asar-unpacked-stream-"); + const source = join(root, "source"); + const archive = join(root, "fixture.asar"); + await mkdir(source); + await writeFile(join(source, "small.js"), "module.exports = 1;\n"); + await createPackageWithOptions(source, archive, { unpack: "small.js" }); + const identity = await lstat(join(`${archive}.unpacked`, "small.js")); + const reader = new AsarArtifactReader(archive); + + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries.find(({ path }) => path === "small.js"); + if (entry === undefined) throw new Error("Expected unpacked ASAR member"); + expect((await buffer(await reader.open(entry))).toString()).toBe( + "module.exports = 1;\n", + ); + await waitForNoMatchingDescriptor(identity); + + const controller = new AbortController(); + const output = await reader.open(entry, controller.signal); + const reading = buffer(output); + controller.abort(); + await expect(reading).rejects.toMatchObject({ reason: "cancelled" }); + await waitForNoMatchingDescriptor(identity); + } finally { + await reader.close(); + } + }, +); + +it.each(["missing", "replaced"] as const)( + "reads a captured zero-length member when the original container is %s", + async (change) => { + const root = await createTestTempDirectory("rea-asar-empty-member-"); + const source = join(root, "source"); + const archive = join(root, "fixture.asar"); + await mkdir(source); + await writeFile(join(source, "empty.js"), ""); + await createPackageWithOptions(source, archive, {}); + const reader = new AsarArtifactReader(archive); + + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries.find(({ path }) => path === "empty.js"); + if (entry === undefined) throw new Error("Expected empty ASAR member"); + if (change === "missing") await rm(archive); + else await writeFile(archive, "replacement container"); + + expect(await buffer(await reader.open(entry))).toHaveLength(0); + } finally { + await reader.close(); + } + }, +); + +it("preserves filesystem errors raised while consuming a member stream", async () => { + const ioError = Object.assign(new Error("device read failed"), { + code: "EIO", + errno: -5, + syscall: "read", + }); + const source = Readable.from( + (async function* () { + yield Buffer.from("partial"); + throw ioError; + })(), + ); + const output = readValidatedAsarEntry( + source, + undefined, + "nested/member.bin", + "/tmp/source.asar", ); - it("preserves filesystem errors raised while consuming a member stream", async () => { - const ioError = Object.assign(new Error("device read failed"), { - code: "EIO", - errno: -5, - syscall: "read", - }); - const source = Readable.from( - (async function* () { - yield Buffer.from("partial"); - throw ioError; - })(), - ); - const output = readValidatedAsarEntry( - source, - undefined, - "nested/member.bin", - "/tmp/source.asar", - ); - - await expect(async () => { - for await (const _chunk of output) { - // Consume through the source failure. - } - }).rejects.toMatchObject({ - name: "ArtifactReaderFailure", - reason: "io", - message: expect.stringContaining( - "Could not read nested/member.bin ASAR at /tmp/source.asar", - ), - cause: { code: "EIO", errno: -5, syscall: "read" }, - }); + await expect(async () => { + for await (const _chunk of output) { + // Consume through the source failure. + } + }).rejects.toMatchObject({ + name: "ArtifactReaderFailure", + reason: "io", + message: expect.stringContaining( + "Could not read nested/member.bin ASAR at /tmp/source.asar", + ), + cause: { code: "EIO", errno: -5, syscall: "read" }, }); });