diff --git a/src/artifacts/OrderedReads.test.ts b/src/artifacts/OrderedReads.test.ts new file mode 100644 index 0000000000..5e6afe006d --- /dev/null +++ b/src/artifacts/OrderedReads.test.ts @@ -0,0 +1,185 @@ +import { describe, expect, it } from "vitest"; + +import { consumeWithOrderedReads } from "./OrderedReads.js"; + +/** A read the test settles explicitly. */ +const controlled = () => { + let resolve!: (value: Value) => void; + let reject!: (error: unknown) => void; + let settled = false; + const promise = new Promise((onResolve, onReject) => { + resolve = (value) => { + settled = true; + onResolve(value); + }; + reject = (error) => { + settled = true; + onReject(error); + }; + }); + return { promise, resolve, reject, settled: () => settled }; +}; + +/** Let every queued continuation run. */ +const drain = () => new Promise((resolve) => setImmediate(resolve)); + +/** Yield the items, then optionally fail; records whether it was closed early. */ +const enumeration = ( + items: readonly string[], + failure?: Error, +): { + readonly items: AsyncIterable; + readonly closed: () => boolean; +} => { + let closed = false; + return { + items: (async function* () { + try { + yield* items; + if (failure !== undefined) throw failure; + } finally { + closed = true; + } + })(), + closed: () => closed, + }; +}; + +describe("consumeWithOrderedReads", () => { + it("consumes in enumeration order when a later read finishes first", async () => { + const reads = new Map( + ["a", "b", "c"].map((item) => [item, controlled()]), + ); + const consumed: string[] = []; + const done = consumeWithOrderedReads( + enumeration(["a", "b", "c"]).items, + 8, + (item) => reads.get(item)!.promise, + async (item, value) => { + consumed.push(`${item}=${value}`); + }, + ); + await drain(); + reads.get("c")!.resolve("3"); + reads.get("b")!.resolve("2"); + await drain(); + expect(consumed).toEqual([]); + reads.get("a")!.resolve("1"); + await done; + expect(consumed).toEqual(["a=1", "b=2", "c=3"]); + }); + + it("keeps at most width reads in flight and skips undefined reads", async () => { + let inFlight = 0; + let maxInFlight = 0; + const consumed: string[] = []; + await consumeWithOrderedReads( + enumeration(["a", "skip", "b", "c", "d", "e"]).items, + 2, + (item) => { + if (item === "skip") return undefined; + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + return drain().then(() => { + inFlight -= 1; + return item; + }); + }, + async (item) => { + consumed.push(item); + }, + ); + expect(maxInFlight).toBe(2); + expect(consumed).toEqual(["a", "b", "c", "d", "e"]); + }); + + it("reports an earlier read failure before a later one", async () => { + const reads = new Map(["a", "b"].map((item) => [item, controlled()])); + const done = consumeWithOrderedReads( + enumeration(["a", "b"]).items, + 8, + (item) => reads.get(item)!.promise, + async () => undefined, + ); + await drain(); + reads.get("b")!.reject(new Error("b failed")); + await drain(); + reads.get("a")!.reject(new Error("a failed")); + await expect(done).rejects.toThrow("a failed"); + }); + + it("consumes earlier items before reporting an enumeration failure", async () => { + const reads = new Map(["a", "b"].map((item) => [item, controlled()])); + const consumed: string[] = []; + const done = consumeWithOrderedReads( + enumeration(["a", "b"], new Error("walk failed")).items, + 8, + (item) => reads.get(item)!.promise, + async (item) => { + consumed.push(item); + }, + ); + await drain(); + reads.get("a")!.resolve(); + reads.get("b")!.reject(new Error("b failed")); + await expect(done).rejects.toThrow("b failed"); + expect(consumed).toEqual(["a"]); + }); + + it("reports the enumeration failure once earlier items succeed", async () => { + const consumed: string[] = []; + await expect( + consumeWithOrderedReads( + enumeration(["a"], new Error("walk failed")).items, + 8, + async (item) => item, + async (item) => { + consumed.push(item); + }, + ), + ).rejects.toThrow("walk failed"); + expect(consumed).toEqual(["a"]); + }); + + it("closes the enumeration and waits for started reads after a failure", async () => { + const later = controlled(); + const source = enumeration(["a", "b", "c"]); + let settledBeforeReturn = false; + const done = consumeWithOrderedReads( + source.items, + 2, + (item) => + item === "a" ? Promise.reject(new Error("a failed")) : later.promise, + async () => undefined, + ).catch((error: unknown) => { + settledBeforeReturn = later.settled(); + throw error; + }); + await drain(); + expect(source.closed()).toBe(true); + later.resolve(); + await expect(done).rejects.toThrow("a failed"); + expect(settledBeforeReturn).toBe(true); + }); + + it("keeps a consume failure when closing the enumeration also fails", async () => { + const items: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ + next: async () => ({ done: false, value: "a" }), + return: async () => { + throw new Error("close failed"); + }, + }), + }; + await expect( + consumeWithOrderedReads( + items, + 1, + async () => undefined, + async () => { + throw new Error("consume failed"); + }, + ), + ).rejects.toThrow("consume failed"); + }); +}); diff --git a/src/artifacts/OrderedReads.ts b/src/artifacts/OrderedReads.ts new file mode 100644 index 0000000000..f5611d6969 --- /dev/null +++ b/src/artifacts/OrderedReads.ts @@ -0,0 +1,63 @@ +type Settled = + | { readonly ok: true; readonly value: Value } + | { readonly ok: false; readonly error: unknown }; + +/** + * Overlap independent reads with an enumeration while consuming their results + * in enumeration order. `read` returns undefined for items it skips. At most + * `width` reads are in flight. A failure is reported as a one-at-a-time loop + * would report it: an earlier item's read or consume failure wins over a + * later read failure or an enumeration failure. Every started read settles + * before this returns or throws, and an enumeration left early is closed as + * `for await` would close it. + */ +export const consumeWithOrderedReads = async ( + items: AsyncIterable, + width: number, + read: (item: Item) => Promise | undefined, + consume: (item: Item, value: Value) => Promise, +): Promise => { + const pending: { + readonly item: Item; + readonly result: Promise>; + }[] = []; + const consumeNext = async (): Promise => { + const next = pending.shift(); + if (next === undefined) return; + const result = await next.result; + if (!result.ok) throw result.error; + await consume(next.item, result.value); + }; + const iterator = items[Symbol.asyncIterator](); + let iterating = true; + try { + while (iterating) { + let step: IteratorResult; + try { + step = await iterator.next(); + } catch (cause: unknown) { + iterating = false; + while (pending.length > 0) await consumeNext(); + throw cause; + } + if (step.done === true) { + iterating = false; + break; + } + const started = read(step.value); + if (started === undefined) continue; + pending.push({ + item: step.value, + result: started.then( + (value) => ({ ok: true, value }) as const, + (error: unknown) => ({ ok: false, error }) as const, + ), + }); + if (pending.length >= width) await consumeNext(); + } + while (pending.length > 0) await consumeNext(); + } finally { + if (iterating) await iterator.return?.().catch(() => undefined); + await Promise.all(pending.map(({ result }) => result)); + } +}; diff --git a/src/artifacts/apple/DylibResolutionReader.ts b/src/artifacts/apple/DylibResolutionReader.ts index 7b07528693..92d03c7e3a 100644 --- a/src/artifacts/apple/DylibResolutionReader.ts +++ b/src/artifacts/apple/DylibResolutionReader.ts @@ -17,10 +17,13 @@ import { import { projectInputIssues } from "../../domain/inputIssueProjection.js"; import { ArtifactReaderFailure } from "../ArtifactReader.js"; import { DirectoryArtifactReader } from "../DirectoryArtifactReader.js"; +import { consumeWithOrderedReads } from "../OrderedReads.js"; import { hasMachoMagic, readMachoImage } from "./MachoLoadCommandReader.js"; const OPERATION = "trace_dylib_resolution"; const HASH_CHUNK_BYTES = 1024 * 1024; +/** Concurrent four-byte magic reads while enumerating bundle roots. */ +const MAGIC_READ_CONCURRENCY = 8; /** Device, inode, size and change times of the file whose bytes were parsed. */ type FileIdentity = string; @@ -184,21 +187,27 @@ const executableRoots = async ( ): Promise<{ readonly roots: string[]; readonly unclassified: string[] }> => { const roots: string[] = []; const unclassified: string[] = []; - for await (const entry of new DirectoryArtifactReader(root).entries(signal)) { - if ( - entry.kind !== "file" || - !(await startsWithMachoMagic(entry.adapterKey)) - ) - continue; - const facts = await view.image(entry.path); - if (facts.status === "malformed" || facts.status === "unsupported") - unclassified.push(entry.path); - else if ( - facts.status === "parsed" && - facts.slices.some(({ file_type: type }) => type === "execute") - ) - roots.push(entry.path); - } + // Bundles hold tens of thousands of files and few Mach-O images, so the + // magic reads overlap the walk while results are classified in walk order. + await consumeWithOrderedReads( + new DirectoryArtifactReader(root).entries(signal), + MAGIC_READ_CONCURRENCY, + (entry) => + entry.kind === "file" + ? startsWithMachoMagic(entry.adapterKey) + : undefined, + async (entry, macho) => { + if (!macho) return; + const facts = await view.image(entry.path); + if (facts.status === "malformed" || facts.status === "unsupported") + unclassified.push(entry.path); + else if ( + facts.status === "parsed" && + facts.slices.some(({ file_type: type }) => type === "execute") + ) + roots.push(entry.path); + }, + ); return { roots: roots.sort(compare), unclassified: unclassified.sort(compare), diff --git a/tests/boundary/cli/dylibResolutionCli.test.ts b/tests/boundary/cli/dylibResolutionCli.test.ts index 6d72e4de61..128c51d161 100644 --- a/tests/boundary/cli/dylibResolutionCli.test.ts +++ b/tests/boundary/cli/dylibResolutionCli.test.ts @@ -685,3 +685,79 @@ cliTest( } }, ); + +/** An app bundle whose roots sit among more files than one batch of magic reads. */ +const crowdedApp = async (): Promise => { + const app = await fixtureApp(); + await withBrokenHelper(app); + const resources: Record = {}; + for (let index = 0; index < 40; index += 1) + resources[`Contents/Resources/r${index % 4}/file${index}.dat`] = "x".repeat( + index % 5, + ); + for (const index of [0, 1, 2]) + resources[`Contents/Resources/r${index}/tool${index}`] = machoImage({ + commands: [buildVersionCommand(1)], + }); + resources["Contents/Resources/r3/libextra.dylib"] = machoImage({ + fileType: FILE_TYPE.dylib, + }); + await writeFiles(app, resources); + return app; +}; + +describe("trace-dylib-resolution root enumeration", () => { + cliTest( + "classifies every Mach-O among more files than one batch of magic reads", + async ({ cli }) => { + const app = await crowdedApp(); + const result = await cli.run({ + arguments: ["trace-dylib-resolution", app, "--json"], + environment: ENVIRONMENT, + }); + expect(result.exitCode, JSON.stringify(result.json)).toBe(0); + const trace = dylibResolutionResultSchema.parse( + (result.json as { normalized_result: unknown }).normalized_result, + ); + expect(trace.roots.map(({ image }) => image)).toEqual([ + "Contents/Helpers/tool", + "Contents/MacOS/App", + "Contents/Resources/r0/tool0", + "Contents/Resources/r1/tool1", + "Contents/Resources/r2/tool2", + ]); + expect(trace.coverage).toMatchObject({ + status: "partial", + unparsed_images: ["Contents/Helpers/broken"], + }); + }, + ); + + // This permission fixture requires POSIX mode bits and a non-root caller. + cliTest.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports an unreadable bundle file found while enumerating roots", + async ({ cli }) => { + const app = await crowdedApp(); + const secret = join(app, "Contents/Resources/r2/secret.dat"); + await writeFile(secret, "x"); + await chmod(secret, 0o000); + try { + const result = await cli.run({ + arguments: ["trace-dylib-resolution", app, "--json"], + environment: ENVIRONMENT, + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + details: { + reason: "unavailable", + detail: expect.stringContaining( + "Permission denied (EACCES) reading Contents/Resources/r2/secret.dat", + ), + }, + }); + } finally { + await chmod(secret, 0o644); + } + }, + ); +});