Merge remote-tracking branch 'origin/farm/59c71bcd/isolate-tiny-model-process'
This commit is contained in:
@@ -18,6 +18,10 @@
|
||||
|
||||
- Fixed `read local://<file>` resolving to the wrong session's artifacts directory in multi-session ACP hosts (e.g. cmux). `LocalProtocolHandler.resolve` now honors `context.localProtocolOptions` supplied by the calling tool before falling back to the process-wide override or the first `main`-kind session in the global `AgentRegistry`; `read`, `find`, `search`, `ast_grep`, and `ast_edit` thread their session's options through so a `local://PLAN.md` lookup hits the calling session's `local` root instead of a sibling session's ([#1608](https://github.com/can1357/oh-my-pi/issues/1608)).
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `omp` segfaulting on exit on Windows after the tiny title/memory model loaded `onnxruntime-node` (issue [#1606](https://github.com/can1357/oh-my-pi/issues/1606)). The tiny model now runs in a Bun subprocess instead of a Worker thread, so the NAPI finalizer that crashes during shutdown never executes in the agent's address space; the subprocess is `SIGKILL`'d on dispose to skip every native destructor on every platform.
|
||||
|
||||
## [15.7.4] - 2026-05-31
|
||||
|
||||
### Removed
|
||||
|
||||
@@ -56,7 +56,6 @@ async function main(): Promise<void> {
|
||||
"../stats/src/sync-worker.ts",
|
||||
"./src/tools/browser/tab-worker-entry.ts",
|
||||
"./src/eval/js/worker-entry.ts",
|
||||
"./src/tiny/worker.ts",
|
||||
// Legacy pi-* extension compat entrypoints served by
|
||||
// `legacy-pi-compat.ts`. These are reached via computed bunfs paths
|
||||
// (which `--compile`'s static analyzer cannot trace), so each must be
|
||||
|
||||
@@ -54,12 +54,60 @@ async function runSmokeTest(): Promise<void> {
|
||||
process.stdout.write("smoke-test: ok\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Hidden subcommand that boots the tiny-model worker inside this process
|
||||
* over the parent's IPC channel. The agent's main process spawns the same
|
||||
* binary with this flag so `onnxruntime-node` (loaded transitively by
|
||||
* `@huggingface/transformers`) lives in a child address space. The parent
|
||||
* `SIGKILL`s the child on shutdown so the NAPI finalizer never runs in
|
||||
* either process — that finalizer segfaults Bun on Windows (issue #1606).
|
||||
*/
|
||||
async function runTinyWorker(): Promise<void> {
|
||||
const { startTinyTitleWorker } = await import("./tiny/worker");
|
||||
const { promise: shuttingDown, resolve: shutdown } = Promise.withResolvers<void>();
|
||||
const send = (message: unknown): void => {
|
||||
// `process.send` only exists when spawned with an IPC channel; the
|
||||
// parent always spawns us that way. If it's missing, the parent
|
||||
// vanished and there's no one to talk to.
|
||||
const sender = (process as NodeJS.Process & { send?: (m: unknown) => boolean }).send;
|
||||
if (!sender) {
|
||||
shutdown();
|
||||
return;
|
||||
}
|
||||
try {
|
||||
sender.call(process, message);
|
||||
} catch {
|
||||
shutdown();
|
||||
}
|
||||
};
|
||||
startTinyTitleWorker({
|
||||
send,
|
||||
onMessage(handler) {
|
||||
const wrap = (data: unknown): void => handler(data as never);
|
||||
process.on("message", wrap);
|
||||
return () => {
|
||||
process.off("message", wrap);
|
||||
};
|
||||
},
|
||||
});
|
||||
// Parent went away (crashed, SIGKILL, etc.) — commit suicide so we don't
|
||||
// linger as an orphan. SIGKILL via `process.kill` keeps us symmetrical
|
||||
// with the parent's hard-kill on shutdown: skip every JS/native finalizer.
|
||||
process.on("disconnect", () => shutdown());
|
||||
await shuttingDown;
|
||||
process.kill(process.pid, "SIGKILL");
|
||||
}
|
||||
|
||||
/** Run the CLI with the given argv (no `process.argv` prefix). */
|
||||
export async function runCli(argv: string[]): Promise<void> {
|
||||
if (argv[0] === "--smoke-test") {
|
||||
await runSmokeTest();
|
||||
return;
|
||||
}
|
||||
if (argv[0] === "--tiny-worker") {
|
||||
await runTinyWorker();
|
||||
return;
|
||||
}
|
||||
// --help and --version are handled by run() directly, don't rewrite those.
|
||||
// Everything else that isn't a known subcommand routes to "launch".
|
||||
const first = argv[0];
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import * as path from "node:path";
|
||||
import { $env, isCompiledBinary, logger } from "@oh-my-pi/pi-utils";
|
||||
import type { Subprocess } from "bun";
|
||||
import { settings } from "../config/settings";
|
||||
import { tinyModelDeviceSettingToEnv } from "./device";
|
||||
import { tinyModelDtypeSettingToEnv } from "./dtype";
|
||||
@@ -12,6 +14,14 @@ import {
|
||||
} from "./models";
|
||||
import type { TinyTitleProgressEvent, TinyTitleWorkerInbound, TinyTitleWorkerOutbound } from "./title-protocol";
|
||||
|
||||
/**
|
||||
* Abstraction over the tiny-model subprocess. Modelled as a worker interface
|
||||
* so existing callers (titles, memory completions, downloads) compose the
|
||||
* same way; the runtime implementation is a Bun child process so
|
||||
* `onnxruntime-node`'s NAPI finalizer never runs inside the main agent
|
||||
* address space — that destructor segfaults Bun on Windows during shutdown
|
||||
* (issue #1606).
|
||||
*/
|
||||
interface WorkerHandle {
|
||||
send(message: TinyTitleWorkerInbound): void;
|
||||
onMessage(handler: (message: TinyTitleWorkerOutbound) => void): () => void;
|
||||
@@ -31,6 +41,12 @@ export interface TinyTitleDownloadOptions {
|
||||
|
||||
const SMOKE_TEST_TIMEOUT_MS = 5_000;
|
||||
|
||||
/**
|
||||
* Hidden subcommand on the main CLI that boots the tiny-model worker in the
|
||||
* spawned subprocess. Kept in sync with the dispatch in `cli.ts`.
|
||||
*/
|
||||
export const TINY_WORKER_ARG = "--tiny-worker";
|
||||
|
||||
function readTinyModelSetting(path: "providers.tinyModelDevice" | "providers.tinyModelDtype"): string | undefined {
|
||||
try {
|
||||
const value = settings.get(path);
|
||||
@@ -66,49 +82,128 @@ export function tinyWorkerEnvOverlay(
|
||||
}
|
||||
|
||||
/**
|
||||
* Env handed to the tiny-model worker. The `PI_TINY_DEVICE` / `PI_TINY_DTYPE` env
|
||||
* vars win; otherwise the persisted `providers.tinyModelDevice` /
|
||||
* `providers.tinyModelDtype` settings are mapped onto those vars so the worker's
|
||||
* env-based resolution picks them up. Resolved once at spawn (pipelines are cached).
|
||||
* Env handed to the tiny-model subprocess. The `PI_TINY_DEVICE` / `PI_TINY_DTYPE`
|
||||
* env vars win; otherwise the persisted `providers.tinyModelDevice` /
|
||||
* `providers.tinyModelDtype` settings are mapped onto those vars so the
|
||||
* subprocess's env-based resolution picks them up. Resolved once at spawn
|
||||
* (pipelines are cached for the lifetime of the subprocess).
|
||||
*/
|
||||
function tinyWorkerEnv(): Record<string, string> | undefined {
|
||||
function tinyWorkerEnv(): Record<string, string> {
|
||||
const overlay = tinyWorkerEnvOverlay(
|
||||
$env,
|
||||
readTinyModelSetting("providers.tinyModelDevice"),
|
||||
readTinyModelSetting("providers.tinyModelDtype"),
|
||||
);
|
||||
if (Object.keys(overlay).length === 0) return undefined;
|
||||
return { ...($env as Record<string, string>), ...overlay };
|
||||
const base = $env as Record<string, string | undefined>;
|
||||
const merged: Record<string, string> = {};
|
||||
for (const key in base) {
|
||||
const value = base[key];
|
||||
if (typeof value === "string") merged[key] = value;
|
||||
}
|
||||
for (const key in overlay) merged[key] = overlay[key];
|
||||
return merged;
|
||||
}
|
||||
|
||||
export function createTinyTitleWorker(): Worker {
|
||||
const env = tinyWorkerEnv();
|
||||
const options: WorkerOptions = env ? { type: "module", env } : { type: "module" };
|
||||
return isCompiledBinary()
|
||||
? new Worker("./packages/coding-agent/src/tiny/worker.ts", options)
|
||||
: new Worker(new URL("./worker.ts", import.meta.url).href, options);
|
||||
/**
|
||||
* Resolve the argv used to relaunch the agent CLI into tiny-worker mode. In a
|
||||
* compiled binary the entry point is the binary itself; in dev/source the
|
||||
* spawned `bun` needs the absolute path to `cli.ts` so it can resolve module
|
||||
* imports against the on-disk source tree.
|
||||
*/
|
||||
function tinyWorkerSpawnCmd(): string[] {
|
||||
if (isCompiledBinary()) return [process.execPath, TINY_WORKER_ARG];
|
||||
const cliPath = path.resolve(import.meta.dir, "..", "cli.ts");
|
||||
return [process.execPath, cliPath, TINY_WORKER_ARG];
|
||||
}
|
||||
|
||||
function wrapBunWorker(worker: Worker): WorkerHandle {
|
||||
(worker as Worker & { unref?: () => void }).unref?.();
|
||||
interface SpawnedSubprocess {
|
||||
proc: Subprocess<"ignore", "inherit", "inherit">;
|
||||
inbound: Set<(message: TinyTitleWorkerOutbound) => void>;
|
||||
errors: Set<(error: Error) => void>;
|
||||
/**
|
||||
* Flipped to `true` by {@link wrapSubprocess}'s `terminate()` right
|
||||
* before it SIGKILLs the child so `onExit` can distinguish the
|
||||
* expected hard-kill from a crash/OOM/external signal. Only the
|
||||
* latter is surfaced as a worker error.
|
||||
*/
|
||||
intentionalExit: { value: boolean };
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn the tiny-model worker as a subprocess. Exported for tests and the
|
||||
* smoke probe; production callers go through {@link spawnTinyTitleWorker}
|
||||
* which wraps the result in a {@link WorkerHandle}.
|
||||
*/
|
||||
export function createTinyTitleSubprocess(): SpawnedSubprocess {
|
||||
const inbound = new Set<(message: TinyTitleWorkerOutbound) => void>();
|
||||
const errors = new Set<(error: Error) => void>();
|
||||
const intentionalExit = { value: false };
|
||||
const proc = Bun.spawn({
|
||||
cmd: tinyWorkerSpawnCmd(),
|
||||
env: tinyWorkerEnv(),
|
||||
stdin: "ignore",
|
||||
stdout: "inherit",
|
||||
stderr: "inherit",
|
||||
serialization: "advanced",
|
||||
windowsHide: true,
|
||||
ipc(message) {
|
||||
for (const handler of inbound) handler(message as TinyTitleWorkerOutbound);
|
||||
},
|
||||
onExit(_proc, exitCode, signalCode) {
|
||||
// Clean exit. The child only exits via SIGKILL in practice, but
|
||||
// treat code 0 as a no-op for symmetry.
|
||||
if (exitCode === 0) return;
|
||||
// `exitCode === null` + non-null `signalCode` covers both the
|
||||
// expected SIGKILL from `terminate()` AND external kills
|
||||
// (SIGSEGV from a native crash, SIGKILL from the OOM killer, an
|
||||
// operator `kill -9`, etc.). Swallow only the expected one;
|
||||
// every other signal exit is a real worker death that must
|
||||
// fault every in-flight request so callers don't await forever.
|
||||
if (exitCode === null && intentionalExit.value) return;
|
||||
const reason = exitCode !== null ? `code ${exitCode}` : `signal ${signalCode ?? "unknown"}`;
|
||||
const err = new Error(`tiny model subprocess exited with ${reason}`);
|
||||
for (const handler of errors) handler(err);
|
||||
},
|
||||
});
|
||||
// Don't keep the parent event loop alive on account of an idle worker; the
|
||||
// agent dispose path calls `terminate()` explicitly when shutting down.
|
||||
proc.unref();
|
||||
return { proc, inbound, errors, intentionalExit };
|
||||
}
|
||||
|
||||
function wrapSubprocess({ proc, inbound, errors, intentionalExit }: SpawnedSubprocess): WorkerHandle {
|
||||
return {
|
||||
send(message) {
|
||||
worker.postMessage(message);
|
||||
try {
|
||||
proc.send(message);
|
||||
} catch (error) {
|
||||
logger.debug("tiny-title: send to subprocess failed", {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
},
|
||||
onMessage(handler) {
|
||||
const wrap = (event: MessageEvent): void => handler(event.data as TinyTitleWorkerOutbound);
|
||||
worker.addEventListener("message", wrap);
|
||||
return () => worker.removeEventListener("message", wrap);
|
||||
inbound.add(handler);
|
||||
return () => inbound.delete(handler);
|
||||
},
|
||||
onError(handler) {
|
||||
const wrap = (event: ErrorEvent): void => {
|
||||
handler(event.error instanceof Error ? event.error : new Error(event.message || "tiny title worker error"));
|
||||
};
|
||||
worker.addEventListener("error", wrap);
|
||||
return () => worker.removeEventListener("error", wrap);
|
||||
errors.add(handler);
|
||||
return () => errors.delete(handler);
|
||||
},
|
||||
async terminate() {
|
||||
worker.terminate();
|
||||
// SIGKILL: the whole point of the subprocess isolation is that the
|
||||
// parent never runs `onnxruntime-node`'s NAPI finalizer. A polite
|
||||
// SIGTERM lets the subprocess try to clean up, which is exactly the
|
||||
// codepath that crashes Bun on Windows. Hard-kill instead — the
|
||||
// model lives in process memory and the OS reclaims everything.
|
||||
// Flip the intentional-exit flag *before* killing so `onExit` can
|
||||
// tell this apart from a crash or external SIGKILL.
|
||||
intentionalExit.value = true;
|
||||
try {
|
||||
proc.kill("SIGKILL");
|
||||
} catch {
|
||||
// Already gone.
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -126,10 +221,6 @@ function spawnInlineUnavailableWorker(error: unknown): WorkerHandle {
|
||||
emit({ type: "pong", id: message.id });
|
||||
return;
|
||||
}
|
||||
if (message.type === "close") {
|
||||
emit({ type: "closed" });
|
||||
return;
|
||||
}
|
||||
emit({ type: "error", id: message.id, error: errorMessage });
|
||||
});
|
||||
},
|
||||
@@ -148,9 +239,9 @@ function spawnInlineUnavailableWorker(error: unknown): WorkerHandle {
|
||||
|
||||
function spawnTinyTitleWorker(): WorkerHandle {
|
||||
try {
|
||||
return wrapBunWorker(createTinyTitleWorker());
|
||||
return wrapSubprocess(createTinyTitleSubprocess());
|
||||
} catch (error) {
|
||||
logger.warn("Tiny title Worker spawn failed; local titles disabled", {
|
||||
logger.warn("Tiny title worker spawn failed; local titles disabled", {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
return spawnInlineUnavailableWorker(error);
|
||||
@@ -293,9 +384,9 @@ export class TinyTitleClient {
|
||||
}
|
||||
this.#pending.clear();
|
||||
try {
|
||||
worker?.send({ type: "close" });
|
||||
await worker?.terminate();
|
||||
} catch {
|
||||
// Worker may already be gone.
|
||||
// Already gone.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -317,7 +408,6 @@ export class TinyTitleClient {
|
||||
this.#emitProgress(message.event);
|
||||
return;
|
||||
}
|
||||
if (message.type === "closed") return;
|
||||
if (message.type === "pong") return;
|
||||
|
||||
const pending = this.#pending.get(message.id);
|
||||
@@ -371,25 +461,25 @@ export async function smokeTestTinyTitleWorker({
|
||||
}: {
|
||||
timeoutMs?: number;
|
||||
} = {}): Promise<void> {
|
||||
const worker = createTinyTitleWorker();
|
||||
const handle = wrapSubprocess(createTinyTitleSubprocess());
|
||||
const { promise, resolve, reject } = Promise.withResolvers<void>();
|
||||
const timer = setTimeout(() => reject(new Error(`tiny title worker did not pong within ${timeoutMs}ms`)), timeoutMs);
|
||||
worker.onmessage = (event: MessageEvent<TinyTitleWorkerOutbound>) => {
|
||||
const message = event.data;
|
||||
const unsubscribeMessage = handle.onMessage(message => {
|
||||
if (message.type === "pong") {
|
||||
resolve();
|
||||
return;
|
||||
}
|
||||
if (message.type === "log") return;
|
||||
reject(new Error(`tiny title worker: expected pong, got ${JSON.stringify(message)}`));
|
||||
};
|
||||
worker.onerror = (event: ErrorEvent) => {
|
||||
reject(event.error instanceof Error ? event.error : new Error(event.message || "tiny title worker error"));
|
||||
};
|
||||
});
|
||||
const unsubscribeError = handle.onError(reject);
|
||||
try {
|
||||
worker.postMessage({ type: "ping", id: "smoke" } satisfies TinyTitleWorkerInbound);
|
||||
handle.send({ type: "ping", id: "smoke" } satisfies TinyTitleWorkerInbound);
|
||||
await promise;
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
worker.terminate();
|
||||
unsubscribeMessage();
|
||||
unsubscribeError();
|
||||
await handle.terminate();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,8 +31,7 @@ export type TinyTitleWorkerInbound =
|
||||
| { type: "ping"; id: string }
|
||||
| { type: "generate"; id: string; modelKey: TinyTitleLocalModelKey; message: string }
|
||||
| { type: "complete"; id: string; modelKey: TinyLocalModelKey; prompt: string; maxTokens?: number }
|
||||
| { type: "download"; id: string; modelKey: TinyLocalModelKey }
|
||||
| { type: "close" };
|
||||
| { type: "download"; id: string; modelKey: TinyLocalModelKey };
|
||||
|
||||
export type TinyTitleWorkerOutbound =
|
||||
| { type: "pong"; id: string }
|
||||
@@ -41,11 +40,17 @@ export type TinyTitleWorkerOutbound =
|
||||
| { type: "downloaded"; id: string }
|
||||
| { type: "error"; id: string; error: string }
|
||||
| { type: "progress"; id: string; event: TinyTitleProgressEvent }
|
||||
| { type: "log"; level: "debug" | "warn" | "error"; msg: string; meta?: Record<string, unknown> }
|
||||
| { type: "closed" };
|
||||
| { type: "log"; level: "debug" | "warn" | "error"; msg: string; meta?: Record<string, unknown> };
|
||||
|
||||
/**
|
||||
* Wire transport between the parent (`TinyTitleClient`) and the tiny-model
|
||||
* subprocess. The parent owns the subprocess lifecycle (graceful work, hard
|
||||
* kill on shutdown); the protocol therefore carries no explicit close
|
||||
* handshake — once the parent decides to terminate, it signals the OS to
|
||||
* reap the child so `onnxruntime-node`'s NAPI finalizer never runs in any
|
||||
* shared address space. See `title-client.ts` for the spawn/kill glue.
|
||||
*/
|
||||
export interface TinyTitleTransport {
|
||||
send(message: TinyTitleWorkerOutbound): void;
|
||||
onMessage(handler: (message: TinyTitleWorkerInbound) => void): () => void;
|
||||
close(): void;
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import * as fs from "node:fs/promises";
|
||||
import { createRequire } from "node:module";
|
||||
import * as path from "node:path";
|
||||
import { parentPort } from "node:worker_threads";
|
||||
import type {
|
||||
ProgressInfo,
|
||||
TextGenerationPipeline,
|
||||
@@ -20,12 +19,7 @@ import {
|
||||
type TinyTitleLocalModelSpec,
|
||||
} from "./models";
|
||||
import { formatTitleUserMessage, normalizeGeneratedTitle } from "./text";
|
||||
import type {
|
||||
TinyTitleProgressEvent,
|
||||
TinyTitleTransport,
|
||||
TinyTitleWorkerInbound,
|
||||
TinyTitleWorkerOutbound,
|
||||
} from "./title-protocol";
|
||||
import type { TinyTitleProgressEvent, TinyTitleTransport, TinyTitleWorkerInbound } from "./title-protocol";
|
||||
|
||||
const TITLE_PREFILL = "<title>";
|
||||
const TITLE_CLOSE = "</title>";
|
||||
@@ -497,16 +491,6 @@ async function generateCompletion(
|
||||
return generated === "" ? null : generated;
|
||||
}
|
||||
|
||||
function releasePipelines(): void {
|
||||
// Intentionally NOT calling `pipeline.dispose()`. transformers.js disposes the
|
||||
// underlying onnxruntime InferenceSession, freeing native memory that Bun's
|
||||
// worker/NAPI teardown then frees a second time — a double-free that aborts the
|
||||
// process on quit ("malloc: pointer being freed was not allocated" /
|
||||
// "NAPI FATAL ERROR"). The worker is torn down immediately after `close`, so the
|
||||
// OS reclaims the model memory regardless; skipping dispose avoids the crash.
|
||||
pipelines.clear();
|
||||
}
|
||||
|
||||
function enqueueRequest(
|
||||
transport: TinyTitleTransport,
|
||||
request: Extract<TinyTitleWorkerInbound, { type: "generate" | "complete" | "download" }>,
|
||||
@@ -555,33 +539,6 @@ export function startTinyTitleWorker(transport: TinyTitleTransport): void {
|
||||
transport.send({ type: "pong", id: message.id });
|
||||
return;
|
||||
}
|
||||
if (message.type === "close") {
|
||||
releasePipelines();
|
||||
transport.send({ type: "closed" });
|
||||
transport.close();
|
||||
return;
|
||||
}
|
||||
enqueueRequest(transport, message);
|
||||
});
|
||||
}
|
||||
|
||||
if (!parentPort) throw new Error("tiny-title-worker: missing parentPort");
|
||||
|
||||
const port = parentPort;
|
||||
const transport: TinyTitleTransport = {
|
||||
send: (message: TinyTitleWorkerOutbound) => port.postMessage(message),
|
||||
onMessage: handler => {
|
||||
const wrap = (data: unknown): void => handler(data as TinyTitleWorkerInbound);
|
||||
port.on("message", wrap);
|
||||
return () => port.off("message", wrap);
|
||||
},
|
||||
close: () => {
|
||||
try {
|
||||
port.close();
|
||||
} catch {
|
||||
// Already closed.
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
startTinyTitleWorker(transport);
|
||||
|
||||
@@ -35,7 +35,6 @@ describe("issue #1150 — release-build script must list all worker --compile en
|
||||
"./packages/stats/src/sync-worker.ts",
|
||||
"./packages/coding-agent/src/tools/browser/tab-worker-entry.ts",
|
||||
"./packages/coding-agent/src/eval/js/worker-entry.ts",
|
||||
"./packages/coding-agent/src/tiny/worker.ts",
|
||||
];
|
||||
|
||||
it("scripts/ci-release-build-binaries.ts lists every worker as an explicit --compile entrypoint", async () => {
|
||||
@@ -56,7 +55,6 @@ describe("issue #1150 — release-build script must list all worker --compile en
|
||||
"../stats/src/sync-worker.ts",
|
||||
"./src/tools/browser/tab-worker-entry.ts",
|
||||
"./src/eval/js/worker-entry.ts",
|
||||
"./src/tiny/worker.ts",
|
||||
];
|
||||
const source = await Bun.file(devScriptPath).text();
|
||||
for (const entry of devEntrypoints) {
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
/**
|
||||
* Regression for https://github.com/can1357/oh-my-pi/issues/1606
|
||||
*
|
||||
* On Windows, `onnxruntime-node`'s NAPI finalizer segfaults Bun during
|
||||
* shutdown after `@huggingface/transformers` has loaded a tiny model in a
|
||||
* Worker thread. The agent used to host the tiny-model worker as a Worker
|
||||
* inside its own process; tearing the worker down ran the native destructor
|
||||
* in the parent's address space and crashed the CLI on exit.
|
||||
*
|
||||
* The fix relocates the worker to a child process: `title-client.ts` spawns
|
||||
* `process.execPath … --tiny-worker`, `cli.ts` dispatches that flag into
|
||||
* `runTinyWorker`, and the parent `SIGKILL`s the child on dispose so the
|
||||
* native finalizer never runs in either address space. These tests pin the
|
||||
* three pieces of that contract so a future refactor cannot quietly land
|
||||
* the original crash again.
|
||||
*/
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { createTinyTitleSubprocess, smokeTestTinyTitleWorker, TINY_WORKER_ARG } from "../src/tiny/title-client";
|
||||
|
||||
describe("issue #1606 — tiny model lives in an isolated subprocess", () => {
|
||||
it("ping/pongs through the spawned worker subprocess and tears it down cleanly", async () => {
|
||||
// `smokeTestTinyTitleWorker` is the runtime probe wired into
|
||||
// `omp --smoke-test`: it spawns the worker subprocess via
|
||||
// `Bun.spawn`, sends a ping over the IPC channel, awaits the pong,
|
||||
// then SIGKILLs the child. If anyone reverts the worker to an
|
||||
// in-process `new Worker(...)` thread or drops the `--tiny-worker`
|
||||
// CLI dispatch, the spawn either picks up the wrong entrypoint or
|
||||
// the ping never round-trips, and this test fails.
|
||||
await expect(smokeTestTinyTitleWorker({ timeoutMs: 15_000 })).resolves.toBeUndefined();
|
||||
}, 30_000);
|
||||
|
||||
it("CLI dispatches the flag that `title-client.ts` passes to the spawned child", async () => {
|
||||
// `tinyWorkerSpawnCmd()` and the cli switch must agree on the exact
|
||||
// flag, character-for-character — the spawned `bun`/binary sees only
|
||||
// `argv` and there is no fallback path that "re-routes" the worker
|
||||
// on misnamed flags. Pin the spelling on both ends.
|
||||
const cliSource = await Bun.file(new URL("../src/cli.ts", import.meta.url)).text();
|
||||
expect(cliSource).toContain(`argv[0] === "${TINY_WORKER_ARG}"`);
|
||||
expect(cliSource).toContain("runTinyWorker");
|
||||
});
|
||||
|
||||
it("surfaces unexpected signal exits so in-flight callers don't await forever", async () => {
|
||||
// If the child dies from a signal we did NOT request — SIGSEGV from a
|
||||
// native crash (the original Windows shutdown bug, now relocated to
|
||||
// the child), an OOM SIGKILL, or an operator `kill -9` — the
|
||||
// subprocess wrapper must fault every in-flight request via the
|
||||
// `errors` channel. The original fix swallowed any `exitCode === null`
|
||||
// exit unconditionally, which left `TinyTitleClient.#pending`
|
||||
// promises hanging forever. Pin the new contract: an external
|
||||
// SIGKILL (no `intentionalExit` flip) MUST surface a worker error.
|
||||
const sub = createTinyTitleSubprocess();
|
||||
try {
|
||||
const { promise, resolve } = Promise.withResolvers<Error>();
|
||||
sub.errors.add(resolve);
|
||||
sub.proc.kill("SIGKILL");
|
||||
const err = await promise;
|
||||
expect(err.message).toMatch(/signal/i);
|
||||
} finally {
|
||||
// Ensure the child is reaped even on assertion failure.
|
||||
try {
|
||||
sub.proc.kill("SIGKILL");
|
||||
} catch {}
|
||||
await sub.proc.exited;
|
||||
}
|
||||
}, 15_000);
|
||||
|
||||
it("does not surface intentional terminate() SIGKILLs as worker errors", async () => {
|
||||
// Inverse of the previous test: a SIGKILL issued by the wrapper's
|
||||
// own `terminate()` MUST NOT fault callers — terminate is the
|
||||
// shutdown path and the worker handle is already torn down by then.
|
||||
// Regression guard against an over-eager fix that surfaces every
|
||||
// signal exit indiscriminately.
|
||||
const sub = createTinyTitleSubprocess();
|
||||
let errored = false;
|
||||
sub.errors.add(() => {
|
||||
errored = true;
|
||||
});
|
||||
// Simulate what `wrapSubprocess.terminate()` does: flip the flag,
|
||||
// then SIGKILL. We test the primitive directly rather than going
|
||||
// through the wrapper to avoid coupling to `WorkerHandle` internals.
|
||||
sub.intentionalExit.value = true;
|
||||
sub.proc.kill("SIGKILL");
|
||||
await sub.proc.exited;
|
||||
// Give onExit a microtask to drain — Bun's exited promise resolves
|
||||
// after onExit fires, but be defensive.
|
||||
await Bun.sleep(20);
|
||||
expect(errored).toBe(false);
|
||||
}, 10_000);
|
||||
});
|
||||
@@ -27,7 +27,6 @@ const workerEntrypoints = [
|
||||
"./packages/stats/src/sync-worker.ts",
|
||||
"./packages/coding-agent/src/tools/browser/tab-worker-entry.ts",
|
||||
"./packages/coding-agent/src/eval/js/worker-entry.ts",
|
||||
"./packages/coding-agent/src/tiny/worker.ts",
|
||||
];
|
||||
const isDryRun = process.argv.includes("--dry-run");
|
||||
const targets: BinaryTarget[] = [
|
||||
|
||||
Reference in New Issue
Block a user