From 2c69918cfe5161a6025fd0b3c1d8535261a95825 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Fri, 30 Oct 2026 03:01:06 -0700 Subject: [PATCH 1/3] Reuse checker hosts across the loads of one compile Each npm-static fixpoint round loaded the program with new frontend services, so every load started new tsc-ts processes and parsed the libraries and unchanged package files again. The Node frontend now keeps one FrontendServices per compile, as the native frontend does, and program hosts serve up to four loads: a host its load closes goes back to an idle list, and the next load rebinds it to its own file system shadow and sends its first snapshot with invalidateAll, so the server reads every file again and reuses only identical text. Loads alive at once hold different hosts; timed hosts are never reused. SCRIPTC_TS7_HOST_LOADS=1 restores a new host per load. A rebound host answers as a new host for changed shadows, hidden files, dropped in-memory files and on-disk edits (unit test), npm builds emit identical IR with and without reuse (parity test), and pi's loads after the scout take 1-3 s less each, with identical coverage diagnostics. Co-Authored-By: Claude Opus 5.5 --- .../compiler/src/frontend/pipeline-native.ts | 3 +- .../compiler/src/frontend/program-node.ts | 28 ++++-- .../compiler/src/frontend/services.test.ts | 68 +++++++++++++- packages/compiler/src/frontend/services.ts | 53 ++++++++++- .../src/frontend/ts7/program-host.test.ts | 94 +++++++++++++++++++ .../compiler/src/frontend/ts7/program-host.ts | 58 +++++++++++- packages/compiler/src/index.ts | 35 ++++++- tests/harness/ts7-host-reuse-parity.test.ts | 71 ++++++++++++++ 8 files changed, 392 insertions(+), 18 deletions(-) create mode 100644 tests/harness/ts7-host-reuse-parity.test.ts diff --git a/packages/compiler/src/frontend/pipeline-native.ts b/packages/compiler/src/frontend/pipeline-native.ts index f2df526084..c6bf0f77fc 100644 --- a/packages/compiler/src/frontend/pipeline-native.ts +++ b/packages/compiler/src/frontend/pipeline-native.ts @@ -1,6 +1,6 @@ import { runFrontend, type Frontend, type FrontendOptions } from "./pipeline.js"; import { loadProgram } from "./program.js"; -import { FrontendServices, type ComptimeEvaluator } from "./services.js"; +import { FrontendServices, programHostLoads, type ComptimeEvaluator } from "./services.js"; import { createNativeTs7Api } from "./ts7/native-api.js"; /** Native callers supply the installed TS7 executable explicitly. The @@ -22,6 +22,7 @@ export function runNativeFrontend( undefined, ); try { + services.reuseProgramHosts(programHostLoads()); const frontend = runFrontend( entryPath, (path, options) => diff --git a/packages/compiler/src/frontend/program-node.ts b/packages/compiler/src/frontend/program-node.ts index 8a45f92ee4..60d23efd8c 100644 --- a/packages/compiler/src/frontend/program-node.ts +++ b/packages/compiler/src/frontend/program-node.ts @@ -7,6 +7,7 @@ import { } from "./program.js"; import { createNodeFrontendServices } from "./services-node.js"; import type { Ts7Host } from "./ts7/program-host.js"; +import type { FrontendServices } from "./services.js"; export * from "./program.js"; export interface NodeProgramLoadOptions { @@ -14,19 +15,32 @@ export interface NodeProgramLoadOptions { externalTypes?: ReadonlyMap | Readonly> | undefined; } +function serviceOptions(options: NodeProgramLoadOptions | undefined) { + return { + npmStatic: [...(options?.npmStatic ?? [])], + externalTypes: + options?.externalTypes instanceof Map + ? [...options.externalTypes] + : Object.entries(options?.externalTypes ?? {}), + }; +} + +/** A load on services the caller owns (and closes). */ +export function loadProgramWithServices( + entryPath: string, + services: FrontendServices, + options?: NodeProgramLoadOptions, +): ReturnType { + return loadWithServices(entryPath, services, serviceOptions(options)); +} + export function loadProgram( entryPath: string, options?: NodeProgramLoadOptions, ): ReturnType { const services = createNodeFrontendServices(); try { - const load = loadWithServices(entryPath, services, { - npmStatic: [...(options?.npmStatic ?? [])], - externalTypes: - options?.externalTypes instanceof Map - ? [...options.externalTypes] - : Object.entries(options?.externalTypes ?? {}), - }); + const load = loadWithServices(entryPath, services, serviceOptions(options)); return { ...load, dispose: () => { diff --git a/packages/compiler/src/frontend/services.test.ts b/packages/compiler/src/frontend/services.test.ts index e491b0ac5b..e80565c31a 100644 --- a/packages/compiler/src/frontend/services.test.ts +++ b/packages/compiler/src/frontend/services.test.ts @@ -2,7 +2,12 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { afterEach, expect, test, vi } from "vitest"; -import { FrontendServices, type ComptimeEvaluator } from "./services.js"; +import { + FrontendServices, + PROGRAM_HOST_LOADS, + programHostLoads, + type ComptimeEvaluator, +} from "./services.js"; import { Ts7Api } from "./ts7/rpc-api.js"; import { checkPreflight, isNodeEsmFile, loadProgram } from "./program.js"; import { clearResolveCaches } from "./resolve.js"; @@ -168,3 +173,64 @@ test("Node format decisions belong to their program even when source ASTs are re } expect(first.analysis.nodeEsmFiles.size).toBe(0); }); + +test("reused program hosts serve later loads, up to their share, one connection per live host", () => { + const { services, connections, dir } = setup(); + services.reuseProgramHosts(3); + const entry = join(dir, "main.ts"); + writeFileSync(entry, "export const value = 1;"); + try { + // Two loads alive at once hold two hosts; after they close, the next + // loads reuse those connections. + const a = loadProgram(entry, services); + const b = loadProgram(entry, services); + expect(connections).toHaveLength(2); + a.dispose(); + b.dispose(); + for (let load = 0; load < 5; load++) { + const next = loadProgram(entry, services); + expect(checkPreflight(next)).toEqual([]); + next.dispose(); + } + // Each host served 3 loads (the share), then closed: a third host + // started for the last load. + expect(connections).toHaveLength(3); + expect(connections[0]!.close).toHaveBeenCalledTimes(1); + expect(connections[1]!.close).toHaveBeenCalledTimes(1); + expect(connections[2]!.close).not.toHaveBeenCalled(); + } finally { + services.close(); + } + expect(connections[2]!.close).toHaveBeenCalledTimes(1); +}); + +test("without reuse, and for timed hosts, every load gets its own host", () => { + const { services, connections } = setup(); + try { + services.createProgramHost().close(); + services.createProgramHost().close(); + expect(connections).toHaveLength(2); + services.reuseProgramHosts(4); + services.createProgramHost({ collectTiming: true }).close(); + services.createProgramHost({ collectTiming: true }).close(); + expect(connections).toHaveLength(4); + for (const connection of connections) expect(connection.close).toHaveBeenCalledTimes(1); + // A host in a different directory is not reused for this one. + services.createProgramHost({ cwd: tmpdir() }).close(); + services.createProgramHost().close(); + expect(connections).toHaveLength(6); + } finally { + services.close(); + } + for (const connection of connections) expect(connection.close).toHaveBeenCalledTimes(1); +}); + +test("SCRIPTC_TS7_HOST_LOADS selects the share and rejects malformed values", () => { + expect(programHostLoads({})).toBe(PROGRAM_HOST_LOADS); + expect(programHostLoads({ SCRIPTC_TS7_HOST_LOADS: "1" })).toBe(1); + expect(programHostLoads({ SCRIPTC_TS7_HOST_LOADS: "12" })).toBe(12); + for (const value of ["0", "-1", "2.5", "many"]) + expect(() => programHostLoads({ SCRIPTC_TS7_HOST_LOADS: value })).toThrow( + "SCRIPTC_TS7_HOST_LOADS must be a positive integer", + ); +}); diff --git a/packages/compiler/src/frontend/services.ts b/packages/compiler/src/frontend/services.ts index 5e493f0baa..ce5e88ed16 100644 --- a/packages/compiler/src/frontend/services.ts +++ b/packages/compiler/src/frontend/services.ts @@ -42,6 +42,25 @@ export interface RuntimeModuleResolver { lookupPaths: (fromFile: string, specifier: string) => readonly string[] | null; } +/** How many loads of one compile a program host serves before a new one + * replaces it. tsc-ts keeps every syntax tree it builds for the life of its + * process, so a host grows with each load whose files differ from the last + * one's (on pi, by 200-400 MB a load); a new one starts empty. */ +export const PROGRAM_HOST_LOADS = 4; + +/** PROGRAM_HOST_LOADS, or SCRIPTC_TS7_HOST_LOADS (1: a new host per load, + * the comparison the host reuse parity test runs). */ +export function programHostLoads(env: NodeJS.ProcessEnv = process.env): number { + const value = env["SCRIPTC_TS7_HOST_LOADS"]; + if (value === undefined || value === "") return PROGRAM_HOST_LOADS; + const loads = Number(value); + if (!Number.isSafeInteger(loads) || loads < 1) + throw new Error( + `SCRIPTC_TS7_HOST_LOADS must be a positive integer, not ${JSON.stringify(value)}`, + ); + return loads; +} + /** Own the syntax and semantic services for a compiler client. The supplied * connection factory and optional evaluator provide host operations; * parsing, projection and filesystem resolution share their implementation @@ -49,6 +68,10 @@ export interface RuntimeModuleResolver { export class FrontendServices { private parser: Ts7SourceParser | undefined; private closed = false; + /** How many loads one program host serves (reuseProgramHosts). */ + private hostLoads = 1; + /** Reusable program hosts that no load holds. */ + private readonly idleHosts: Ts7Host[] = []; constructor( private readonly createApi: Ts7ApiFactory, @@ -81,9 +104,29 @@ export class FrontendServices { throw new Error("this compiler host does not provide compile-time evaluation"); return evaluate(source, timeoutMs); } + /** Let every program host serve up to `loads` loads (reuseProgramHosts). */ + reuseProgramHosts(loads: number): void { + this.ensureOpen(); + this.hostLoads = loads > 1 ? loads : 1; + } createProgramHost(options?: Ts7HostOptions): Ts7Host { this.ensureOpen(); - return new Ts7Host(this.createApi, { ...options, cwd: options?.cwd ?? this.cwd }); + const cwd = options?.cwd ?? this.cwd; + // A timed host reports its own load's timing, so it is never reused. + if (this.hostLoads === 1 || options?.collectTiming === true) + return new Ts7Host(this.createApi, { ...options, cwd }); + const index = this.idleHosts.findIndex((host) => host.cwd === cwd); + if (index >= 0) { + const host = this.idleHosts.splice(index, 1)[0]!; + host.rebind(options?.fsShadow); + return host; + } + return new Ts7Host(this.createApi, { ...options, cwd }, (released) => { + // The host's load closed it: keep its connection for the next load, + // unless it has served its share or these services are closed. + if (this.closed || released.loads >= this.hostLoads) released.closeConnection(); + else this.idleHosts.push(released); + }); } rewriteCjs( source: string, @@ -214,6 +257,12 @@ export class FrontendServices { this.closed = true; const parser = this.parser; this.parser = undefined; - parser?.close(); + // A host a load still holds closes when that load closes it. + const idle = this.idleHosts.splice(0); + try { + for (const host of idle) host.closeConnection(); + } finally { + parser?.close(); + } } } diff --git a/packages/compiler/src/frontend/ts7/program-host.test.ts b/packages/compiler/src/frontend/ts7/program-host.test.ts index 2ab6a0293f..5003353feb 100644 --- a/packages/compiler/src/frontend/ts7/program-host.test.ts +++ b/packages/compiler/src/frontend/ts7/program-host.test.ts @@ -333,3 +333,97 @@ test("closed hosts refuse every operation and release virtual contents", () => { expect(() => host.getTimingInfo()).toThrow("closed"); expect(() => host.createProgram([entry], options)).toThrow("closed"); }); + +/** What a program answers that a reused host could answer stale: its file + * list, each file's text, and its diagnostics. */ +function answers(program: ReturnType) { + const names = [...program.getSourceFileNames()]; + return { + names, + texts: names.map((name) => program.getSourceFile(name)?.text), + diagnostics: getPreEmitDiagnostics(program), + }; +} + +test("a rebound host answers as a new host when shadows, disk files and in-memory files change", () => { + const { directory, entry } = fixture(); + const dep = join(directory, "dep.ts"); + const extra = join(directory, "extra.d.ts"); + const virtual = join(directory, "virtual.ts"); + writeFileSync( + entry, + '/// \nimport { value } from "./dep.js";\nimport { v } from "./virtual.js";\nexport const sum: number = value + extra + v;\n', + ); + writeFileSync(dep, "export const value: number = 1;\n"); + writeFileSync(extra, "declare const extra: number;\n"); + // Load 1: dep.ts reads as a string-typed replacement, extra.d.ts is + // hidden, and virtual.ts exists in memory only. + const shadow = { + readFile: (path: string) => + tsgoPath(path) === tsgoPath(dep) ? 'export const value: string = "1";\n' : undefined, + hideFile: (path: string) => tsgoPath(path) === tsgoPath(extra), + }; + const factory = vi.fn((connection: Ts7ApiOptions) => new Ts7Api(connection)); + const reused = new Ts7Host(factory, { cwd: directory, fsShadow: shadow }, () => {}); + hosts.push(reused); + reused.addVirtualFile(virtual, "export const v = 2;\n"); + const first = reused.createProgram([entry], options); + const firstAnswers = answers(first); + first.dispose(); + expect(firstAnswers.diagnostics.map((d) => d.code).sort()).toEqual([2304, 2322, 6053]); + // Load 2: no shadow, no in-memory file, and dep.ts changed on disk + // without a change notification. + writeFileSync(dep, "export const value: boolean = true;\n"); + reused.rebind(null); + const second = reused.createProgram([entry], options); + const fresh = makeHost(directory).host.createProgram([entry], options); + try { + expect(factory).toHaveBeenCalledTimes(1); + expect(answers(second)).toEqual(answers(fresh)); + expect( + answers(second) + .diagnostics.map((d) => d.code) + .sort(), + ).toEqual([2307, 2365]); + } finally { + second.dispose(); + fresh.dispose(); + } + // Load 3: the first load's shadow again answers as it did. + writeFileSync(dep, "export const value: number = 1;\n"); + reused.rebind(shadow); + reused.addVirtualFile(virtual, "export const v = 2;\n"); + const third = reused.createProgram([entry], options); + try { + expect(answers(third)).toEqual(firstAnswers); + } finally { + third.dispose(); + } +}); + +test("a reusable host returns to its owner on close and closes its connection only once", () => { + const { directory, entry } = fixture(); + const released: Ts7Host[] = []; + let api!: Ts7Api; + const host = new Ts7Host( + (connection) => (api = new Ts7Api(connection)), + { cwd: directory }, + (h) => released.push(h), + ); + hosts.push(host); + const program = host.createProgram([entry], options); + program.dispose(); + const close = vi.spyOn(api, "close"); + host.close(); + host.close(); + expect(released).toEqual([host]); + expect(close).not.toHaveBeenCalled(); + // Still open: the next load rebinds it. + host.rebind(null); + expect(host.loads).toBe(2); + host.createProgram([entry], options).dispose(); + host.closeConnection(); + host.close(); + expect(close).toHaveBeenCalledTimes(1); + expect(() => host.createProgram([entry], options)).toThrow("closed"); +}); diff --git a/packages/compiler/src/frontend/ts7/program-host.ts b/packages/compiler/src/frontend/ts7/program-host.ts index 1b6b7b1035..a61fcffa44 100644 --- a/packages/compiler/src/frontend/ts7/program-host.ts +++ b/packages/compiler/src/frontend/ts7/program-host.ts @@ -166,16 +166,37 @@ export class Ts7Host { /** The text the server reads for a file (its readFile hook). */ private readonly readSource: (fileName: string) => string | null; private closed = false; + /** The file system shadow of the load that holds this host (rebind). */ + private shadow: NonNullable | null; + /** Whether the next snapshot drops every file the server read: set when + * the host passes to another load, whose files may read differently. */ + private invalidateAll = false; + /** The working directory of the server connection. */ + readonly cwd: string; + /** The loads this host has served: 1, and one more for each rebind. */ + loads = 1; + /** Called instead of closing the connection when the host is reusable. */ + private release: ((host: Ts7Host) => void) | undefined; + /** Whether a load holds this reusable host: from its creation or rebind + * until that load closes it. */ + private held = true; - constructor(createApi: Ts7ApiFactory, options?: Ts7HostOptions) { + constructor( + createApi: Ts7ApiFactory, + options?: Ts7HostOptions, + release?: (host: Ts7Host) => void, + ) { const virtualFiles = this.virtualFiles; - const shadow = options?.fsShadow ?? null; + this.shadow = options?.fsShadow ?? null; + this.cwd = options?.cwd ?? process.cwd(); + this.release = release; // A virtual file, a shadow's replacement, a package shim's declaration // entry, a type-patched package file or the file on disk; null when shadowed out of existence or // missing. this.readSource = (fileName) => { const virtual = virtualFiles.get(tsgoPath(fileName)); if (virtual !== undefined) return stripSourceBom(virtual); + const shadow = this.shadow; if (shadow !== null) { if (shadow.hideFile(fileName)) return null; const replacement = shadow.readFile(fileName); @@ -199,7 +220,7 @@ export class Ts7Host { readFile: this.readSource, fileExists: (fileName) => { if (virtualFiles.has(tsgoPath(fileName))) return true; - if (shadow !== null && shadow.hideFile(fileName)) return false; + if (this.shadow !== null && this.shadow.hideFile(fileName)) return false; return trackedFileExists(fileName) || typePatchFileExists(fileName); }, directoryExists: (path) => trackedDirectoryExists(path), @@ -270,8 +291,9 @@ export class Ts7Host { const snapshot = this.api.updateSnapshot({ openProjects: [configPath], closeProjects, - fileChanges: { created, changed }, + fileChanges: this.invalidateAll ? { invalidateAll: true } : { created, changed }, }); + this.invalidateAll = false; this.closingProjects.clear(); this.virtualChanges.clear(); const project = snapshot.getProject(configPath); @@ -303,8 +325,36 @@ export class Ts7Host { if (this.closed) throw new InternalCompilerError("Ts7Host is closed"); } + /** Pass this pooled host to another load: its shadow replaces the last + * load's, the last load's in-memory files are gone, and the next snapshot + * reads every file again (the server reuses a file's syntax tree only for + * identical text). */ + rebind(fsShadow: Ts7HostOptions["fsShadow"]): void { + this.ensureOpen(); + this.held = true; + this.loads++; + this.shadow = fsShadow ?? null; + this.virtualFiles.clear(); + this.virtualChanges.clear(); + this.invalidateAll = true; + } + + /** Close the server connection, even when the host is pooled. */ + closeConnection(): void { + this.release = undefined; + this.close(); + } + close(): void { if (this.closed) return; + const release = this.release; + if (release !== undefined) { + // A second close by the same load returns nothing twice. + if (!this.held) return; + this.held = false; + release(this); + return; + } this.closed = true; try { this.api.close(); diff --git a/packages/compiler/src/index.ts b/packages/compiler/src/index.ts index ca9211c121..cb991759af 100644 --- a/packages/compiler/src/index.ts +++ b/packages/compiler/src/index.ts @@ -1,7 +1,7 @@ import { createRequire } from "node:module"; import { setShapeOverlayMemoDir, shapeOverlayEnabled } from "./frontend/shape-inference/index.js"; import { jsInferenceEnabled } from "./frontend/js-inference.js"; -import { nodeFrontendServices } from "./frontend/services-node.js"; +import { createNodeFrontendServices, nodeFrontendServices } from "./frontend/services-node.js"; import { runTypePatchCommand, type TypePatchCommandRequest, @@ -173,7 +173,8 @@ import { serializeModuleParts } from "./ir/serialize.js"; import { readTextParts, writeTextParts } from "./text-parts.js"; import { validateModule } from "./ir/validate.js"; import { compilerOwnedFileRoots } from "./frontend/dts-paths.js"; -import { loadProgram } from "./frontend/program-node.js"; +import { loadProgramWithServices } from "./frontend/program-node.js"; +import { programHostLoads } from "./frontend/services.js"; import { runFrontend, type FrontendFactory } from "./frontend/pipeline.js"; import { type ProvenanceSources, @@ -485,7 +486,35 @@ const nodeFrontend: FrontendFactory = ( externalTypes, libraryNpmStatic, options, -) => runFrontend(entry, loadProgram, npmStatic, externalTypes, libraryNpmStatic, options); +) => { + // One syntax parser for every load of this compile, and program hosts + // that serve several loads (FrontendServices.reuseProgramHosts). + const services = createNodeFrontendServices(); + try { + services.reuseProgramHosts(programHostLoads()); + const frontend = runFrontend( + entry, + (path, loadOptions) => loadProgramWithServices(path, services, loadOptions), + npmStatic, + externalTypes, + libraryNpmStatic, + options, + ); + return { + ...frontend, + dispose: () => { + try { + frontend.dispose(); + } finally { + services.close(); + } + }, + }; + } catch (error) { + services.close(); + throw error; + } +}; /** The checker's part of the executable cache key (`checkerCacheIdentity`), as the CLI's startup route computes it. */ function checkerField(): { checker?: string } { diff --git a/tests/harness/ts7-host-reuse-parity.test.ts b/tests/harness/ts7-host-reuse-parity.test.ts new file mode 100644 index 0000000000..22284f4266 --- /dev/null +++ b/tests/harness/ts7-host-reuse-parity.test.ts @@ -0,0 +1,71 @@ +/* Program host reuse (FrontendServices.reuseProgramHosts) must not change + * a compile: a build whose loads share program hosts emits the same IR, + * the same diagnostics and the same exit status as a build with a new host + * for every load (SCRIPTC_TS7_HOST_LOADS=1). Share 2 replaces each host after + * two loads, so a fixpoint of several loads exercises both a rebind and a + * replacement. The npm cases are the frontend's multi-load builds: each + * imports packages, so its build loads the program at least twice. */ +import { execFile } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { promisify } from "node:util"; +import { afterAll, beforeEach, describe, expect, test } from "vitest"; +import { npmCases } from "./npm-cases.js"; +import { skipUnownedTest } from "./shard.js"; + +const execFileAsync = promisify(execFile); +const repoRoot = join(import.meta.dirname, "../.."); +const cli = join(repoRoot, "packages/cli/dist/bootstrap.js"); +const out = mkdtempSync(join(tmpdir(), "scriptc-host-reuse-")); +afterAll(() => rmSync(out, { recursive: true, force: true })); + +interface Build { + ir: string | null; + stderr: string; + status: number; +} + +async function build(entry: string, name: string, loads: string): Promise { + const ir = join(out, `${name}.${loads}.ir.json`); + const env = { + ...process.env, + SCRIPTC_TS7_HOST_LOADS: loads, + SCRIPTC_CACHE_DIR: join(out, `cache-${loads}`), + }; + try { + const { stderr } = await execFileAsync( + process.execPath, + ["--max-old-space-size=8192", cli, "build", entry, "--emit=ir", "-o", ir], + { env, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }, + ); + return { ir: readFileSync(ir, "utf8"), stderr, status: 0 }; + } catch (error) { + const e = error as { code?: unknown; stderr?: string }; + if (typeof e.code !== "number") throw error; + return { ir: null, stderr: e.stderr ?? "", status: e.code }; + } +} + +// Every sixth case by name (all of them with SCRIPTC_HOST_REUSE_PARITY_ALL=1). +const all = process.env["SCRIPTC_HOST_REUSE_PARITY_ALL"] === "1"; +const cases = npmCases(join(repoRoot, "tests/fixtures")) + .sort((a, b) => a.name.localeCompare(b.name)) + .filter((_, index) => all || index % 6 === 0); + +describe("program host reuse parity", () => { + beforeEach((context) => skipUnownedTest(context, {})); + for (const { name, entry } of cases) { + test( + name, + async () => { + const fresh = await build(entry, name, "1"); + const reused = await build(entry, name, "2"); + expect(reused.status).toBe(fresh.status); + expect(reused.stderr).toBe(fresh.stderr); + expect(reused.ir === fresh.ir).toBe(true); + }, + 600_000, + ); + } +}); From 3bc9ac827951be9304f7ad75d1adbc07e708c5ed Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Fri, 30 Oct 2026 03:01:06 -0700 Subject: [PATCH 2/3] Start POSIX checker servers outside this process's children Node reaps children only from its event loop, which a synchronous compile never returns to, so every replaced or closed tsc-ts process stayed a zombie until the compile ended: 797 on a pi build, most of them syntax parsers replaced every 256 K of source. On Linux and macOS a shell that spawnSync runs and reaps now creates two FIFOs, starts the server in the background on them and exits; init reaps the server. The compiler opens the FIFOs with openSync, as it already opens the Windows named pipe, and closing them ends the session: the server reads end of file and exits, and is never signaled. This also removes the dependency on Node's private stream handles. Windows is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/ts7/rpc-process.test.ts | 76 +++-- .../compiler/src/frontend/ts7/rpc-process.ts | 278 ++++++++++++------ 2 files changed, 235 insertions(+), 119 deletions(-) diff --git a/packages/compiler/src/frontend/ts7/rpc-process.test.ts b/packages/compiler/src/frontend/ts7/rpc-process.test.ts index faad0cc028..e91b0b8200 100644 --- a/packages/compiler/src/frontend/ts7/rpc-process.test.ts +++ b/packages/compiler/src/frontend/ts7/rpc-process.test.ts @@ -1,37 +1,69 @@ -import { execFile } from "node:child_process"; +import { execFile, execFileSync } from "node:child_process"; import { promisify } from "node:util"; import { expect, test } from "vitest"; +import { Ts7FifoIo } from "./rpc-process.js"; const execFileAsync = promisify(execFile); +const posix = process.platform !== "win32"; -test.skipIf(process.platform === "win32")( - "closing the owned server is quiet even when its cancellation handler writes stderr", +function alive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + +test.skipIf(!posix)("a FIFO server echoes, and exits when its transport closes", () => { + const io = new Ts7FifoIo("/bin/cat", []); + const bytes = Buffer.from("ping\n"); + expect(io.write(bytes, 0, bytes.length)).toBe(bytes.length); + const reply = Buffer.alloc(16); + expect(reply.subarray(0, io.read(reply, 0, reply.length)).toString()).toBe("ping\n"); + io.close(); + io.close(); + expect(() => io.read(reply, 0, 1)).toThrow("closed"); + // The server is not this process's child: init reaps it once it exits. + const deadline = Date.now() + 5000; + while (alive(io.pid) && Date.now() < deadline) execFileSync("sleep", ["0.01"]); + expect(alive(io.pid)).toBe(false); +}); + +test.skipIf(!posix)("a FIFO server that cannot start fails its first read", () => { + const io = new Ts7FifoIo("/nonexistent/scriptc-ts7-server", []); + try { + expect(io.read(Buffer.alloc(1), 0, 1)).toBe(0); + } finally { + io.close(); + } +}); + +test.skipIf(!posix)( + "replaced and closed checker servers leave no zombie children behind", async () => { - // A ready message ensures the signal handler is installed before close. - // Keep stdin idle, matching an API server waiting for its next request. - const server = ` - process.on('SIGTERM', () => { - process.stderr.write('context canceled\\n'); - process.exit(0); - }); - process.stdout.write(Buffer.from([0x93, 4, 0xc4, 0, 0xc4, 0])); - setInterval(() => {}, 1000); - `; - const moduleUrl = new URL("./rpc-process.ts", import.meta.url).href; + // Source budget 1: every parse after the first replaces the server. + const parserUrl = new URL("./source-parser.ts", import.meta.url).href; + const apiUrl = new URL("./rpc-api.ts", import.meta.url).href; const host = ` - import { spawnTs7Wire } from ${JSON.stringify(moduleUrl)}; - const wire = spawnTs7Wire(process.execPath, ['--eval', ${JSON.stringify(server)}]); - const ready = wire.read(); - if (ready.kind !== 4) throw new Error('server was not ready'); - wire.close(); - wire.close(); + import { execFileSync } from 'node:child_process'; + import { Ts7SourceParser } from ${JSON.stringify(parserUrl)}; + import { Ts7Api } from ${JSON.stringify(apiUrl)}; + const parser = new Ts7SourceParser((options) => new Ts7Api(options), process.cwd(), 1); + for (let n = 0; n < 12; n++) parser.parse('f' + n + '.ts', 'export const v' + n + ' = ' + n + ';', 'ts'); + parser.close(); + const zombies = execFileSync('ps', ['-A', '-o', 'ppid=,stat='], { encoding: 'utf8' }) + .split('\\n') + .map((line) => line.trim().split(/\\s+/)) + .filter(([ppid, stat]) => Number(ppid) === process.pid && (stat ?? '').startsWith('Z')); + console.log(JSON.stringify({ zombies: zombies.length })); `; const result = await execFileAsync( process.execPath, ["--import", "tsx", "--input-type=module", "--eval", host], - { timeout: 10_000 }, + { timeout: 60_000 }, ); - expect(result.stdout).toBe(""); + expect(JSON.parse(result.stdout)).toEqual({ zombies: 0 }); expect(result.stderr).toBe(""); }, ); diff --git a/packages/compiler/src/frontend/ts7/rpc-process.ts b/packages/compiler/src/frontend/ts7/rpc-process.ts index 416f5e4f75..24d8f5a99a 100644 --- a/packages/compiler/src/frontend/ts7/rpc-process.ts +++ b/packages/compiler/src/frontend/ts7/rpc-process.ts @@ -1,98 +1,96 @@ -import { spawn, type ChildProcess } from "node:child_process"; +import { spawn, spawnSync, type ChildProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; -import { closeSync, openSync, readSync, writeSync } from "node:fs"; -import type { Readable, Writable } from "node:stream"; +import { closeSync, constants, mkdtempSync, openSync, readSync, rmSync, writeSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { Ts7Wire, type Ts7WireIo } from "./rpc-wire.js"; -// Node's public spawn API does not expose synchronous pipe descriptors. -// Keep this host-specific boundary out of the statically compiled protocol. -// Windows uses a named pipe because libuv's Windows handles are not CRT fds. -interface PipeHandle { - fd: number; - setBlocking: (blocking: boolean) => void; -} -type PipeStream = (Readable | Writable) & { - _handle?: PipeHandle; - unref: () => void; -}; +// Node's public spawn API does not expose synchronous pipe descriptors, so +// the server's end of the channel is a named pipe on Windows and a FIFO pair +// elsewhere, both opened with openSync. Keep this host-specific boundary out +// of the statically compiled protocol. -const liveTransports = new Set(); +const liveTransports = new Set(); const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)); function closeLiveTransports(): void { for (const transport of liveTransports) transport.close(); } -/** Own the child and its descriptors together: every failure path closes - * both, and the exit hook exists only while at least one host is alive. */ -class Ts7ProcessIo implements Ts7WireIo { +function track(transport: Ts7WireIo): void { + if (liveTransports.size === 0) process.on("exit", closeLiveTransports); + liveTransports.add(transport); +} + +function untrack(transport: Ts7WireIo): void { + liveTransports.delete(transport); + if (liveTransports.size === 0) process.removeListener("exit", closeLiveTransports); +} + +/** A synchronous read or write that retries an interrupted call and waits + * out a full pipe. */ +function transfer( + operation: ( + fd: number, + buffer: Uint8Array, + offset: number, + length: number, + position: null, + ) => number, + fd: number, + buffer: Uint8Array, + offset: number, + length: number, +): number { + for (;;) { + try { + return operation(fd, buffer, offset, length, null); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "EINTR") continue; + if (code !== "EAGAIN") throw error; + Atomics.wait(sleepBuffer, 0, 0, 1); + } + } +} + +/** Windows: the server is a child process that creates a named pipe + * (`--pipe`), which this process opens. Windows has no zombie processes; the + * child's handle closes with the process. */ +class Ts7NamedPipeIo implements Ts7WireIo { private readonly child: ChildProcess; - private readFd = -1; - private writeFd = -1; private pipeFd = -1; private closed = false; private spawnError: Error | undefined; constructor(executable: string, args: string[]) { - const windows = process.platform === "win32"; const pipe = `\\\\.\\pipe\\scriptc-ts7-${process.pid}-${randomUUID()}`; - this.child = spawn(executable, windows ? [...args, "--pipe", pipe] : args, { - stdio: windows ? ["ignore", "ignore", "inherit"] : ["pipe", "pipe", "inherit"], + this.child = spawn(executable, [...args, "--pipe", pipe], { + stdio: ["ignore", "ignore", "inherit"], windowsHide: true, }); - // A failed spawn emits asynchronously even when descriptor validation - // below has already thrown. Always consume that event. + // A failed spawn emits asynchronously even when the connection below has + // already thrown. Always consume that event. this.child.on("error", (error: Error) => { this.spawnError = error; }); try { if (this.child.pid === undefined) throw new Error(`Unable to start TypeScript server: ${executable}`); - if (windows) { - for (let attempt = 0; attempt < 500; attempt++) { - try { - this.pipeFd = openSync(pipe, "r+"); - break; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== "ENOENT" && code !== "EBUSY") throw error; - Atomics.wait(sleepBuffer, 0, 0, 10); - } + for (let attempt = 0; attempt < 500; attempt++) { + try { + this.pipeFd = openSync(pipe, "r+"); + break; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT" && code !== "EBUSY") throw error; + Atomics.wait(sleepBuffer, 0, 0, 10); } - if (this.pipeFd === -1) - throw new Error("TypeScript server: timed out connecting to named pipe"); - this.readFd = this.pipeFd; - this.writeFd = this.pipeFd; - } else { - const stdout = this.child.stdout as PipeStream | null; - const stdin = this.child.stdin as PipeStream | null; - const readHandle = stdout?._handle; - const writeHandle = stdin?._handle; - if ( - !stdout || - !stdin || - !readHandle || - !writeHandle || - !Number.isInteger(readHandle.fd) || - readHandle.fd < 0 || - !Number.isInteger(writeHandle.fd) || - writeHandle.fd < 0 || - typeof readHandle.setBlocking !== "function" || - typeof writeHandle.setBlocking !== "function" - ) { - throw new Error("TypeScript server: Node synchronous pipe handles are unavailable"); - } - readHandle.setBlocking(true); - writeHandle.setBlocking(true); - this.child.stdout!.pause(); - stdout.unref(); - stdin.unref(); - this.readFd = readHandle.fd; - this.writeFd = writeHandle.fd; } + if (this.pipeFd === -1) + throw new Error("TypeScript server: timed out connecting to named pipe"); this.child.unref(); - if (liveTransports.size === 0) process.on("exit", closeLiveTransports); - liveTransports.add(this); + track(this); } catch (error) { this.close(); throw error; @@ -102,52 +100,27 @@ class Ts7ProcessIo implements Ts7WireIo { read = (buffer: Uint8Array, offset: number, length: number): number => { if (this.spawnError) throw this.spawnError; if (this.closed) throw new Error("TypeScript server transport is closed"); - for (;;) { - try { - return readSync(this.readFd, buffer, offset, length, null); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === "EINTR") continue; - if (code !== "EAGAIN") throw error; - Atomics.wait(sleepBuffer, 0, 0, 1); - } - } + return transfer(readSync, this.pipeFd, buffer, offset, length); }; write = (buffer: Uint8Array, offset: number, length: number): number => { if (this.spawnError) throw this.spawnError; if (this.closed) throw new Error("TypeScript server transport is closed"); - for (;;) { - try { - return writeSync(this.writeFd, buffer, offset, length, null); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === "EINTR") continue; - if (code !== "EAGAIN") throw error; - Atomics.wait(sleepBuffer, 0, 0, 1); - } - } + return transfer(writeSync, this.pipeFd, buffer, offset, length); }; close = (): void => { if (this.closed) return; this.closed = true; - liveTransports.delete(this); - if (liveTransports.size === 0) process.removeListener("exit", closeLiveTransports); + untrack(this); if (this.pipeFd !== -1) { try { closeSync(this.pipeFd); } catch { - /* still close the child */ + /* still end the child */ } this.pipeFd = -1; } - this.readFd = -1; - this.writeFd = -1; - this.child.stdout?.destroy(); - this.child.stdin?.destroy(); - // A failed spawn can leave libuv's process handle without a PID. - // Never signal it: PID zero has process-group semantics on POSIX. // The channel is closed and all its work is discarded. SIGTERM invokes // tsgo's cancellation handler, which can race EOF and print "context // canceled" into an otherwise successful compile's stderr. @@ -155,6 +128,117 @@ class Ts7ProcessIo implements Ts7WireIo { }; } +/* POSIX: the server is not this process's child. A shell that spawnSync runs + * and reaps creates two FIFOs, starts the server in the background with its + * standard input and output on them, prints its PID and exits; the server's + * parent becomes init (or the nearest subreaper), which reaps it. A child of + * this process would stay a zombie after it exits: Node reaps children only + * from its event loop, which a synchronous compile never returns to. The + * subshell points its standard streams at /dev/null before it opens the + * FIFOs, so spawnSync's output pipe closes as soon as the shell exits. */ +const FIFO_LAUNCH = + 'i=$1 o=$2; shift 2; mkfifo -m 600 "$i" "$o" || exit 1; ' + + '( exec /dev/null; exec "$@" <"$i" >"$o" ) & echo $!'; + +/** How long a starting server may take to open its input, in milliseconds. */ +const FIFO_OPEN_TIMEOUT = 10_000; + +function processAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + +export class Ts7FifoIo implements Ts7WireIo { + private readFd = -1; + private writeFd = -1; + private closed = false; + /** The server's process ID, for tests: it is never signaled. */ + readonly pid: number; + + constructor(executable: string, args: string[]) { + const directory = mkdtempSync(join(tmpdir(), "scriptc-ts7-")); + const input = join(directory, "in"); + const output = join(directory, "out"); + try { + const shell = spawnSync( + "/bin/sh", + ["-c", FIFO_LAUNCH, "sh", input, output, executable, ...args], + { stdio: ["ignore", "pipe", "inherit"], encoding: "utf8" }, + ); + const pid = Number(shell.stdout?.trim()); + if (shell.status !== 0 || !Number.isSafeInteger(pid) || pid <= 0) + throw new Error(`Unable to start TypeScript server: ${executable}`); + this.pid = pid; + // A non-blocking open fails with ENXIO until the server opens its end, + // so a server that exits as it starts cannot block this process. + let probe = -1; + for (let waited = 0; probe < 0; waited++) { + try { + probe = openSync(input, constants.O_WRONLY | constants.O_NONBLOCK); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENXIO") throw error; + if (waited >= FIFO_OPEN_TIMEOUT || !processAlive(pid)) + throw new Error(`Unable to start TypeScript server: ${executable}`); + Atomics.wait(sleepBuffer, 0, 0, 1); + } + } + try { + // The server reads now, so a blocking open returns at once. + this.writeFd = openSync(input, "w"); + } finally { + closeSync(probe); + } + // The server's shell opens its output right after its input. + this.readFd = openSync(output, "r"); + track(this); + } catch (error) { + this.close(); + throw error; + } finally { + // Both ends are open, or the start failed: the names are not needed. + rmSync(directory, { recursive: true, force: true }); + } + } + + read = (buffer: Uint8Array, offset: number, length: number): number => { + if (this.closed) throw new Error("TypeScript server transport is closed"); + return transfer(readSync, this.readFd, buffer, offset, length); + }; + + write = (buffer: Uint8Array, offset: number, length: number): number => { + if (this.closed) throw new Error("TypeScript server transport is closed"); + return transfer(writeSync, this.writeFd, buffer, offset, length); + }; + + /** Closing both FIFOs ends the session: the server reads end of file on + * its input and exits, and a write to its closed output ends it with + * SIGPIPE. It is never signaled, because once init has reaped it its PID + * can belong to another process. */ + close = (): void => { + if (this.closed) return; + this.closed = true; + untrack(this); + for (const fd of [this.writeFd, this.readFd]) { + if (fd === -1) continue; + try { + closeSync(fd); + } catch { + /* close the other end too */ + } + } + this.writeFd = -1; + this.readFd = -1; + }; +} + export function spawnTs7Wire(executable: string, args: string[]): Ts7Wire { - return new Ts7Wire(new Ts7ProcessIo(executable, args)); + return new Ts7Wire( + process.platform === "win32" + ? new Ts7NamedPipeIo(executable, args) + : new Ts7FifoIo(executable, args), + ); } From e3724fef7523a3af63c50f962aa45af7e3c18f3e Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Fri, 30 Oct 2026 10:53:19 -0700 Subject: [PATCH 3/3] Fail a missing checker server at startup instead of at its first read The POSIX launcher backgrounded a subshell that blocked opening its input FIFO, so a missing executable looked started and its transport stayed tracked with an exit hook. Check the executable before creating the FIFOs. Co-Authored-By: Claude 5.5 --- .../compiler/src/frontend/ts7/rpc-process.test.ts | 13 ++++++------- packages/compiler/src/frontend/ts7/rpc-process.ts | 7 +++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/compiler/src/frontend/ts7/rpc-process.test.ts b/packages/compiler/src/frontend/ts7/rpc-process.test.ts index e91b0b8200..91d07590ee 100644 --- a/packages/compiler/src/frontend/ts7/rpc-process.test.ts +++ b/packages/compiler/src/frontend/ts7/rpc-process.test.ts @@ -30,13 +30,12 @@ test.skipIf(!posix)("a FIFO server echoes, and exits when its transport closes", expect(alive(io.pid)).toBe(false); }); -test.skipIf(!posix)("a FIFO server that cannot start fails its first read", () => { - const io = new Ts7FifoIo("/nonexistent/scriptc-ts7-server", []); - try { - expect(io.read(Buffer.alloc(1), 0, 1)).toBe(0); - } finally { - io.close(); - } +test.skipIf(!posix)("a missing FIFO server fails at construction and tracks nothing", () => { + const before = process.listenerCount("exit"); + expect(() => new Ts7FifoIo("/nonexistent/scriptc-ts7-server", [])).toThrow( + "Unable to start TypeScript server", + ); + expect(process.listenerCount("exit")).toBe(before); }); test.skipIf(!posix)( diff --git a/packages/compiler/src/frontend/ts7/rpc-process.ts b/packages/compiler/src/frontend/ts7/rpc-process.ts index 24d8f5a99a..bdcf30478f 100644 --- a/packages/compiler/src/frontend/ts7/rpc-process.ts +++ b/packages/compiler/src/frontend/ts7/rpc-process.ts @@ -135,9 +135,12 @@ class Ts7NamedPipeIo implements Ts7WireIo { * this process would stay a zombie after it exits: Node reaps children only * from its event loop, which a synchronous compile never returns to. The * subshell points its standard streams at /dev/null before it opens the - * FIFOs, so spawnSync's output pipe closes as soon as the shell exits. */ + * FIFOs, so spawnSync's output pipe closes as soon as the shell exits. A + * missing or non-executable server fails here, before any FIFO exists: the + * background subshell would otherwise block opening its input and the failure + * would only surface as end of file on the first read. */ const FIFO_LAUNCH = - 'i=$1 o=$2; shift 2; mkfifo -m 600 "$i" "$o" || exit 1; ' + + 'i=$1 o=$2; shift 2; command -v "$1" >/dev/null || exit 127; mkfifo -m 600 "$i" "$o" || exit 1; ' + '( exec /dev/null; exec "$@" <"$i" >"$o" ) & echo $!'; /** How long a starting server may take to open its input, in milliseconds. */