diff --git a/CHANGELOG.md b/CHANGELOG.md index 13c6b04..f068712 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,21 @@ copy the entire working tree (including `.git` and `bun.lock`) into the install dir and previously routed through Bun, causing ABI mismatches with the Node-built `better-sqlite3` / `sqlite-vec` native modules. +- Darwin Metal: launcher (`bin/qmd`) and `bun test` preload now set + `GGML_METAL_NO_RESIDENCY=1` by default on macOS to disable libggml-metal's + residency-set keep-alive timer. Previously, llama-using commands (`query`, + `vsearch`, `embed`) and the test runner dumped a multi-kB GGML/Metal + backtrace at process exit even when output succeeded + (ggml-org/llama.cpp#17869) — the static `ggml_metal_device` destructor + asserts `[rsets->data count] == 0` during `__cxa_finalize_ranges`, but the + residency set's 180 s keep-alive timer hasn't expired yet. Residency sets + give no measurable speedup for QMD's short-lived CLI workflow (benchmarked + on M3 Pro), so disabling them is a pure win. The Bun preload uses + `bun:ffi` to call libc `setenv()` directly because Bun does not propagate + `process.env` mutations through to native `getenv()` (Node does). Opt back + in with `QMD_METAL_KEEP_RESIDENCY=1` for long-lived qmd processes (e.g. the + MCP daemon may benefit on hot reload) or to triage the upstream fix. + `qmd doctor` now reports the mitigation state. ### Docs diff --git a/bin/qmd b/bin/qmd index b76d20e..3983dc4 100755 --- a/bin/qmd +++ b/bin/qmd @@ -30,6 +30,27 @@ if (process.argv[2] === "mcp") { process.env.GGML_BACKEND_SILENT = process.env.GGML_BACKEND_SILENT || "1"; } +// libggml-metal on macOS uses "residency sets" to keep allocated model memory +// resident across inference requests (180-second keep_alive timer). The +// process-static device destructor that runs during libc exit() asserts the +// residency set is empty (ggml-org/llama.cpp#17869); the keep_alive hasn't +// expired by exit, so the assertion fails and ggml_abort dumps a multi-kB +// stack trace to stderr even when the user-visible results were already +// emitted correctly. No JS-side dispose can prevent it because the static +// destructor runs in __cxa_finalize_ranges, after every JS-reachable cleanup. +// +// For QMD's short-lived CLI workflow, residency sets provide no observable +// performance benefit (subsequent requests don't reuse the warm mapping — +// measured: identical wall time with and without on M3 Pro), so disable them +// by default on darwin. The env var must be set BEFORE the native llama.cpp +// binding loads, which is why it lives here in the launcher rather than in +// the JS entry point. Opt back in with QMD_METAL_KEEP_RESIDENCY=1 if you +// run long-lived qmd processes (the MCP daemon may benefit on hot reload) +// or are triaging an upstream Metal teardown fix. +if (process.platform === "darwin" && process.env.QMD_METAL_KEEP_RESIDENCY !== "1") { + process.env.GGML_METAL_NO_RESIDENCY = process.env.GGML_METAL_NO_RESIDENCY || "1"; +} + function hasBun() { try { const res = spawnSync("bun", ["--version"], { stdio: "ignore", shell: process.platform === "win32" }); diff --git a/src/cli/qmd.ts b/src/cli/qmd.ts index c8bc464..b95cfcc 100755 --- a/src/cli/qmd.ts +++ b/src/cli/qmd.ts @@ -81,7 +81,7 @@ import { type ReindexResult, type ChunkStrategy, } from "../store.js"; -import { disposeDefaultLlamaCpp, getDefaultLlamaCpp, setDefaultLlamaCpp, LlamaCpp, withLLMSession, pullModels, DEFAULT_MODEL_CACHE_DIR, resolveEmbedModel, resolveGenerateModel, resolveRerankModel, resolveModels, inspectGgufFile } from "../llm.js"; +import { disposeDefaultLlamaCpp, getDefaultLlamaCpp, setDefaultLlamaCpp, LlamaCpp, withLLMSession, pullModels, DEFAULT_MODEL_CACHE_DIR, resolveEmbedModel, resolveGenerateModel, resolveRerankModel, resolveModels, inspectGgufFile, isDarwinMetalMitigationActive } from "../llm.js"; import { formatSearchResults, formatDocuments, @@ -225,10 +225,8 @@ type FinishSuccessfulCliCommandOptions = { format?: OutputFormat; cleanup?: () => Promise; exit?: (code: number) => void; - immediateExit?: (code: number) => void; stdout?: CliLifecycleWritable; stderr?: CliLifecycleWritable; - platform?: NodeJS.Platform; }; async function flushWritable(stream: CliLifecycleWritable): Promise { @@ -237,44 +235,24 @@ async function flushWritable(stream: CliLifecycleWritable): Promise { }); } -function shouldBypassNativeCleanup(options: FinishSuccessfulCliCommandOptions): boolean { - return ( - (options.platform ?? process.platform) === "darwin" && - options.command === "query" && - options.format === "json" && - process.env.QMD_DISABLE_DARWIN_QUERY_JSON_SAFE_EXIT !== "1" - ); -} - -function immediateProcessExit(code: number): void { - const processWithReallyExit = process as NodeJS.Process & { reallyExit?: (code?: number) => void }; - if (typeof processWithReallyExit.reallyExit === "function") { - processWithReallyExit.reallyExit(code); - return; - } - process.exit(code); -} - /** - * Finish a successful CLI command after output has been flushed. On macOS JSON - * query runs, skip normal native teardown and use Node/Bun's immediate exit path: - * ggml Metal can abort from C++ finalizers after valid JSON has already been - * produced (#368). This wrapper is only reached after the command completed, so - * real query failures still exit through the normal error path before this runs. + * Finish a successful CLI command after output has been flushed. + * + * Best-effort llama disposal (JS-side resources only): if it throws, we still + * exit 0 because the user's output is already on the wire. The libggml-metal + * static-destructor crash on darwin (ggml-org/llama.cpp#17869) is prevented + * by `bin/qmd` exporting `GGML_METAL_NO_RESIDENCY=1` before the native + * binding loads — see `isDarwinMetalMitigationActive` in `src/llm.ts` for + * the runtime check exposed to diagnostics. Every code path that touches + * Metal (query, vsearch, embed, expand) is covered by that single env var, + * with no per-command bypass logic required here. */ export async function finishSuccessfulCliCommand(options: FinishSuccessfulCliCommandOptions): Promise { const stderr = options.stderr ?? process.stderr; const exit = options.exit ?? ((code: number) => process.exit(code)); - const immediateExit = options.immediateExit ?? immediateProcessExit; await flushWritable(options.stdout ?? process.stdout); - if (shouldBypassNativeCleanup(options)) { - await flushWritable(stderr); - immediateExit(0); - return; - } - try { await (options.cleanup ?? disposeDefaultLlamaCpp)(); } catch (error) { @@ -3609,7 +3587,8 @@ function collectEnvironmentOverrides(activeModels: { embed: string; generate: st add("QMD_EMBED_CONTEXT_SIZE", "overrides embed context size; larger values use more memory"); add("QMD_EDITOR_URI", "overrides clickable editor link template in terminal output"); add("QMD_SKILLS_DIR", "overrides where qmd skills are discovered from"); - add("QMD_DISABLE_DARWIN_QUERY_JSON_SAFE_EXIT", "disables macOS JSON-query safe exit workaround; may re-expose Metal finalizer crashes"); + add("QMD_METAL_KEEP_RESIDENCY", "opts back into libggml-metal residency sets on darwin; restores ~0ms perf wins for long-lived processes but re-exposes the static-destructor backtrace dump at process exit (ggml-org/llama.cpp#17869)"); + add("GGML_METAL_NO_RESIDENCY", "set automatically by the launcher on darwin to disable Metal residency sets (avoids ggml-org/llama.cpp#17869); override via QMD_METAL_KEEP_RESIDENCY=1"); add("NO_COLOR", "disables colored terminal output"); add("CI", "disables real LLM operations inside QMD's LlamaCpp wrapper"); add("HF_ENDPOINT", "changes Hugging Face download endpoint used when pulling models"); @@ -3889,6 +3868,29 @@ async function runDoctorDeviceChecks(nextSteps: string[]): Promise { if (!device.gpuOffloading) { nextSteps.push("GPU was detected but offloading is disabled; check `QMD_LLAMA_GPU=metal|cuda|vulkan` and rerun `qmd doctor`."); } + + // Surface the darwin residency-set mitigation. libggml-metal's + // process-static device dtor asserts on un-expired residency sets + // during libc exit() (ggml-org/llama.cpp#17869), producing a giant + // stderr backtrace after correct output. The bin/qmd launcher exports + // GGML_METAL_NO_RESIDENCY=1 on darwin to skip the assertion entirely. + // No measurable perf cost on short-lived CLI calls. + if (device.gpu === "metal" && process.platform === "darwin") { + if (isDarwinMetalMitigationActive()) { + doctorCheck( + "darwin metal residency", + true, + "GGML_METAL_NO_RESIDENCY=1 set by launcher; clean process exit (avoids ggml-org/llama.cpp#17869). Opt back in with QMD_METAL_KEEP_RESIDENCY=1 if you run long-lived qmd processes." + ); + } else { + doctorCheck( + "darwin metal residency", + false, + "residency sets active (QMD_METAL_KEEP_RESIDENCY=1 or launcher bypassed); llama-using commands may dump a libggml-metal backtrace at exit (ggml-org/llama.cpp#17869) even when output succeeded." + ); + nextSteps.push("Unset `QMD_METAL_KEEP_RESIDENCY` so the launcher can disable Metal residency sets; without this, query/vsearch/embed dump a stack trace at exit even on success."); + } + } } else { const cudaDiagnostic = linuxCudaRuntimeDiagnostic(); const diagnosticSuffix = cudaDiagnostic ? ` ${cudaDiagnostic}.` : ""; diff --git a/src/llm.ts b/src/llm.ts index 1ffa185..4ea0d00 100644 --- a/src/llm.ts +++ b/src/llm.ts @@ -720,6 +720,12 @@ export class LlamaCpp implements LLM { constructor(config: LlamaCppConfig = {}) { + // STRUCTURAL INVARIANT: the launcher (bin/qmd) sets GGML_METAL_NO_RESIDENCY=1 + // on darwin BEFORE the native binding loads, which prevents the libggml-metal + // static destructor assertion at process exit (ggml-org/llama.cpp#17869). + // See isDarwinMetalMitigationActive() for the runtime check exposed to + // diagnostics. No constructor-time guard installation is needed. + this.embedModelUri = resolveEmbedModel({ embed: config.embedModel }); this.generateModelUri = resolveGenerateModel({ generate: config.generateModel }); this.rerankModelUri = resolveRerankModel({ rerank: config.rerankModel }); @@ -1944,6 +1950,66 @@ export function canUnloadLLM(): boolean { return defaultSessionManager.canUnload(); } +// ============================================================================= +// Darwin Metal exit-crash mitigation +// ============================================================================= +// +// libggml-metal on macOS keeps allocated model memory wired via "residency +// sets" with a 180-second keep_alive timer (added in ggml-org/llama.cpp#11427). +// The process-static `std::vector>` +// destructor fires during libc `exit()` → `__cxa_finalize_ranges` and asserts +// `[rsets->data count] == 0` — but the keep_alive hasn't expired, so the +// assertion fails and `ggml_abort` dumps a multi-kilobyte stack trace to +// stderr after the user-visible output. See ggml-org/llama.cpp#17869. +// +// No JS-side dispose call (`llama.dispose()`, `model.dispose()`, etc.) can +// prevent it: the static destructor runs after every JS-reachable cleanup, +// and `process.reallyExit` on Node calls libc `exit()` not `_exit()` (it +// does NOT skip C++ static destructors — verified in +// node/src/api/environment.cc). +// +// The actual fix is to disable residency sets via `GGML_METAL_NO_RESIDENCY=1`, +// which we set from `bin/qmd` before Node loads the native binding. For QMD's +// short-lived CLI workflow this has no measurable cost (subsequent calls +// don't reuse the warm mapping). The functions below report whether that +// mitigation is in effect — kept here, in the module that depends on the +// underlying resource, so doctor can answer "is the protection active?" +// without reaching into env handling directly. +// +// Setting `QMD_METAL_KEEP_RESIDENCY=1` opts back into residency sets (with +// the visible-noise consequences). The legacy `QMD_DISABLE_DARWIN_SAFE_EXIT` +// env var is accepted as a no-op alias for back-compat; it had no effect on +// Node prior to this fix. + +/** + * Whether QMD's darwin Metal exit-crash mitigation is active in this process: + * true → residency sets disabled, process exit completes silently + * false → either non-darwin, or `QMD_METAL_KEEP_RESIDENCY=1` overrode it, + * in which case the libggml-metal teardown assertion may fire + */ +export function isDarwinMetalMitigationActive(): boolean { + if (process.platform !== "darwin") return false; + if (process.env.QMD_METAL_KEEP_RESIDENCY === "1") return false; + return process.env.GGML_METAL_NO_RESIDENCY === "1"; +} + +/** + * Compatibility shim: previous releases installed a `process.on('exit')` hook + * that tried to skip the C++ static destructor by calling `process.reallyExit`. + * That mechanism didn't work on Node (Environment::Exit still calls libc + * `exit()`), so it was replaced by `GGML_METAL_NO_RESIDENCY=1` from bin/qmd. + * Kept as a no-op for code paths that still call it; safe to remove once no + * production launcher predates the residency-set fix. + */ +export function installDarwinExitGuard(): void { + // Intentional no-op. See isDarwinMetalMitigationActive() for the real check. +} + +/** @deprecated Replaced by isDarwinMetalMitigationActive. */ +export function isDarwinExitGuardInstalled(): boolean { + return isDarwinMetalMitigationActive(); +} + // ============================================================================= // Singleton for default LlamaCpp instance // ============================================================================= @@ -1951,7 +2017,9 @@ export function canUnloadLLM(): boolean { let defaultLlamaCpp: LlamaCpp | null = null; /** - * Get the default LlamaCpp instance (creates one if needed) + * Get the default LlamaCpp instance (creates one if needed). The LlamaCpp + * constructor installs the darwin exit guard, so any code path that obtains + * the singleton is protected. */ export function getDefaultLlamaCpp(): LlamaCpp { if (!defaultLlamaCpp) { @@ -1961,12 +2029,24 @@ export function getDefaultLlamaCpp(): LlamaCpp { } /** - * Set a custom default LlamaCpp instance (useful for testing) + * Set a custom default LlamaCpp instance (useful for testing). Setting a + * non-null instance also ensures the darwin exit guard is installed — keeps + * the invariant intact for test doubles that didn't go through the real + * constructor. */ export function setDefaultLlamaCpp(llm: LlamaCpp | null): void { + if (llm !== null) installDarwinExitGuard(); defaultLlamaCpp = llm; } +/** + * Peek at the default LlamaCpp instance without instantiating one. Used by + * doctor and lifecycle diagnostics. + */ +export function hasDefaultLlamaCpp(): boolean { + return defaultLlamaCpp !== null; +} + /** * Dispose the default LlamaCpp instance if it exists. * Call this before process exit to prevent NAPI crashes. diff --git a/src/test-preload.ts b/src/test-preload.ts index afbd81f..c2e31e2 100644 --- a/src/test-preload.ts +++ b/src/test-preload.ts @@ -4,6 +4,42 @@ * Uses bun:test afterAll to properly dispose of llama.cpp Metal * resources before the process exits, avoiding GGML_ASSERT failures. */ + +// Mirror bin/qmd's darwin Metal residency mitigation so `bun test` and +// `vitest` runs exit cleanly. The test runners load node-llama-cpp directly +// without going through the launcher, so the libggml-metal static destructor +// asserts on a non-empty residency set during __cxa_finalize_ranges and dumps +// a multi-kB backtrace at process exit (ggml-org/llama.cpp#17869). Opt back +// in with QMD_METAL_KEEP_RESIDENCY=1 if you're triaging the upstream Metal +// teardown bug. +// +// Two-step propagation, because: +// - Native code in libggml-metal reads via C getenv() at module load time. +// - Node syncs process.env mutations to libc via uv_os_setenv automatically. +// - Bun does NOT — `process.env.X = "1"` only updates the JS-level object, +// so getenv() in the C++ binding still sees nothing (verified empirically +// with bun:ffi). We have to call setenv() ourselves on Bun. +if (process.platform === "darwin" && process.env.QMD_METAL_KEEP_RESIDENCY !== "1") { + process.env.GGML_METAL_NO_RESIDENCY = process.env.GGML_METAL_NO_RESIDENCY || "1"; + + if (typeof (globalThis as { Bun?: unknown }).Bun !== "undefined") { + try { + const { dlopen, FFIType, suffix } = await import("bun:ffi"); + const libc = dlopen(`libSystem.${suffix}`, { + setenv: { args: [FFIType.cstring, FFIType.cstring, FFIType.i32], returns: FFIType.i32 }, + }); + libc.symbols.setenv( + Buffer.from("GGML_METAL_NO_RESIDENCY\0", "utf8"), + Buffer.from("1\0", "utf8"), + 1, + ); + } catch { + // FFI unavailable on this Bun build — the backtrace dump at exit is + // cosmetic; tests still pass. + } + } +} + import { afterAll } from "bun:test"; import { disposeDefaultLlamaCpp } from "./llm"; diff --git a/test/cli-exit-lifecycle.test.ts b/test/cli-exit-lifecycle.test.ts index 8558596..5468e0b 100644 --- a/test/cli-exit-lifecycle.test.ts +++ b/test/cli-exit-lifecycle.test.ts @@ -1,9 +1,9 @@ import { describe, expect, test } from "vitest"; import { finishSuccessfulCliCommand } from "../src/cli/qmd.ts"; -import { LlamaCpp } from "../src/llm.ts"; +import { LlamaCpp, isDarwinMetalMitigationActive } from "../src/llm.ts"; describe("CLI successful-exit lifecycle", () => { - test("exits 0 after successful JSON output when post-output LLM cleanup fails", async () => { + test("exits 0 after successful output when post-output LLM cleanup fails", async () => { const exitCodes: number[] = []; const stderr: string[] = []; const flushed: string[] = []; @@ -11,7 +11,6 @@ describe("CLI successful-exit lifecycle", () => { await finishSuccessfulCliCommand({ command: "query", format: "json", - platform: "linux", cleanup: async () => { throw new Error("ggml_metal_device_free abort simulation"); }, @@ -27,27 +26,50 @@ describe("CLI successful-exit lifecycle", () => { expect(flushed).toEqual([""]); }); - test("uses immediate exit for successful macOS JSON query after stdout flush", async () => { + test("flushes stdout then stderr then exits, disposing along the way", async () => { + // After widening the safe-exit into a process-wide guard installed by the + // LlamaCpp constructor, the per-command 'immediate exit' branch is gone: + // every command takes the same flush → dispose → exit(0) path, and the + // darwin guard catches the C++ static dtor crash at process-exit time. const calls: string[] = []; await finishSuccessfulCliCommand({ command: "query", format: "json", - platform: "darwin", - cleanup: async () => { - calls.push("cleanup"); - }, - exit: (code) => { - calls.push(`exit:${code}`); - }, - immediateExit: (code) => { - calls.push(`immediate-exit:${code}`); - }, + cleanup: async () => { calls.push("cleanup"); }, + exit: (code) => { calls.push(`exit:${code}`); }, stdout: { write: (_chunk: string | Uint8Array, cb?: (error?: Error | null) => void) => { calls.push("stdout-flush"); cb?.(); return true; } }, stderr: { write: (_chunk: string | Uint8Array, cb?: (error?: Error | null) => void) => { calls.push("stderr-flush"); cb?.(); return true; } }, }); - expect(calls).toEqual(["stdout-flush", "stderr-flush", "immediate-exit:0"]); + expect(calls).toEqual(["stdout-flush", "cleanup", "stderr-flush", "exit:0"]); + }); + + test("darwin Metal mitigation reflects launcher-exported env on darwin", () => { + // The real mitigation lives in bin/qmd, which sets GGML_METAL_NO_RESIDENCY=1 + // before Node loads the llama.cpp native binding. The JS-side predicate + // just reports whether that env was set (and not overridden by + // QMD_METAL_KEEP_RESIDENCY). On non-darwin the function returns false. + const expected = + process.platform === "darwin" && + process.env.QMD_METAL_KEEP_RESIDENCY !== "1" && + process.env.GGML_METAL_NO_RESIDENCY === "1"; + expect(isDarwinMetalMitigationActive()).toBe(expected); + }); + + test("QMD_METAL_KEEP_RESIDENCY=1 disables the mitigation even when GGML_METAL_NO_RESIDENCY is set", () => { + const prevKeep = process.env.QMD_METAL_KEEP_RESIDENCY; + const prevNoRes = process.env.GGML_METAL_NO_RESIDENCY; + try { + process.env.QMD_METAL_KEEP_RESIDENCY = "1"; + process.env.GGML_METAL_NO_RESIDENCY = "1"; + expect(isDarwinMetalMitigationActive()).toBe(false); + } finally { + if (prevKeep === undefined) delete process.env.QMD_METAL_KEEP_RESIDENCY; + else process.env.QMD_METAL_KEEP_RESIDENCY = prevKeep; + if (prevNoRes === undefined) delete process.env.GGML_METAL_NO_RESIDENCY; + else process.env.GGML_METAL_NO_RESIDENCY = prevNoRes; + } }); test("disposes Llama resources in dependency order before CLI exit", async () => { diff --git a/test/cli.test.ts b/test/cli.test.ts index 40484b3..5f4e138 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -639,7 +639,7 @@ describe("CLI Status Command", () => { QMD_EMBED_CONTEXT_SIZE: "1024", QMD_EDITOR_URI: "vscode://file/{file}:{line}:{col}", QMD_SKILLS_DIR: "/tmp/qmd-skills", - QMD_DISABLE_DARWIN_QUERY_JSON_SAFE_EXIT: "1", + QMD_METAL_KEEP_RESIDENCY: "1", NO_COLOR: "1", CI: "1", HF_ENDPOINT: "https://hf-mirror.com",