From 612bbee947ef0f14709cf9830397e1bd35193f70 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:56:42 +0800 Subject: [PATCH] fix(artifacts): retain dylib enumeration readers through failed cleanup --- src/artifacts/apple/DylibResolutionReader.ts | 76 ++++++++++------ .../dylibDirectoryLifecycle.test.ts | 89 +++++++++++++++++++ 2 files changed, 137 insertions(+), 28 deletions(-) create mode 100644 tests/process-global/dylibDirectoryLifecycle.test.ts diff --git a/src/artifacts/apple/DylibResolutionReader.ts b/src/artifacts/apple/DylibResolutionReader.ts index 6e6954734d..f839348386 100644 --- a/src/artifacts/apple/DylibResolutionReader.ts +++ b/src/artifacts/apple/DylibResolutionReader.ts @@ -147,7 +147,7 @@ export const traceDylibResolution = async (options: { } = parsed.data.roots !== undefined ? { roots: parsed.data.roots, unclassified: [], traversal: undefined } : options.enumerateRoots - ? await executableRoots(root, view, files, options.signal) + ? await executableRoots(root, view, files, resources, options.signal) : { roots: [target], unclassified: [], traversal: undefined }; const roots = await requireMachoRoots( view, @@ -203,6 +203,7 @@ const executableRoots = async ( root: string, view: FilesystemTreeView, files: DylibFileAccess, + resources: ArtifactResourceScope, signal?: AbortSignal, ): Promise<{ readonly roots: string[]; @@ -216,34 +217,53 @@ const executableRoots = async ( let firstSymlink: string | undefined; // 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, traversal), - MAGIC_READ_CONCURRENCY, - (entry) => { - // The ordered-read owner skips consume() when read() returns undefined. - // Discovery facts for skipped entries belong at this selection boundary. - if (entry.kind === "symlink") { - symlinks += 1; - firstSymlink ??= entry.path; - } - return entry.kind === "file" - ? files.startsWithMachoMagic(entry) - : 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" && - machoImageSlices(facts).some( - ({ file_type: type }) => type === "execute", + const reader = new DirectoryArtifactReader(root); + let outcome: + | { readonly kind: "completed" } + | { readonly kind: "failed"; readonly cause: unknown } = { + kind: "completed", + }; + try { + await consumeWithOrderedReads( + reader.entries(signal, traversal), + MAGIC_READ_CONCURRENCY, + (entry) => { + // The ordered-read owner skips consume() when read() returns undefined. + // Discovery facts for skipped entries belong at this selection boundary. + if (entry.kind === "symlink") { + symlinks += 1; + firstSymlink ??= entry.path; + } + return entry.kind === "file" + ? files.startsWithMachoMagic(entry) + : 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" && + machoImageSlices(facts).some( + ({ file_type: type }) => type === "execute", + ) ) - ) - roots.push(entry.path); - }, - ); + roots.push(entry.path); + }, + ); + } catch (cause: unknown) { + outcome = { kind: "failed", cause }; + } + const released = await resources.release({ + kind: "reader", + reader, + resource: `executable-root reader for ${root}`, + }); + if (released.kind === "failed") + throw resources.cleanupFailure(outcome, [released]); + if (outcome.kind === "failed") throw outcome.cause; + assertArtifactReadActive(signal); if (symlinks > 0) traversal.onCoverage({ status: "unknown", diff --git a/tests/process-global/dylibDirectoryLifecycle.test.ts b/tests/process-global/dylibDirectoryLifecycle.test.ts new file mode 100644 index 0000000000..c6f349eef3 --- /dev/null +++ b/tests/process-global/dylibDirectoryLifecycle.test.ts @@ -0,0 +1,89 @@ +import type { Dir } from "node:fs"; +import { mkdir, realpath, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, vi } from "vitest"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +const fault = vi.hoisted(() => ({ + root: "", + block: false, + closes: 0, + nativeClose: undefined as (() => Promise) | undefined, + handle: undefined as Dir | undefined, +})); +vi.mock("node:fs/promises", async (importOriginal) => { + const filesystem = await importOriginal(); + return { + ...filesystem, + opendir: async (...args: Parameters) => { + const handle = await filesystem.opendir(...args); + if (String(args[0]) === fault.root) { + fault.handle = handle; + const close = handle.close.bind(handle); + fault.nativeClose = close; + handle.close = async () => { + fault.closes += 1; + if (fault.block) + throw new Error("controlled directory native-close failure"); + await close(); + }; + } + return handle; + }, + }; +}); +import { ArtifactProvider } from "../../src/artifacts/ArtifactProvider.js"; +import { parseBinaryTarget } from "../../src/application/BinaryTargetResolver.js"; +import { machoImage } from "../../src/artifacts/apple/MachoImage.fixture.js"; + +it.each([false, true])( + "retains automatic dylib-root reader cleanup authority after native-close failure (blocked: %s)", + async (blocked) => { + const parent = await createTestTempDirectory("rea-dyld-directory-owner-"); + const root = join(parent, "Fixture.app"); + await mkdir(join(root, "Contents", "MacOS"), { recursive: true }); + await writeFile( + join(root, "Contents", "Info.plist"), + "CFBundleExecutableMain", + ); + await writeFile(join(root, "Contents", "MacOS", "Main"), machoImage({})); + const target = await parseBinaryTarget(root); + if (!target.ok) throw target.error; + const client = new ArtifactProvider({}).createClient(target.value); + fault.root = await realpath(root); + fault.block = blocked; + fault.closes = 0; + try { + const traced = await client.execute("trace_dylib_resolution", {}); + const closed = await client.close(); + if (blocked) { + expect(traced).toMatchObject({ + ok: false, + error: { cleanupIncomplete: true }, + }); + expect(closed).toMatchObject({ + ok: false, + error: { cleanupIncomplete: true }, + }); + // A rejected native close is unknown, so ownership remains retained; + // a second operation must not authorize another native close attempt. + expect(fault.closes).toBe(1); + await expect(fault.handle?.read()).resolves.toBeDefined(); + } else { + expect(traced.ok).toBe(true); + expect(closed.ok).toBe(true); + expect(fault.closes).toBe(1); + await expect(fault.handle?.read()).rejects.toMatchObject({ + code: "ERR_DIR_CLOSED", + }); + } + } finally { + fault.block = false; + if (blocked) await fault.nativeClose?.(); + await client.close(); + fault.root = ""; + fault.handle = undefined; + fault.nativeClose = undefined; + } + }, +);