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
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. 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.
36 changes: 15 additions & 21 deletions packages/python/dirsql/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
39 changes: 11 additions & 28 deletions packages/python/dirsql/cli/main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,21 @@
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):
recorded["signum"] = signum
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():
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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",
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"command": "just test-e2e",
"ran_at": 1789910327,
"exit_code": 0,
"commit": "dfc68b057d52f3d4f586fd0233f292ccc2062dd6",
"branch": "claude/1164-sigint-fatal"
}
Original file line number Diff line number Diff line change
@@ -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 <pid>` during a query: ignored; the run continues to completion | `kill -INT <pid>` 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.
75 changes: 75 additions & 0 deletions packages/python/tests/e2e/sigint_interrupt_test.py
Original file line number Diff line number Diff line change
@@ -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
Loading