Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**Fixed** Ctrl-C now ends a `dirsql` run that is still in progress through the npm launcher. The launcher's `process.on("SIGINT")` listener could never fire during a scan: the addon's `runCli` export is synchronous, so it blocks the event loop for the whole of the core's run and the listener is only reached once the work has already finished. The addon now drops SIGINT and SIGTERM to their default disposition for that window, which the kernel acts on unaided, matching the standalone binary. `dirsql server`'s graceful shutdown still exits 0 — the core registers its own handler over the default once it starts.
7 changes: 7 additions & 0 deletions packages/ts/e2e-attestations/claude-1166-ts-sigint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"command": "pnpm test:e2e",
"ran_at": 1789916885,
"exit_code": 0,
"commit": "618f340fed1f91cb42689de96f7bc5b0d9d1fd5b",
"branch": "claude/1166-ts-sigint"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
### SIGINT ends a run in progress through the npm launcher

**Summary**

The npm launcher registered JS listeners for SIGINT and SIGTERM, but the napi `runCli` export is synchronous and blocks the event loop for the whole of the core's run, so neither listener could run while a scan was in progress. A signal arriving then was absorbed and the process could only be ended with SIGKILL. The addon now sets both signals to their default disposition for the duration of the core's run and restores the listeners' disposition after. No API changes; the observable difference is that a signal that used to do nothing now ends the process with the usual 128+signal status.

**Required changes**

| Before | After |
| --- | --- |
| `kill -INT <pid>` during a query: absorbed; the run continues to completion | `kill -INT <pid>` during a query: the process dies of SIGINT (shell reports `130`) |
| `kill -TERM <pid>` during a query: absorbed; SIGKILL was the only way out | `kill -TERM <pid>` during a query: the process dies of SIGTERM (shell reports `143`) |

A script that sent SIGINT to a `dirsql` process and relied on it being ignored must stop sending it. A script that escalated to SIGKILL after an absorbed signal can drop the escalation.

`dirsql server` is unchanged: its graceful shutdown still exits `0` on SIGINT and SIGTERM.

**Deprecations removed**

_None._

**Behavior changes without code changes**

The exit status of an interrupted run changes from "no exit" to death by the signal. This aligns the npm launcher with the standalone `dirsql` binary and with the Python launcher.

**Verification**

Against a directory large enough to take a few seconds to scan:

```bash
npx dirsql "SELECT count(*) FROM './'" # press Ctrl-C while it runs
echo $?
```

Expected: the process ends promptly and the shell reports `130`. Before this change it kept running and the signal was absorbed. Run it in the foreground: a non-interactive shell sets SIGINT to `SIG_IGN` for `&` background jobs, which hides the difference.
1 change: 1 addition & 0 deletions packages/ts/napi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ crate-type = ["cdylib"]
# calls it in-process instead of spawning a bundled binary (#739), which is
# what lets the `@dirsql/cli-*` package family go away.
dirsql = { workspace = true, features = ["cli"] }
libc = "0.2"
napi = { version = "3", features = ["napi6"] }
napi-derive = "3"

Expand Down
36 changes: 35 additions & 1 deletion packages/ts/napi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,41 @@ pub fn run_cli(argv: Vec<String>) -> i32 {
let mut full = Vec::with_capacity(argv.len() + 1);
full.push("dirsql".to_string());
full.extend(argv);
dirsql::cli::run_cli(full)
with_default_signal_disposition(|| dirsql::cli::run_cli(full))
}

const FATAL_SIGNALS: [i32; 2] = [libc::SIGINT, libc::SIGTERM];

/// Run `f` with SIGINT/SIGTERM at `SIG_DFL`, restoring the prior disposition
/// after.
///
/// The launcher's JS listeners cannot fire while this call blocks the event
/// loop, so without this the signal is absorbed and a scan becomes
/// SIGKILL-only. `server` still exits gracefully: the core registers its own
/// handler over `SIG_DFL` once it starts.
#[cfg(unix)]
fn with_default_signal_disposition<T>(f: impl FnOnce() -> T) -> T {
#[expect(
unsafe_code,
reason = "no safe API reaches a signal disposition; the raw call is the whole point"
)]
let saved: Vec<_> = FATAL_SIGNALS
.iter()
.map(|&sig| (sig, unsafe { libc::signal(sig, libc::SIG_DFL) }))
.collect();
let out = f();
for (sig, prev) in saved {
#[expect(unsafe_code, reason = "restores what the call above saved")]
unsafe {
libc::signal(sig, prev);
}
}
out
}

