From fd4dd801aa2fe4a060e64c6b3d02fcfb7130d5ef Mon Sep 17 00:00:00 2001 From: Navid Nadali Date: Wed, 4 Nov 2026 10:53:15 +0000 Subject: [PATCH 1/2] fix(shared): looking up the Xcode git no longer blocks the server Resolve the Xcode git with an asynchronous xcrun spawn instead of execFileSync on the event loop, share one in-flight lookup between callers, and give the lookup its own deadline. Co-authored-by: Claude Opus 5.5 --- packages/shared/src/shell.test.ts | 100 +++++++++++++++++++++++++- packages/shared/src/shell.ts | 112 ++++++++++++++++++++++++------ 2 files changed, 190 insertions(+), 22 deletions(-) diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index e3af0855ebfac..bc514fc156dc1 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -23,6 +23,7 @@ import { type CommandAvailabilityChecker, isCommandAvailable, listLoginShellCandidates, + locateXcodeGitWithXcrun, mergePathEntries, mergePathValues, npmShimExecutable, @@ -44,12 +45,17 @@ const decodeDesktopHostMessage = Schema.decodeUnknownSync(DesktopHostTelemetryMe const nativeProbe = vi.hoisted(() => ({ execFile: vi.fn(), original: undefined as unknown as typeof import("node:child_process").execFile, + spawn: vi.fn(), })); vi.mock("node:child_process", async (importOriginal) => { const original = await importOriginal(); nativeProbe.original = original.execFile; - return { ...original, execFile: nativeProbe.execFile.mockImplementation(original.execFile) }; + return { + ...original, + execFile: nativeProbe.execFile.mockImplementation(original.execFile), + spawn: nativeProbe.spawn.mockImplementation(original.spawn), + }; }); const withWindowsEnvironmentMocks = ( @@ -860,7 +866,7 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { Effect.provideService(SpawnExecutableResolution, (command) => command === "git" ? options.gitOnPath : files.has(command) ? command : undefined, ), - Effect.provideService(XcodeGitLocation, () => { + Effect.provideService(XcodeGitLocation, async () => { host.xcrunCalls += 1; return host.xcrunGit; }), @@ -879,6 +885,96 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); + it.effect("waits for xcrun without blocking, sharing one lookup between callers", () => + Effect.gen(function* () { + let finishLookup: (located: string | undefined) => void = () => {}; + const lookup = new Promise((resolve) => { + finishLookup = resolve; + }); + let xcrunCalls = 0; + const resolve = resolveSpawnCommand("git", ["status"], { env: { PATH: "/fake/bin" } }).pipe( + Effect.map((resolved) => resolved.command), + Effect.provideService(HostProcess.Platform, "darwin"), + Effect.provideService(SpawnExecutableResolution, (command) => + command === "git" ? "/usr/bin/git" : command === XCODE_GIT ? command : undefined, + ), + Effect.provideService(XcodeGitLocation, () => { + xcrunCalls += 1; + return lookup; + }), + ); + const first = yield* Effect.forkChild(resolve, { startImmediately: true }); + const second = yield* Effect.forkChild(resolve, { startImmediately: true }); + finishLookup(XCODE_GIT); + + expect(yield* Fiber.joinAll([first, second])).toEqual([XCODE_GIT, XCODE_GIT]); + expect(xcrunCalls).toBe(1); + }).pipe(Effect.provideService(CommandResolutionCache, new Map())), + ); + + it.effect("the xcrun lookup resolves nothing when xcrun is missing or fails", () => + Effect.gen(function* () { + const env = { PATH: "/usr/bin:/bin", DEVELOPER_DIR: "/nonexistent/t3-developer-dir" }; + const locate = yield* XcodeGitLocation; + expect(yield* Effect.promise(() => locate(env))).toBeUndefined(); + const missing = locateXcodeGitWithXcrun(env, { xcrun: "/nonexistent/t3-xcrun" }); + expect(yield* Effect.promise(() => missing)).toBeUndefined(); + }), + ); + + it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")( + "the xcrun lookup settles at its deadline when a descendant keeps stdout open", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-xcrun-" }); + const descendantPid = path.join(directory, "descendant.pid"); + const xcrun = path.join(directory, "xcrun"); + yield* fs.writeFileString( + xcrun, + `#!/bin/sh\nsleep 600 &\necho $! > '${descendantPid}'\necho /fake/git\n`, + ); + yield* fs.chmod(xcrun, 0o755); + const stopDescendant = fs.readFileString(descendantPid).pipe( + Effect.flatMap((pid) => Effect.sync(() => process.kill(Number(pid.trim())))), + Effect.ignore, + ); + const located = yield* Effect.promise(() => + locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun, timeoutMs: 200 }), + ).pipe(Effect.ensuring(stopDescendant)); + expect(located).toBeUndefined(); + }).pipe(Effect.scoped), + ); + + it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")( + "the xcrun lookup kills an xcrun that ignores SIGTERM at its deadline", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-xcrun-" }); + const xcrun = path.join(directory, "xcrun"); + yield* fs.writeFileString(xcrun, "#!/bin/sh\ntrap '' TERM\nexec sleep 600\n"); + yield* fs.chmod(xcrun, 0o755); + const lookup = locateXcodeGitWithXcrun( + { PATH: "/usr/bin:/bin" }, + { xcrun, timeoutMs: 200 }, + ); + const child = nativeProbe.spawn.mock.results.at(-1) + ?.value as NodeChildProcess.ChildProcess; + const kill = vi.spyOn(child, "kill"); + const exited = new Promise((resolve) => + child.once("exit", (_code, signal) => resolve(signal)), + ); + yield* Effect.addFinalizer(() => Effect.sync(() => child.kill("SIGKILL"))); + + expect(yield* Effect.promise(() => lookup)).toBeUndefined(); + expect(kill).toHaveBeenCalledWith("SIGKILL"); + expect(yield* Effect.promise(() => exited)).toBe("SIGKILL"); + }).pipe(Effect.scoped), + ); + it.effect("keeps git when PATH has another git or the host is not macOS", () => Effect.gen(function* () { const homebrew = setup({ gitOnPath: "/opt/homebrew/bin/git", xcrunGit: XCODE_GIT }); diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 28b666ce05531..54038a22f45ab 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -760,21 +760,62 @@ export function npmShimExecutable( return isFile(executable) ? executable : undefined; } -export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => string | undefined; +export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => Promise; + +const XCRUN_MAX_OUTPUT_BYTES = 1024 * 1024; // xcrun honors DEVELOPER_DIR and the `xcode-select` choice, as the shim does. -function locateXcodeGitWithXcrun(env: NodeJS.ProcessEnv): string | undefined { - try { - const located = NodeChildProcess.execFileSync("/usr/bin/xcrun", ["--find", "git"], { - encoding: "utf8", - env, - timeout: 5_000, - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return NodePath.isAbsolute(located) ? located : undefined; - } catch { - return undefined; - } +// It takes ~15 ms and reruns when the cached answer expires, so it must not block the event loop. +// The deadline also covers a descendant that keeps stdout open after xcrun exits. +export function locateXcodeGitWithXcrun( + env: NodeJS.ProcessEnv, + options: { readonly xcrun?: string; readonly timeoutMs?: number } = {}, +): Promise { + return new Promise((resolve) => { + let child: NodeChildProcess.ChildProcess; + try { + child = NodeChildProcess.spawn(options.xcrun ?? "/usr/bin/xcrun", ["--find", "git"], { + env, + stdio: ["ignore", "pipe", "ignore"], + }); + } catch { + resolve(undefined); + return; + } + const settle = (located: string | undefined) => { + clearTimeout(deadline); + resolve(located); + }; + // An xcrun that ignores SIGTERM must neither outlive the lookup nor hold the server open. + const giveUp = () => { + child.kill("SIGKILL"); + child.unref(); + child.stdout?.destroy(); + settle(undefined); + }; + // @effect-diagnostics-next-line globalTimers:off - a Promise-based lookup outside any fiber. + const deadline = setTimeout(giveUp, options.timeoutMs ?? 5_000); + deadline.unref(); + const chunks: Array = []; + let bytes = 0; + child.stdout?.on("data", (chunk: Buffer) => { + bytes += chunk.byteLength; + if (bytes > XCRUN_MAX_OUTPUT_BYTES) { + giveUp(); + return; + } + chunks.push(chunk); + }); + child.once("error", () => settle(undefined)); + child.once("close", (code) => { + if (code !== 0) { + settle(undefined); + return; + } + const located = Buffer.concat(chunks).toString("utf8").trim(); + settle(NodePath.isAbsolute(located) ? located : undefined); + }); + }); } export const XcodeGitLocation = Context.Reference( @@ -784,6 +825,26 @@ export const XcodeGitLocation = Context.Reference( const MACOS_GIT_SHIM = "/usr/bin/git"; +// Callers that miss the cache together wait for one xcrun run instead of starting their own. +const xcodeGitLookups = new WeakMap< + Map, + Map> +>(); + +async function lookupXcodeGit( + env: NodeJS.ProcessEnv, + resolveExecutable: SpawnExecutableResolver, + locateXcodeGit: XcodeGitLocator, +): Promise { + const located = + resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM + ? await locateXcodeGit(env).catch(() => undefined) + : undefined; + return located !== undefined && resolveExecutable(located, "darwin", env) !== undefined + ? located + : null; +} + /** * Skip the xcrun shim's per-spawn lookup only when git resolves to /usr/bin/git. * User-selected git on PATH still wins, children keep PATH, and lookup failures use `git`. @@ -812,13 +873,24 @@ const resolveMacosGit = Effect.fnUntraced(function* ( ) { return cached.resolvedPath ?? "git"; } - const located = - resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM ? locateXcodeGit(env) : undefined; - const realGit = - located !== undefined && resolveExecutable(located, "darwin", env) !== undefined - ? located - : null; - cacheCommandResolution(cache, cacheKey, realGit, nowNanos); + let lookups = xcodeGitLookups.get(cache); + if (lookups === undefined) { + lookups = new Map(); + xcodeGitLookups.set(cache, lookups); + } + let lookup = lookups.get(cacheKey); + if (lookup === undefined) { + const pending = lookups; + lookup = lookupXcodeGit(env, resolveExecutable, locateXcodeGit) + .then((realGit) => { + cacheCommandResolution(cache, cacheKey, realGit, nowNanos); + return realGit; + }) + .finally(() => pending.delete(cacheKey)); + pending.set(cacheKey, lookup); + } + const shared = lookup; + const realGit = yield* Effect.promise(() => shared); return realGit ?? "git"; }); From 3b823dcb21d69d7b150d27af248f4b6b98d17bbc Mon Sep 17 00:00:00 2001 From: Navid Nadali Date: Wed, 4 Nov 2026 14:16:07 +0000 Subject: [PATCH 2/2] fix(shared): the xcrun lookup's deadline runs on Effect's clock The Xcode git lookup is now an Effect. Its deadline is an Effect.timeoutOption, as the Windows shell probe in the same file does, instead of a global setTimeout behind a diagnostic suppression. Interrupting it at the deadline still kills xcrun with SIGKILL and stops reading a stdout a descendant holds open. Callers still share one in-flight lookup. It is registered and started in one uninterruptible step and runs detached, so a scheduler yield cannot let a second caller start its own lookup, and a lookup that ends at once never leaves a finished entry behind. The tests drive the deadline with the test clock and cover small yield budgets. Co-Authored-By: Claude Opus 5.5 --- packages/shared/src/shell.test.ts | 154 +++++++++++++++++++----------- packages/shared/src/shell.ts | 129 ++++++++++++++----------- 2 files changed, 169 insertions(+), 114 deletions(-) diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index bc514fc156dc1..3850c76696f6a 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -4,6 +4,8 @@ import { it as effectIt } from "@effect/vitest"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { DesktopHostTelemetryMessage } from "@t3tools/contracts"; import * as Schema from "effect/Schema"; +import * as Scheduler from "effect/Scheduler"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; @@ -58,6 +60,13 @@ vi.mock("node:child_process", async (importOriginal) => { }; }); +const exitOf = (child: NodeChildProcess.ChildProcess) => + child.exitCode !== null || child.signalCode !== null + ? Promise.resolve({ code: child.exitCode, signal: child.signalCode }) + : new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => + child.once("exit", (code, signal) => resolve({ code, signal })), + ); + const withWindowsEnvironmentMocks = ( effect: Effect.Effect, readEnvironment: ( @@ -866,10 +875,12 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { Effect.provideService(SpawnExecutableResolution, (command) => command === "git" ? options.gitOnPath : files.has(command) ? command : undefined, ), - Effect.provideService(XcodeGitLocation, async () => { - host.xcrunCalls += 1; - return host.xcrunGit; - }), + Effect.provideService(XcodeGitLocation, () => + Effect.sync(() => { + host.xcrunCalls += 1; + return host.xcrunGit; + }), + ), ); return { files, host, resolve }; }; @@ -885,40 +896,55 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); - it.effect("waits for xcrun without blocking, sharing one lookup between callers", () => - Effect.gen(function* () { - let finishLookup: (located: string | undefined) => void = () => {}; - const lookup = new Promise((resolve) => { - finishLookup = resolve; - }); - let xcrunCalls = 0; - const resolve = resolveSpawnCommand("git", ["status"], { env: { PATH: "/fake/bin" } }).pipe( - Effect.map((resolved) => resolved.command), - Effect.provideService(HostProcess.Platform, "darwin"), - Effect.provideService(SpawnExecutableResolution, (command) => - command === "git" ? "/usr/bin/git" : command === XCODE_GIT ? command : undefined, - ), - Effect.provideService(XcodeGitLocation, () => { - xcrunCalls += 1; - return lookup; - }), - ); - const first = yield* Effect.forkChild(resolve, { startImmediately: true }); - const second = yield* Effect.forkChild(resolve, { startImmediately: true }); - finishLookup(XCODE_GIT); - - expect(yield* Fiber.joinAll([first, second])).toEqual([XCODE_GIT, XCODE_GIT]); - expect(xcrunCalls).toBe(1); - }).pipe(Effect.provideService(CommandResolutionCache, new Map())), + // A small yield budget lets the scheduler switch callers between any two operations. + it.effect.each([undefined, 3, 4, 5, 6] as const)( + "waits for xcrun without blocking, sharing one lookup between callers (yield budget %s)", + (budget) => + Effect.gen(function* () { + let lookup = yield* Deferred.make(); + let xcrunCalls = 0; + const resolve = resolveSpawnCommand("git", ["status"], { + env: { PATH: "/fake/bin" }, + }).pipe( + Effect.map((resolved) => resolved.command), + Effect.provideService(HostProcess.Platform, "darwin"), + Effect.provideService(SpawnExecutableResolution, (command) => + command === "git" ? "/usr/bin/git" : command === XCODE_GIT ? command : undefined, + ), + Effect.provideService(XcodeGitLocation, () => { + xcrunCalls += 1; + return Deferred.await(lookup); + }), + budget === undefined + ? (effect) => effect + : Effect.provideService(Scheduler.MaxOpsBeforeYield, budget), + ); + const callers = Effect.forEach([1, 2, 3], () => + Effect.forkChild(resolve, { startImmediately: true }), + ); + const first = yield* callers; + yield* Deferred.succeed(lookup, XCODE_GIT); + expect(yield* Fiber.joinAll(first)).toEqual([XCODE_GIT, XCODE_GIT, XCODE_GIT]); + expect(xcrunCalls).toBe(1); + + // Once the answer expires, the next callers share one new lookup. + lookup = yield* Deferred.make(); + yield* TestClock.adjust("31 seconds"); + const second = yield* callers; + yield* Deferred.succeed(lookup, XCODE_GIT); + expect(yield* Fiber.joinAll(second)).toEqual([XCODE_GIT, XCODE_GIT, XCODE_GIT]); + expect(xcrunCalls).toBe(2); + }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); it.effect("the xcrun lookup resolves nothing when xcrun is missing or fails", () => Effect.gen(function* () { const env = { PATH: "/usr/bin:/bin", DEVELOPER_DIR: "/nonexistent/t3-developer-dir" }; const locate = yield* XcodeGitLocation; - expect(yield* Effect.promise(() => locate(env))).toBeUndefined(); - const missing = locateXcodeGitWithXcrun(env, { xcrun: "/nonexistent/t3-xcrun" }); - expect(yield* Effect.promise(() => missing)).toBeUndefined(); + expect(yield* locate(env)).toBeUndefined(); + expect( + yield* locateXcodeGitWithXcrun(env, { xcrun: "/nonexistent/t3-xcrun" }), + ).toBeUndefined(); }), ); @@ -940,10 +966,18 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { Effect.flatMap((pid) => Effect.sync(() => process.kill(Number(pid.trim())))), Effect.ignore, ); - const located = yield* Effect.promise(() => - locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun, timeoutMs: 200 }), - ).pipe(Effect.ensuring(stopDescendant)); - expect(located).toBeUndefined(); + yield* Effect.addFinalizer(() => stopDescendant); + const lookup = yield* locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun }).pipe( + Effect.forkChild({ startImmediately: true }), + ); + const child = nativeProbe.spawn.mock.results.at(-1) + ?.value as NodeChildProcess.ChildProcess; + yield* Effect.promise(() => exitOf(child)); + + // xcrun has exited and printed a path, but its descendant still holds stdout open. + expect(lookup.pollUnsafe()).toBeUndefined(); + yield* TestClock.adjust("5 seconds"); + expect(yield* Fiber.join(lookup)).toBeUndefined(); }).pipe(Effect.scoped), ); @@ -957,21 +991,20 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { const xcrun = path.join(directory, "xcrun"); yield* fs.writeFileString(xcrun, "#!/bin/sh\ntrap '' TERM\nexec sleep 600\n"); yield* fs.chmod(xcrun, 0o755); - const lookup = locateXcodeGitWithXcrun( - { PATH: "/usr/bin:/bin" }, - { xcrun, timeoutMs: 200 }, + const lookup = yield* locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun }).pipe( + Effect.forkChild({ startImmediately: true }), ); const child = nativeProbe.spawn.mock.results.at(-1) ?.value as NodeChildProcess.ChildProcess; const kill = vi.spyOn(child, "kill"); - const exited = new Promise((resolve) => - child.once("exit", (_code, signal) => resolve(signal)), - ); + const exited = exitOf(child); yield* Effect.addFinalizer(() => Effect.sync(() => child.kill("SIGKILL"))); - expect(yield* Effect.promise(() => lookup)).toBeUndefined(); + expect(lookup.pollUnsafe()).toBeUndefined(); + yield* TestClock.adjust("5 seconds"); + expect(yield* Fiber.join(lookup)).toBeUndefined(); expect(kill).toHaveBeenCalledWith("SIGKILL"); - expect(yield* Effect.promise(() => exited)).toBe("SIGKILL"); + expect(yield* Effect.promise(() => exited)).toEqual({ code: null, signal: "SIGKILL" }); }).pipe(Effect.scoped), ); @@ -997,18 +1030,27 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); - it.effect("looks again when the resolved git disappears", () => - Effect.gen(function* () { - const betaGit = "/Applications/Xcode-beta.app/Contents/Developer/usr/bin/git"; - const { files, host, resolve } = setup({ gitOnPath: "/usr/bin/git", xcrunGit: XCODE_GIT }); - expect(yield* resolve).toBe(XCODE_GIT); - - files.delete(XCODE_GIT); - files.add(betaGit); - host.xcrunGit = betaGit; - expect(yield* resolve).toBe(betaGit); - expect(host.xcrunCalls).toBe(2); - }).pipe(Effect.provideService(CommandResolutionCache, new Map())), + it.effect.each([undefined, 3] as const)( + "looks again when the resolved git disappears (yield budget %s)", + (budget) => + Effect.gen(function* () { + const betaGit = "/Applications/Xcode-beta.app/Contents/Developer/usr/bin/git"; + const { files, host, resolve } = setup({ + gitOnPath: "/usr/bin/git", + xcrunGit: XCODE_GIT, + }); + const budgeted = + budget === undefined + ? resolve + : resolve.pipe(Effect.provideService(Scheduler.MaxOpsBeforeYield, budget)); + expect(yield* budgeted).toBe(XCODE_GIT); + + files.delete(XCODE_GIT); + files.add(betaGit); + host.xcrunGit = betaGit; + expect(yield* budgeted).toBe(betaGit); + expect(host.xcrunCalls).toBe(2); + }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); }); diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 54038a22f45ab..aaf30870c6240 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -6,7 +6,9 @@ import * as NodeFS from "node:fs"; import * as Cache from "effect/Cache"; import * as Clock from "effect/Clock"; import * as Duration from "effect/Duration"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; @@ -760,63 +762,63 @@ export function npmShimExecutable( return isFile(executable) ? executable : undefined; } -export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => Promise; +export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => Effect.Effect; const XCRUN_MAX_OUTPUT_BYTES = 1024 * 1024; +const XCRUN_DEADLINE = Duration.seconds(5); -// xcrun honors DEVELOPER_DIR and the `xcode-select` choice, as the shim does. -// It takes ~15 ms and reruns when the cached answer expires, so it must not block the event loop. -// The deadline also covers a descendant that keeps stdout open after xcrun exits. -export function locateXcodeGitWithXcrun( - env: NodeJS.ProcessEnv, - options: { readonly xcrun?: string; readonly timeoutMs?: number } = {}, -): Promise { - return new Promise((resolve) => { +// Stopping it kills xcrun with SIGKILL, which it cannot ignore, and stops reading a stdout +// that a descendant may still hold open after xcrun exits. +const runXcrunFindGit = (xcrun: string, env: NodeJS.ProcessEnv) => + Effect.callback>((resume) => { let child: NodeChildProcess.ChildProcess; try { - child = NodeChildProcess.spawn(options.xcrun ?? "/usr/bin/xcrun", ["--find", "git"], { + child = NodeChildProcess.spawn(xcrun, ["--find", "git"], { env, stdio: ["ignore", "pipe", "ignore"], }); } catch { - resolve(undefined); + resume(Effect.succeedNone); return; } - const settle = (located: string | undefined) => { - clearTimeout(deadline); - resolve(located); - }; - // An xcrun that ignores SIGTERM must neither outlive the lookup nor hold the server open. - const giveUp = () => { + const stop = () => { child.kill("SIGKILL"); child.unref(); child.stdout?.destroy(); - settle(undefined); }; - // @effect-diagnostics-next-line globalTimers:off - a Promise-based lookup outside any fiber. - const deadline = setTimeout(giveUp, options.timeoutMs ?? 5_000); - deadline.unref(); const chunks: Array = []; let bytes = 0; child.stdout?.on("data", (chunk: Buffer) => { bytes += chunk.byteLength; if (bytes > XCRUN_MAX_OUTPUT_BYTES) { - giveUp(); + stop(); + resume(Effect.succeedNone); return; } chunks.push(chunk); }); - child.once("error", () => settle(undefined)); + child.once("error", () => resume(Effect.succeedNone)); child.once("close", (code) => { - if (code !== 0) { - settle(undefined); - return; - } const located = Buffer.concat(chunks).toString("utf8").trim(); - settle(NodePath.isAbsolute(located) ? located : undefined); + resume( + code === 0 && NodePath.isAbsolute(located) + ? Effect.succeedSome(located) + : Effect.succeedNone, + ); }); + return Effect.sync(stop); }); -} + +// xcrun honors DEVELOPER_DIR and the `xcode-select` choice, as the shim does. +// It takes ~15 ms and reruns when the cached answer expires, so it must not block the event loop. +export const locateXcodeGitWithXcrun = ( + env: NodeJS.ProcessEnv, + options: { readonly xcrun?: string } = {}, +): Effect.Effect => + runXcrunFindGit(options.xcrun ?? "/usr/bin/xcrun", env).pipe( + Effect.timeoutOption(XCRUN_DEADLINE), + Effect.map((found) => Option.getOrUndefined(Option.flatten(found))), + ); export const XcodeGitLocation = Context.Reference( "@t3tools/shared/shell/XcodeGitLocation", @@ -828,22 +830,24 @@ const MACOS_GIT_SHIM = "/usr/bin/git"; // Callers that miss the cache together wait for one xcrun run instead of starting their own. const xcodeGitLookups = new WeakMap< Map, - Map> + Map> >(); -async function lookupXcodeGit( +const lookupXcodeGit = ( env: NodeJS.ProcessEnv, resolveExecutable: SpawnExecutableResolver, locateXcodeGit: XcodeGitLocator, -): Promise { - const located = - resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM - ? await locateXcodeGit(env).catch(() => undefined) - : undefined; - return located !== undefined && resolveExecutable(located, "darwin", env) !== undefined - ? located - : null; -} +): Effect.Effect => + resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM + ? locateXcodeGit(env).pipe( + Effect.map((located) => + located !== undefined && resolveExecutable(located, "darwin", env) !== undefined + ? located + : null, + ), + Effect.catchCause(() => Effect.succeed(null)), + ) + : Effect.succeed(null); /** * Skip the xcrun shim's per-spawn lookup only when git resolves to /usr/bin/git. @@ -873,24 +877,33 @@ const resolveMacosGit = Effect.fnUntraced(function* ( ) { return cached.resolvedPath ?? "git"; } - let lookups = xcodeGitLookups.get(cache); - if (lookups === undefined) { - lookups = new Map(); - xcodeGitLookups.set(cache, lookups); - } - let lookup = lookups.get(cacheKey); - if (lookup === undefined) { - const pending = lookups; - lookup = lookupXcodeGit(env, resolveExecutable, locateXcodeGit) - .then((realGit) => { - cacheCommandResolution(cache, cacheKey, realGit, nowNanos); - return realGit; - }) - .finally(() => pending.delete(cacheKey)); - pending.set(cacheKey, lookup); - } - const shared = lookup; - const realGit = yield* Effect.promise(() => shared); + const pending = xcodeGitLookups.get(cache) ?? new Map>(); + xcodeGitLookups.set(cache, pending); + // Registering and starting the lookup cannot be split by a yield or an interrupt, so callers + // find it as soon as it exists. Detached, a caller that stops waiting does not stop it. + const lookup = yield* Effect.uninterruptible( + Effect.suspend(() => { + const existing = pending.get(cacheKey); + if (existing !== undefined) return Effect.succeed(existing); + const shared = Deferred.makeUnsafe(); + pending.set(cacheKey, shared); + return lookupXcodeGit(env, resolveExecutable, locateXcodeGit).pipe( + Effect.tap((realGit) => + Effect.sync(() => cacheCommandResolution(cache, cacheKey, realGit, nowNanos)), + ), + Effect.onExit((exit) => + Effect.sync(() => { + if (pending.get(cacheKey) === shared) pending.delete(cacheKey); + }).pipe( + Effect.andThen(Deferred.done(shared, Exit.isSuccess(exit) ? exit : Exit.succeed(null))), + ), + ), + Effect.forkDetach, + Effect.as(shared), + ); + }), + ); + const realGit = yield* Deferred.await(lookup); return realGit ?? "git"; });