FN-7709: unref background integrity-check spawn and scheduling timer
Prevents short-lived CLI processes from being held open by background SQLite integrity checks. - unref the sqlite3 child process (and its stdio) spawned by integrityCheckSqliteFileAsync via the shared unrefQmdChildProcess helper, immediately after spawn - unref the 60s scheduling timer in scheduleBackgroundIntegrityCheck so a short-lived caller isn't pinned waiting for a background check it never asked to block on - add regression test coverage (db-integrity-check-unref.test.ts) plus a CLI fixture (db-integrity-check-fixture.mjs) that exercises the fix in a real short-lived process - add changeset documenting the fix and the audit of other spawn sites across @fusion/core/@fusion/engine/@fusion/dashboard/cli confirming they are safe Files changed: .changeset/fn-7709-db-integrity-check-unref.md | 7 ++ .../src/__tests__/db-integrity-check-unref.test.ts | 135 +++++++++++++++++++++ .../fixtures/db-integrity-check-fixture.mjs | 28 +++++ packages/core/src/db.ts | 26 ++++ 4 files changed, 196 insertions(+) Fusion-Task-Id: FN-7709 Fusion-Task-Lineage: 6594aca4-0268-4bba-9a7f-af96d695f1e9 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7709-db-integrity-check-unref.md
Normal file
7
.changeset/fn-7709-db-integrity-check-unref.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Fix background SQLite integrity checks holding short-lived CLI commands open unnecessarily.
|
||||
category: fix
|
||||
dev: `integrityCheckSqliteFileAsync`'s spawned `sqlite3` child (+ stdio) is now unref'd via the shared `unrefQmdChildProcess` helper, and `scheduleBackgroundIntegrityCheck`'s 60s scheduling timer is now `.unref()`'d, so a short-lived process (e.g. a `fn` one-shot CLI command) that opens a disk-backed `Database` exits promptly instead of being pinned by the background integrity check (FN-7706/FN-7707-class leak). Audited every other non-FN-7708 inline spawn site across `@fusion/core`/`@fusion/engine`/`@fusion/dashboard`/cli and found them SAFE (synchronous, awaited-as-own-work, or intentionally-tracked persistent processes) — see FN-7709's audit document.
|
||||
135
packages/core/src/__tests__/db-integrity-check-unref.test.ts
Normal file
135
packages/core/src/__tests__/db-integrity-check-unref.test.ts
Normal file
@@ -0,0 +1,135 @@
|
||||
/**
|
||||
* FNXC:Database 2026-07-08-00:00:
|
||||
* Regression coverage for FN-7709: the background sqlite3 integrity-check child
|
||||
* spawned by `integrityCheckSqliteFileAsync` (reached fire-and-forget from
|
||||
* `scheduleBackgroundIntegrityCheck`) must be unref'd (child + stdio), and the
|
||||
* 60s scheduling `setTimeout` in `scheduleBackgroundIntegrityCheck` must be
|
||||
* `.unref()`'d, so a short-lived caller (e.g. a `fn` one-shot CLI command that
|
||||
* opens a disk-backed `Database` and exits without `close()`) is not pinned
|
||||
* alive by either handle. Two layers, per FN-5048 (prefer bounded
|
||||
* fixtures/fake timers over real multi-second waits):
|
||||
* 1. A fast fake-timer unit test asserting the 60s scheduling timer is
|
||||
* unref'd (`Timeout#hasRef()`) immediately after `init()` schedules it —
|
||||
* no real 60s wait.
|
||||
* 2. An end-to-end symptom test: a fixture Node process fires
|
||||
* `integrityCheckSqliteFileAsync` (fire-and-forget, mirroring the real
|
||||
* call site) against a slow fake `sqlite3` stub on PATH and must exit
|
||||
* well before the stub does.
|
||||
*/
|
||||
import { describe, it, expect, afterEach, vi } from "vitest";
|
||||
import { spawn } from "node:child_process";
|
||||
import { mkdtempSync, rmSync, writeFileSync, chmodSync, closeSync, openSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { createRequire } from "node:module";
|
||||
import { Database } from "../db.js";
|
||||
|
||||
const tsxPackageJsonPath = createRequire(import.meta.url).resolve("tsx/package.json");
|
||||
const tsxCliPath = join(tsxPackageJsonPath, "..", "dist", "cli.mjs");
|
||||
|
||||
describe("scheduleBackgroundIntegrityCheck 60s timer is unref'd (unit)", () => {
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("the shared scheduling timer is unref'd right after init() so it can't pin a short-lived caller alive", () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const freshDir = mkdtempSync(join(tmpdir(), "fn-7709-db-unref-"));
|
||||
tempDirs.push(freshDir);
|
||||
const freshFusionDir = join(freshDir, ".fusion");
|
||||
const freshDb = new Database(freshFusionDir);
|
||||
|
||||
try {
|
||||
freshDb.init();
|
||||
|
||||
const shared = (
|
||||
Database as unknown as {
|
||||
sharedIntegrityChecks: Map<string, { timer: { hasRef?: () => boolean } | null }>;
|
||||
}
|
||||
).sharedIntegrityChecks.get((freshDb as unknown as { dbPath: string }).dbPath);
|
||||
|
||||
expect(shared?.timer).toBeTruthy();
|
||||
// A ref'd (default) Node Timeout reports hasRef() === true; our fix must
|
||||
// flip this to false immediately after scheduling, without waiting for
|
||||
// the 60s delay to elapse.
|
||||
expect(shared?.timer?.hasRef?.()).toBe(false);
|
||||
} finally {
|
||||
freshDb.close();
|
||||
}
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("integrityCheckSqliteFileAsync does not keep a short-lived caller alive (symptom)", () => {
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
function writeSlowSqlite3Stub(stubDir: string): void {
|
||||
// Fake `sqlite3`: sleeps well past our exit-bound assertion regardless of
|
||||
// args — models a disk-stalled / slow integrity-check walk without a real
|
||||
// multi-second wait dominating the test budget on the assertion side; only
|
||||
// the *stub* sleeps long, the *test* just measures how fast the fixture
|
||||
// process exits.
|
||||
const stubPath = join(stubDir, "sqlite3");
|
||||
writeFileSync(stubPath, ["#!/usr/bin/env bash", "sleep 8", "exit 0", ""].join("\n"), "utf8");
|
||||
chmodSync(stubPath, 0o755);
|
||||
}
|
||||
|
||||
async function runFixture(dbPath: string, stubDir: string) {
|
||||
const fixturePath = join(import.meta.dirname, "fixtures", "db-integrity-check-fixture.mjs");
|
||||
const startedAt = Date.now();
|
||||
return new Promise<{ code: number | null; elapsedMs: number; stdout: string }>((resolvePromise, reject) => {
|
||||
let stdout = "";
|
||||
const child = spawn(process.execPath, [tsxCliPath, fixturePath, dbPath], {
|
||||
env: {
|
||||
...process.env,
|
||||
PATH: `${stubDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? ""}`,
|
||||
},
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
|
||||
child.stdout.on("data", (chunk) => {
|
||||
stdout += String(chunk);
|
||||
});
|
||||
child.on("error", reject);
|
||||
child.on("exit", (code) => {
|
||||
resolvePromise({ code, elapsedMs: Date.now() - startedAt, stdout });
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
it("fixture process exits promptly even though the background sqlite3 stub is still sleeping", async () => {
|
||||
const stubDir = mkdtempSync(join(tmpdir(), "fn-7709-sqlite3-stub-"));
|
||||
tempDirs.push(stubDir);
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-7709-sqlite3-root-"));
|
||||
tempDirs.push(rootDir);
|
||||
writeSlowSqlite3Stub(stubDir);
|
||||
|
||||
// integrityCheckSqliteFileAsync only requires the path to exist — an empty
|
||||
// file is enough since the real check never runs (the stub short-circuits
|
||||
// it) and we only assert on process exit timing here.
|
||||
const dbPath = join(rootDir, "fusion.db");
|
||||
closeSync(openSync(dbPath, "w"));
|
||||
|
||||
const exitInfo = await runFixture(dbPath, stubDir);
|
||||
|
||||
expect(exitInfo.stdout).toContain("db-integrity-check-fixture:scheduled");
|
||||
expect(exitInfo.code).toBe(0);
|
||||
// The fake sqlite3 sleeps 8s; the fixture process must exit well before
|
||||
// that, proving the background child + stdio were unref'd rather than
|
||||
// holding the fixture's event loop open for the stub's full runtime.
|
||||
expect(exitInfo.elapsedMs).toBeLessThan(5_000);
|
||||
}, 15_000);
|
||||
});
|
||||
@@ -0,0 +1,28 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* FNXC:Database 2026-07-08-00:00:
|
||||
* Symptom-verification fixture for FN-7709. Fires
|
||||
* `integrityCheckSqliteFileAsync` (fire-and-forget, mirroring how
|
||||
* `scheduleBackgroundIntegrityCheck` invokes it in production) against
|
||||
* whatever `sqlite3` resolves on PATH, then returns from main immediately.
|
||||
* If the async spawn path does not unref its child + stdio, this process
|
||||
* stays alive until the background `sqlite3` child exits (or is
|
||||
* force-killed by the AbortSignal timeout) instead of exiting on its own
|
||||
* once this script's own work is done. Loaded via `tsx` so it can import
|
||||
* the real TypeScript source directly (no separate build step required).
|
||||
*/
|
||||
import { integrityCheckSqliteFileAsync } from "../../db.ts";
|
||||
|
||||
const dbPath = process.argv[2];
|
||||
if (!dbPath) {
|
||||
throw new Error("db-integrity-check-fixture: missing dbPath argument");
|
||||
}
|
||||
|
||||
// Intentionally NOT awaited — mirrors `scheduleBackgroundIntegrityCheck`'s
|
||||
// `void (async () => { await integrityCheckSqliteFileAsync(...) })()` fire-and-forget
|
||||
// call site in db.ts.
|
||||
void integrityCheckSqliteFileAsync(dbPath);
|
||||
|
||||
// Print a marker so the test can confirm the fixture actually reached this
|
||||
// point (i.e. the call itself did not throw synchronously) before exiting.
|
||||
console.log("db-integrity-check-fixture:scheduled");
|
||||
@@ -13,6 +13,7 @@ import { basename, dirname, isAbsolute, join } from "node:path";
|
||||
import { mkdirSync, existsSync, statSync, renameSync, rmSync } from "node:fs";
|
||||
import { spawn, spawnSync } from "node:child_process";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { unrefQmdChildProcess } from "./memory-backend.js";
|
||||
import { DEFAULT_PROJECT_SETTINGS } from "./types.js";
|
||||
import type { PluginOnSchemaInit } from "./plugin-types.js";
|
||||
import type { SteeringComment, TaskComment } from "./types.js";
|
||||
@@ -1798,6 +1799,18 @@ export function quickCheckSqliteFile(dbPath: string): { ok: boolean; verified: b
|
||||
* would strand the background scheduler's shared entry and pin every
|
||||
* participant's `integrityCheckPending` true forever. AbortSignal.timeout's
|
||||
* internal timer is unref'd, so it never keeps the process alive on shutdown.
|
||||
*
|
||||
* FNXC:Database 2026-07-08-00:00:
|
||||
* This spawn is reached fire-and-forget from `scheduleBackgroundIntegrityCheck`'s
|
||||
* `void (async () => …)()` — nothing here is guaranteed to be awaited by a live
|
||||
* caller. A short-lived process (e.g. a `fn` one-shot CLI command) that opens a
|
||||
* disk-backed `Database` and exits before the 60s scheduling delay elapses must
|
||||
* not be pinned alive by this child's stdio pipes. Unref the child (+ stdio)
|
||||
* immediately after spawn via the shared `unrefQmdChildProcess` helper — see its
|
||||
* doc comment in `memory-backend.ts` for why a plain `spawn()` (not
|
||||
* `promisify(execFile)`) is required for the unref to actually stick (FN-7706).
|
||||
* This does not affect resolve/reject semantics: long-lived callers that DO stay
|
||||
* alive still see the promise settle normally on `close`/`error` (FN-7709).
|
||||
*/
|
||||
const INTEGRITY_CHECK_TIMEOUT_MS = 5 * 60 * 1000;
|
||||
|
||||
@@ -1823,6 +1836,11 @@ export function integrityCheckSqliteFileAsync(
|
||||
// option-validation errors, e.g. an already-aborted signal.)
|
||||
signal: AbortSignal.timeout(INTEGRITY_CHECK_TIMEOUT_MS),
|
||||
});
|
||||
// FNXC:Database 2026-07-08-00:00: unref synchronously right after spawn — a
|
||||
// manual unref later (e.g. inside a `data`/`close` handler) would race the
|
||||
// execFile-style nextTick stdio re-ref; see the doc comment above this
|
||||
// function (FN-7709 / FN-7706).
|
||||
unrefQmdChildProcess(child);
|
||||
} catch {
|
||||
resolve({ ok: true, verified: false });
|
||||
return;
|
||||
@@ -5914,6 +5932,14 @@ export class Database {
|
||||
Database.sharedIntegrityChecks.delete(this.dbPath);
|
||||
});
|
||||
}, 60_000);
|
||||
// FNXC:Database 2026-07-08-00:00: unref the 60s scheduling delay so a
|
||||
// short-lived caller (e.g. a `fn` one-shot CLI command that opens a
|
||||
// disk-backed Database and exits before this fires) is not pinned alive
|
||||
// waiting for a background check it never asked to block on (FN-7709).
|
||||
// The check itself still fires normally for any process that stays alive
|
||||
// past the delay — unref only affects whether the handle keeps the event
|
||||
// loop open, not whether/when the timer callback runs.
|
||||
shared.timer.unref?.();
|
||||
|
||||
Database.sharedIntegrityChecks.set(this.dbPath, shared);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user