#[cfg(not(unix))]
fn with_default_signal_disposition<T>(f: impl FnOnce() -> T) -> T {
f()
}

/// The config paths the npm launcher must inspect for package-name
Expand Down
7 changes: 4 additions & 3 deletions packages/ts/src/cli/keep-signals-fatal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
* is the whole remedy — no `unsafe`, no `sigaction`.
*/
export function keepSignalsFatal(): void {
// While the core is running, its own handler drives shutdown and this
// listener is the tail of the chain; it matters for signals arriving
// outside that window, where exiting is exactly right.
// This listener cannot fire while the core runs — the napi call blocks the
// event loop — so the addon drops to `SIG_DFL` for that window.
// Registering here is still what gives signal-hook a disposition to chain
// to, and it handles signals arriving outside that window.
process.on("SIGINT", () => process.exit(130));
process.on("SIGTERM", () => process.exit(143));
}
90 changes: 90 additions & 0 deletions packages/ts/tests/e2e/sigint-interrupt.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// CLI e2e: SIGINT ends a run that is still in progress.
//
// Spawns the real launcher over a real config whose `on-file` hook blocks, so
// the core is provably mid-scan when the signal lands, then asserts the
// process dies of it. Nothing is mocked: real launcher, real process, real
// filesystem, real core.

import { spawn } from "node:child_process";
import { existsSync } from "node:fs";
import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { fileURLToPath } from "node:url";
import { afterEach, beforeEach, describe, expect, it } from "vitest";

const __dirname = dirname(fileURLToPath(import.meta.url));
const PKG_ROOT = join(__dirname, "..", "..");

const PKG: { bin: { dirsql: string } } = JSON.parse(
await readFile(join(PKG_ROOT, "package.json"), "utf8"),
);
const CLI_ENTRY = join(PKG_ROOT, PKG.bin.dirsql);

async function waitUntilScanning(ready: string, timeoutMs = 30_000) {
const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
if (existsSync(ready)) {
return;
}
await delay(50);
}
throw new Error("the scan never reached the on-file hook");
}

describe("SIGINT during a scan (CLI)", () => {
let dir: string;
let configPath: string;
let ready: string;

beforeEach(async () => {
dir = await mkdtemp(join(tmpdir(), "dirsql-sigint-e2e-"));
configPath = join(dir, ".dirsql.toml");
ready = join(dir, "scanning");
await mkdir(join(dir, "data"), { recursive: true });
await writeFile(join(dir, "data", "a.txt"), "hello");
await writeFile(
configPath,
`
[[table]]
name = "files"
ddl = "CREATE TABLE files (path TEXT)"
glob = "data/*.txt"
on-file = "sh -c 'touch ${ready}; sleep 120; printf \\"[]\\"'"
`,
);
});

afterEach(async () => {
await rm(dir, { recursive: true, force: true });
});

it("kills the process", async () => {
const proc = spawn(
process.execPath,
[CLI_ENTRY, "query", "SELECT * FROM files", "--config", configPath],
{ cwd: dir, detached: true, stdio: ["ignore", "ignore", "ignore"] },
);
const died = new Promise<[number | null, NodeJS.Signals | null]>(
(resolve) => proc.on("close", (code, signal) => resolve([code, signal])),
);

try {
await waitUntilScanning(ready);
proc.kill("SIGINT");
const settled = await Promise.race([
died,
delay(15_000).then(() => "still running" as const),
]);
expect(settled).toEqual([null, "SIGINT"]);
} finally {
const pid = proc.pid;
if (pid && proc.exitCode === null && proc.signalCode === null) {
// The blocking hook is a child in the same group; take the group.
process.kill(-pid, "SIGKILL");
await died;
}
}
}, 60_000);
});
Loading