diff --git a/packages/python/changelog.d/2026-09-20-sigint-ends-a-run-in-progress.md b/packages/python/changelog.d/2026-09-20-sigint-ends-a-run-in-progress.md new file mode 100644 index 00000000..1b11cc7e --- /dev/null +++ b/packages/python/changelog.d/2026-09-20-sigint-ends-a-run-in-progress.md @@ -0,0 +1 @@ +**Fixed** Ctrl-C now ends a `dirsql` run that is still in progress. The launcher installed an empty SIGINT handler for the whole of the core's run, and because the core detaches the GIL for that call no Python handler can run until the work has already finished — so a signal sent during a long directory scan was swallowed and the process was SIGTERM-only. SIGINT is left at its default disposition instead, which the kernel acts on immediately, matching the standalone binary. `dirsql server`'s graceful shutdown still exits 0. diff --git a/packages/python/dirsql/cli/main.py b/packages/python/dirsql/cli/main.py index 13220591..5405e75b 100644 --- a/packages/python/dirsql/cli/main.py +++ b/packages/python/dirsql/cli/main.py @@ -18,27 +18,21 @@ from .resolve_config_extensions import with_resolved_extensions -def _absorb_interrupt(*_args: object) -> None: - """Let the core's own shutdown decide the exit code on SIGINT. - - signal-hook (which tokio uses) *chains*: it runs tokio's handler — which - drives `dirsql server`'s graceful shutdown, after which `run_cli` returns - 0 — and then whatever handler was installed before it. CPython's default - is `default_int_handler`, which raises `KeyboardInterrupt`; that lands - after `run_cli` has already returned 0 and turns a clean shutdown into a - 130. This handler occupies that slot without raising, so the core's exit - code is the one that survives, exactly as it does when the CLI is its own - process. - - A signal arriving when the core is NOT handling signals still terminates: - `run_cli` is only reached with this installed, and it returns promptly for - every non-server command. +def keep_sigint_fatal(handler=signal.signal): + """Give SIGINT its default disposition for the core's run, returning the prior one. + + No Python-level handler can end a run in progress: `run_cli` detaches the + GIL for its whole duration, so nothing reaches the eval loop until the + core has already finished — CPython's `default_int_handler` included. Only + a disposition the kernel acts on by itself works, which is `SIG_DFL`, and + it is what the standalone binary runs with. + + `dirsql server` keeps its exit code: signal-hook (which tokio uses) + replaces the disposition when the server registers its handlers, and does + not re-raise `SIG_DFL` afterwards, so the core's graceful shutdown is the + only thing that acts and its 0 survives. """ - - -def with_core_owned_signals(handler=signal.signal): - """Install `_absorb_interrupt` for SIGINT and return the prior handler.""" - return handler(signal.SIGINT, _absorb_interrupt) + return handler(signal.SIGINT, signal.SIG_DFL) def main(argv: list[str] | None = None) -> int: @@ -56,7 +50,7 @@ def main(argv: list[str] | None = None) -> int: print(f"dirsql: {exc}", file=sys.stderr) return 1 - previous = with_core_owned_signals() + previous = keep_sigint_fatal() try: return run_in_process(argv=argv, module="dirsql._dirsql") except Exception as exc: diff --git a/packages/python/dirsql/cli/main_test.py b/packages/python/dirsql/cli/main_test.py index 487d8c7b..bf5ccfe2 100644 --- a/packages/python/dirsql/cli/main_test.py +++ b/packages/python/dirsql/cli/main_test.py @@ -5,11 +5,11 @@ from unittest.mock import patch from . import main as main_module -from .main import _absorb_interrupt, main, with_core_owned_signals +from .main import keep_sigint_fatal, main -def describe_with_core_owned_signals(): - def it_installs_the_absorbing_handler_and_returns_the_previous_one(): +def describe_keep_sigint_fatal(): + def it_installs_the_default_disposition_and_returns_the_previous_handler(): recorded = {} def fake_signal(signum, new): @@ -17,14 +17,9 @@ def fake_signal(signum, new): recorded["new"] = new return "prior-handler" - assert with_core_owned_signals(fake_signal) == "prior-handler" + assert keep_sigint_fatal(fake_signal) == "prior-handler" assert recorded["signum"] == main_module.signal.SIGINT - assert recorded["new"] is _absorb_interrupt - - def it_absorbs_the_interrupt_without_raising(): - # The whole point: it must NOT raise KeyboardInterrupt, or a graceful - # `dirsql server` shutdown (the core returns 0) surfaces as 130. - assert _absorb_interrupt(main_module.signal.SIGINT, None) is None + assert recorded["new"] is main_module.signal.SIG_DFL def describe_main(): @@ -47,9 +42,7 @@ def it_returns_the_cores_exit_code(): with ( patch.object(main_module, "with_discovered_plugins", lambda a: a), patch.object(main_module, "with_resolved_extensions", lambda a: a), - patch.object( - main_module, "with_core_owned_signals", return_value="prior" - ), + patch.object(main_module, "keep_sigint_fatal", return_value="prior"), patch.object(main_module.signal, "signal"), patch.object( main_module, "run_in_process", return_value=23 @@ -72,9 +65,7 @@ def it_discovers_plugins_before_resolving_extensions(): "with_resolved_extensions", lambda a: [*a, "--extension", "/r/vec0"], ), - patch.object( - main_module, "with_core_owned_signals", return_value="prior" - ), + patch.object(main_module, "keep_sigint_fatal", return_value="prior"), patch.object(main_module.signal, "signal"), patch.object( main_module, "run_in_process", return_value=0 @@ -94,9 +85,7 @@ def it_defaults_argv_to_sys_argv_minus_the_program_name(): patch.object(sys, "argv", ["dirsql", "--version"]), patch.object(main_module, "with_discovered_plugins", lambda a: a), patch.object(main_module, "with_resolved_extensions", lambda a: a), - patch.object( - main_module, "with_core_owned_signals", return_value="prior" - ), + patch.object(main_module, "keep_sigint_fatal", return_value="prior"), patch.object(main_module.signal, "signal"), patch.object( main_module, "run_in_process", return_value=0 @@ -111,9 +100,7 @@ def it_returns_1_with_the_message(): with ( patch.object(main_module, "with_discovered_plugins", lambda a: a), patch.object(main_module, "with_resolved_extensions", lambda a: a), - patch.object( - main_module, "with_core_owned_signals", return_value="prior" - ), + patch.object(main_module, "keep_sigint_fatal", return_value="prior"), patch.object(main_module.signal, "signal"), patch.object( main_module, @@ -131,9 +118,7 @@ def it_restores_the_previous_handler_after_the_run(): with ( patch.object(main_module, "with_discovered_plugins", lambda a: a), patch.object(main_module, "with_resolved_extensions", lambda a: a), - patch.object( - main_module, "with_core_owned_signals", return_value="prior" - ), + patch.object(main_module, "keep_sigint_fatal", return_value="prior"), patch.object( main_module.signal, "signal", @@ -149,9 +134,7 @@ def it_restores_the_previous_handler_even_when_the_run_raises(): with ( patch.object(main_module, "with_discovered_plugins", lambda a: a), patch.object(main_module, "with_resolved_extensions", lambda a: a), - patch.object( - main_module, "with_core_owned_signals", return_value="prior" - ), + patch.object(main_module, "keep_sigint_fatal", return_value="prior"), patch.object( main_module.signal, "signal", diff --git a/packages/python/e2e-attestations/claude-1164-sigint-fatal.json b/packages/python/e2e-attestations/claude-1164-sigint-fatal.json new file mode 100644 index 00000000..74dcf923 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1164-sigint-fatal.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1789910327, + "exit_code": 0, + "commit": "dfc68b057d52f3d4f586fd0233f292ccc2062dd6", + "branch": "claude/1164-sigint-fatal" +} diff --git a/packages/python/migrations.d/2026-09-20-sigint-ends-a-run-in-progress.md b/packages/python/migrations.d/2026-09-20-sigint-ends-a-run-in-progress.md new file mode 100644 index 00000000..d8764c16 --- /dev/null +++ b/packages/python/migrations.d/2026-09-20-sigint-ends-a-run-in-progress.md @@ -0,0 +1,35 @@ +### SIGINT ends a run in progress through the Python launcher + +**Summary** + +The `dirsql` console script installed an empty SIGINT handler for the duration of the core's run. A signal arriving during a long directory scan was therefore discarded, and the process could only be ended with SIGTERM. SIGINT is now left at its default disposition for that window, so the kernel terminates the process immediately. No API changes; the observable difference is that a signal that used to do nothing now ends the process with the usual 128+SIGINT status. + +**Required changes** + +| Before | After | +| --- | --- | +| `kill -INT ` during a query: ignored; the run continues to completion | `kill -INT ` during a query: the process dies of SIGINT (shell reports `130`) | +| Ctrl-C while the REPL is executing a statement: ignored | Ctrl-C while the REPL is executing a statement: the session ends | + +A script that sent SIGINT to a `dirsql` process and relied on it being ignored must stop sending it. A script that escalated to SIGTERM after an ignored SIGINT can drop the escalation. + +`dirsql server` is unchanged: its graceful shutdown still exits `0` on SIGINT and SIGTERM. Ctrl-C at an idle REPL prompt is unchanged too — reedline reads it as a key event, not a signal. + +**Deprecations removed** + +_None._ + +**Behavior changes without code changes** + +The exit status of an interrupted run changes from "no exit" to death by SIGINT. This aligns the Python launcher with the standalone `dirsql` binary, which has always behaved this way. + +**Verification** + +Against a directory large enough to take a few seconds to scan: + +```bash +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 SIGINT was discarded. Run it in the foreground: a non-interactive shell sets SIGINT to `SIG_IGN` for `&` background jobs, which hides the difference. diff --git a/packages/python/tests/e2e/sigint_interrupt_test.py b/packages/python/tests/e2e/sigint_interrupt_test.py new file mode 100644 index 00000000..aadf43d7 --- /dev/null +++ b/packages/python/tests/e2e/sigint_interrupt_test.py @@ -0,0 +1,75 @@ +"""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. No mocks: real process, real filesystem, real core. +""" + +from __future__ import annotations + +import os +import signal +import subprocess +import sys +import time + +import pytest + +_LAUNCH = "import sys; from dirsql.cli.main import main; sys.exit(main())" + + +def _wait_until_scanning(proc, ready, timeout=30.0): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if ready.exists(): + return + assert proc.poll() is None, "the run exited before the scan began" + time.sleep(0.05) + raise AssertionError("the scan never reached the on-file hook") + + +def describe_sigint_during_a_scan(): + @pytest.fixture + def scanning(tmp_path): + root = tmp_path / "data" + root.mkdir() + (root / "a.txt").write_text("hello") + ready = tmp_path / "scanning" + cfg = root / ".dirsql.toml" + cfg.write_text( + "[[table]]\n" + 'name = "files"\n' + 'ddl = "CREATE TABLE files (path TEXT)"\n' + 'glob = "*.txt"\n' + f'on-file = "sh -c \'touch {ready}; sleep 120; printf \\"[]\\"\'"\n' + ) + + proc = subprocess.Popen( + [ + sys.executable, + "-c", + _LAUNCH, + "--config", + str(cfg), + "SELECT * FROM files", + ], + cwd=str(root), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + start_new_session=True, + ) + try: + yield proc, ready + finally: + try: + os.killpg(proc.pid, signal.SIGKILL) + except OSError: + pass + proc.wait(timeout=10) + + def it_kills_the_process(scanning): + proc, ready = scanning + _wait_until_scanning(proc, ready) + proc.send_signal(signal.SIGINT) + assert proc.wait(timeout=15) == -signal.SIGINT