From 388d3f91a3ea660fc4852082f2773bc34e5eae20 Mon Sep 17 00:00:00 2001 From: Chris OHearn Date: Thu, 12 Nov 2026 22:31:29 -0700 Subject: [PATCH 1/2] fix(server): one in-flight temporary-folder probe per path, with a short result cache A probe on a dead network mount never settles and pins a libuv thread. A retry now joins the unanswered probe instead of starting another, and a settled answer is reused for 30 s. Co-Authored-By: Claude Sonnet 5.5 --- .../src/project/temporaryFolder.test.ts | 36 +++++++++++++- apps/server/src/project/temporaryFolder.ts | 47 ++++++++++++++++++- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/apps/server/src/project/temporaryFolder.test.ts b/apps/server/src/project/temporaryFolder.test.ts index 6b983de659f4d..c17f343a3e2dc 100644 --- a/apps/server/src/project/temporaryFolder.test.ts +++ b/apps/server/src/project/temporaryFolder.test.ts @@ -3,12 +3,17 @@ import * as NodeFS from "node:fs"; import * as NodeOS from "node:os"; import * as NodePath from "node:path"; -import { assert, it } from "@effect/vitest"; +import { assert, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as TestClock from "effect/testing/TestClock"; -import { isTemporaryFolder, isTemporaryPath, temporaryFolderWarning } from "./temporaryFolder.ts"; +import { + dedupeProbe, + isTemporaryFolder, + isTemporaryPath, + temporaryFolderWarning, +} from "./temporaryFolder.ts"; it.effect.each([ ["/tmp/review", true], @@ -70,3 +75,30 @@ it.effect("resolves symlinks, and ignores a persistent TMPDIR", () => assert.isTrue(yield* isTemporaryFolder(NodePath.join(sandbox, "missing"))); }).pipe(Effect.scoped), ); + +it("joins an unanswered probe for the same path and reuses a settled answer until it expires", async () => { + let clock = 0; + let calls = 0; + const answers = [new Promise(() => {})]; + const probe = dedupeProbe( + { + inspect: (path) => { + calls += 1; + return answers.shift() ?? Promise.resolve({ resolved: path, tmpfs: false }); + }, + }, + () => clock, + 1_000, + ); + // The first probe never settles, as on a dead mount: later calls join it, not add another. + probe.inspect("/mnt/dead/a"); + probe.inspect("/mnt/dead/a"); + expect(calls).toBe(1); + // A different path is its own probe, and its answer is reused until the TTL passes. + await probe.inspect("/home/a"); + await probe.inspect("/home/a"); + expect(calls).toBe(2); + clock = 1_001; + await probe.inspect("/home/a"); + expect(calls).toBe(3); +}); diff --git a/apps/server/src/project/temporaryFolder.ts b/apps/server/src/project/temporaryFolder.ts index 790f081af0464..bec6073f63310 100644 --- a/apps/server/src/project/temporaryFolder.ts +++ b/apps/server/src/project/temporaryFolder.ts @@ -55,7 +55,50 @@ const exists = (path: string) => () => false, ); -const nodeProbe: FolderProbe = { +const RESULT_TTL_MS = 30_000; +const MAX_CACHED_PATHS = 256; + +/** + * One probe per path at a time, and a settled answer reused for a short while. A probe on a dead + * network mount never settles and pins a libuv thread, so a retry must join it, not add another. + */ +export function dedupeProbe( + probe: FolderProbe, + now: () => number = Date.now, + resultTtlMs = RESULT_TTL_MS, +): FolderProbe { + const entries = new Map< + string, + { readonly answer: ReturnType; settledAt: number | null } + >(); + return { + inspect: (path) => { + const known = entries.get(path); + if ( + known !== undefined && + (known.settledAt === null || now() - known.settledAt < resultTtlMs) + ) { + return known.answer; + } + const entry = { answer: probe.inspect(path), settledAt: null as number | null }; + entries.delete(path); + entries.set(path, entry); + // Map order is insertion order, so the first key is the oldest. + if (entries.size > MAX_CACHED_PATHS) entries.delete(entries.keys().next().value!); + entry.answer.then( + () => { + entry.settledAt = now(); + }, + () => { + if (entries.get(path) === entry) entries.delete(path); + }, + ); + return entry.answer; + }, + }; +} + +const nodeProbe: FolderProbe = dedupeProbe({ inspect: async (path) => { // A missing folder is the usual case here, so inspect its nearest existing ancestor. let existing = path; @@ -72,7 +115,7 @@ const nodeProbe: FolderProbe = { return { resolved: path, tmpfs: false }; } }, -}; +}); /** * A folder the system clears at reboot: a tmpfs, or under a well-known temporary directory. From d08e392acb73ae6f04c31c46dd82bc5080d98dce Mon Sep 17 00:00:00 2001 From: Chris OHearn Date: Fri, 13 Nov 2026 02:21:09 -0700 Subject: [PATCH 2/2] fix(server): track unanswered temporary-folder probes apart from the result cache Eviction could forget an unanswered probe, and every retry added a reaction to its pending promise. A retry during an unanswered probe now gets no answer at once, which sends it to the name check. Co-Authored-By: Claude Sonnet 5.5 --- .../src/project/temporaryFolder.test.ts | 27 +++++++-- apps/server/src/project/temporaryFolder.ts | 55 ++++++++++--------- 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/apps/server/src/project/temporaryFolder.test.ts b/apps/server/src/project/temporaryFolder.test.ts index c17f343a3e2dc..9110681bde812 100644 --- a/apps/server/src/project/temporaryFolder.test.ts +++ b/apps/server/src/project/temporaryFolder.test.ts @@ -76,7 +76,7 @@ it.effect("resolves symlinks, and ignores a persistent TMPDIR", () => }).pipe(Effect.scoped), ); -it("joins an unanswered probe for the same path and reuses a settled answer until it expires", async () => { +it("sends a retry to the name while a probe is unanswered, and reuses a settled answer until it expires", async () => { let clock = 0; let calls = 0; const answers = [new Promise(() => {})]; @@ -90,15 +90,30 @@ it("joins an unanswered probe for the same path and reuses a settled answer unti () => clock, 1_000, ); - // The first probe never settles, as on a dead mount: later calls join it, not add another. - probe.inspect("/mnt/dead/a"); - probe.inspect("/mnt/dead/a"); + // The first probe never settles, as on a dead mount: later calls get no answer, not another probe. + void probe.inspect("/mnt/dead/a"); + expect(await probe.inspect("/mnt/dead/a")).toBeUndefined(); + expect(await probe.inspect("/mnt/dead/a")).toBeUndefined(); expect(calls).toBe(1); - // A different path is its own probe, and its answer is reused until the TTL passes. - await probe.inspect("/home/a"); + // Another path probes on its own, and its answer is reused until the TTL passes. + expect(await probe.inspect("/home/a")).toEqual({ resolved: "/home/a", tmpfs: false }); await probe.inspect("/home/a"); expect(calls).toBe(2); clock = 1_001; await probe.inspect("/home/a"); expect(calls).toBe(3); }); + +it("keeps an unanswered probe when many other paths fill the result cache", async () => { + let calls = 0; + const probe = dedupeProbe({ + inspect: (path) => { + calls += 1; + return path === "/mnt/dead" ? new Promise(() => {}) : Promise.resolve(undefined); + }, + }); + void probe.inspect("/mnt/dead"); + for (let index = 0; index < 300; index += 1) await probe.inspect(`/home/p${index}`); + expect(await probe.inspect("/mnt/dead")).toBeUndefined(); + expect(calls).toBe(301); +}); diff --git a/apps/server/src/project/temporaryFolder.ts b/apps/server/src/project/temporaryFolder.ts index bec6073f63310..3f4b8141f0845 100644 --- a/apps/server/src/project/temporaryFolder.ts +++ b/apps/server/src/project/temporaryFolder.ts @@ -43,10 +43,15 @@ export function isTemporaryPath(path: string): boolean { } export interface FolderProbe { - /** The path with symlinks resolved, and whether its filesystem is a tmpfs. */ - readonly inspect: ( - path: string, - ) => Promise<{ readonly resolved: string; readonly tmpfs: boolean }>; + /** + * The path with symlinks resolved, and whether its filesystem is a tmpfs. No answer means the + * caller goes by the name. + */ + readonly inspect: (path: string) => Promise; +} +interface FolderInspection { + readonly resolved: string; + readonly tmpfs: boolean; } const exists = (path: string) => @@ -59,41 +64,39 @@ const RESULT_TTL_MS = 30_000; const MAX_CACHED_PATHS = 256; /** - * One probe per path at a time, and a settled answer reused for a short while. A probe on a dead - * network mount never settles and pins a libuv thread, so a retry must join it, not add another. + * One probe per path at a time. A probe on a dead mount never settles and pins a libuv thread, so + * a retry during an unanswered probe gets no answer at once (the caller goes by name) and adds no + * reaction to it. Unanswered probes are kept apart from the bounded cache of settled answers. */ export function dedupeProbe( probe: FolderProbe, now: () => number = Date.now, resultTtlMs = RESULT_TTL_MS, ): FolderProbe { - const entries = new Map< - string, - { readonly answer: ReturnType; settledAt: number | null } - >(); + const unanswered = new Set(); + const settled = new Map(); return { inspect: (path) => { - const known = entries.get(path); - if ( - known !== undefined && - (known.settledAt === null || now() - known.settledAt < resultTtlMs) - ) { - return known.answer; + if (unanswered.has(path)) return Promise.resolve(undefined); + const known = settled.get(path); + if (known !== undefined && now() - known.at < resultTtlMs) { + return Promise.resolve(known.answer); } - const entry = { answer: probe.inspect(path), settledAt: null as number | null }; - entries.delete(path); - entries.set(path, entry); - // Map order is insertion order, so the first key is the oldest. - if (entries.size > MAX_CACHED_PATHS) entries.delete(entries.keys().next().value!); - entry.answer.then( - () => { - entry.settledAt = now(); + unanswered.add(path); + return probe.inspect(path).then( + (answer) => { + unanswered.delete(path); + settled.delete(path); + settled.set(path, { answer, at: now() }); + // Map order is insertion order, so the first key is the oldest. + if (settled.size > MAX_CACHED_PATHS) settled.delete(settled.keys().next().value!); + return answer; }, () => { - if (entries.get(path) === entry) entries.delete(path); + unanswered.delete(path); + return undefined; }, ); - return entry.answer; }, }; }