Skip to content

Commit db8604b

Browse files
zahariashclaude
authored andcommitted
fix(pybind): let Ctrl-C interrupt a running query
Python runs its SIGINT handler only between bytecodes. While the main thread waited in the engine for query(), execute() or query_as_arrow(), Ctrl-C therefore took effect only after the query had finished: a 15 s query ran to the end before KeyboardInterrupt appeared. During such a wait on the main thread, a scoped SIGINT handler now interrupts the connection's query and then calls Python's own handler. Once the call returns, the pending signal is raised as KeyboardInterrupt instead of the engine's "Interrupted." error, and the connection stays usable. The handler is installed only while Python's default handler (KeyboardInterrupt) is active, so a custom SIGINT handler keeps its behaviour, and only on POSIX systems. That check goes through the driver's import cache and never fails a query, e.g. one run during interpreter shutdown. A Ctrl-C that lands while the query is still compiling is dropped by the engine, as any interrupt is; a second Ctrl-C stops the query. The engine interrupts a connection, not a query: if the main thread's call is waiting behind another thread's query on the same connection, Ctrl-C stops that query instead, and the main thread's query runs before KeyboardInterrupt is raised. The new tests send SIGINT one second into a long query through query(), execute() with parameters and query_as_arrow(), and expect KeyboardInterrupt within seconds; they fail on main. A custom handler test checks that the query then still finishes and the handler runs. The C-API backend calls the engine through ctypes and is not covered, so the interrupt tests are listed in capi_xfails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7bcc788 commit db8604b

5 files changed

Lines changed: 199 additions & 0 deletions

File tree

‎src_cpp/include/cached_import/py_cached_import.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ class PythonCachedImport {
2828
PandasCachedItem pandas;
2929
PolarsCachedItem polars;
3030
PyarrowCachedItem pyarrow;
31+
SignalCachedItem signal;
32+
ThreadingCachedItem threading;
3133
UUIDCachedItem uuid;
3234

3335
private:

‎src_cpp/include/cached_import/py_cached_modules.h‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,24 @@ class PyarrowCachedItem : public PythonCachedItem {
154154
LibCachedItem lib;
155155
};
156156

157+
class SignalCachedItem : public PythonCachedItem {
158+
public:
159+
SignalCachedItem()
160+
: PythonCachedItem("signal"), getsignal("getsignal", this), sigint("SIGINT", this),
161+
default_int_handler("default_int_handler", this) {}
162+
163+
PythonCachedItem getsignal;
164+
PythonCachedItem sigint;
165+
PythonCachedItem default_int_handler;
166+
};
167+
168+
class ThreadingCachedItem : public PythonCachedItem {
169+
public:
170+
ThreadingCachedItem() : PythonCachedItem("threading"), main_thread("main_thread", this) {}
171+
172+
PythonCachedItem main_thread;
173+
};
174+
157175
class UUIDCachedItem : public PythonCachedItem {
158176
public:
159177
UUIDCachedItem() : PythonCachedItem("uuid"), UUID("UUID", this) {}

‎src_cpp/py_connection.cpp‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
#include "include/py_connection.h"
22

33
#include <algorithm>
4+
#include <atomic>
45
#include <cctype>
56
#include <utility>
67

8+
#ifndef _WIN32
9+
#include <csignal>
10+
#endif
11+
712
#include "cached_import/py_cached_import.h"
813
#include "common/constants.h"
914
#include "common/exception/not_implemented.h"
@@ -183,34 +188,125 @@ void PyConnection::interrupt() {
183188
static std::unordered_map<std::string, std::unique_ptr<Value>> transformPythonParameters(
184189
const py::dict& params, Connection* conn);
185190

191+
namespace {
192+
193+
// Python runs its SIGINT handler only between bytecodes, so not while the main thread waits in
194+
// the engine. During such a wait, SIGINT interrupts the connection's query and then calls
195+
// Python's handler. This is only done on the main thread while Python's default handler
196+
// (KeyboardInterrupt) is active, so custom handlers keep their behaviour.
197+
class SigintInterruptsQuery {
198+
public:
199+
explicit SigintInterruptsQuery([[maybe_unused]] main::ClientContext* context) {
200+
#ifndef _WIN32
201+
if (target.load() != nullptr || !onMainThreadWithDefaultHandler()) {
202+
return;
203+
}
204+
if (sigaction(SIGINT, nullptr, &previous) != 0) {
205+
return;
206+
}
207+
struct sigaction action {};
208+
action.sa_sigaction = handle;
209+
action.sa_flags = SA_SIGINFO | (previous.sa_flags & SA_ONSTACK);
210+
sigemptyset(&action.sa_mask);
211+
received = false;
212+
target = context;
213+
installed = sigaction(SIGINT, &action, nullptr) == 0;
214+
if (!installed) {
215+
target = nullptr;
216+
}
217+
#endif
218+
}
219+
220+
~SigintInterruptsQuery() {
221+
#ifndef _WIN32
222+
if (installed) {
223+
sigaction(SIGINT, &previous, nullptr);
224+
target = nullptr;
225+
}
226+
#endif
227+
}
228+
229+
// Call with the GIL held once the query has returned.
230+
void raiseIfInterrupted() const {
231+
#ifndef _WIN32
232+
if (installed && received && PyErr_CheckSignals() != 0) {
233+
throw py::error_already_set();
234+
}
235+
#endif
236+
}
237+
238+
private:
239+
#ifndef _WIN32
240+
static bool onMainThreadWithDefaultHandler() {
241+
// The check must never fail a query, e.g. once imports stop working at shutdown.
242+
try {
243+
auto mainThread = importCache->threading.main_thread()();
244+
if (mainThread.attr("ident").cast<unsigned long>() != PyThread_get_thread_ident()) {
245+
return false;
246+
}
247+
auto& signal = importCache->signal;
248+
return signal.getsignal()(signal.sigint()).is(signal.default_int_handler());
249+
} catch (py::error_already_set&) {
250+
return false;
251+
}
252+
}
253+
254+
static void handle(int signum, siginfo_t* info, void* ucontext) {
255+
if (auto* context = target.load()) {
256+
context->interrupt();
257+
}
258+
received = true;
259+
// Python's handler only records the signal; KeyboardInterrupt is raised later.
260+
if (previous.sa_flags & SA_SIGINFO) {
261+
previous.sa_sigaction(signum, info, ucontext);
262+
} else if (previous.sa_handler != SIG_DFL && previous.sa_handler != SIG_IGN) {
263+
previous.sa_handler(signum);
264+
}
265+
}
266+
267+
static inline std::atomic<main::ClientContext*> target{nullptr};
268+
static inline std::atomic<bool> received{false};
269+
static inline struct sigaction previous {};
270+
bool installed = false;
271+
#endif
272+
};
273+
274+
} // namespace
275+
186276
std::unique_ptr<PyQueryResult> PyConnection::execute(PyPreparedStatement* preparedStatement,
187277
const py::dict& params) {
188278
auto& stateRef = refState();
189279
if (preparedStatement == nullptr || preparedStatement->state == nullptr) {
190280
throw RuntimeException("Prepared statement is closed.");
191281
}
192282
auto parameters = transformPythonParameters(params, &stateRef.ref());
283+
SigintInterruptsQuery sigint(stateRef.ref().getClientContext());
193284
py::gil_scoped_release release;
194285
auto queryResult =
195286
stateRef.ref().executeWithParams(&preparedStatement->state->ref(), std::move(parameters));
196287
py::gil_scoped_acquire acquire;
288+
sigint.raiseIfInterrupted();
197289
return checkAndWrapQueryResult(queryResult, state);
198290
}
199291

200292
std::unique_ptr<PyQueryResult> PyConnection::query(const std::string& statement) {
201293
auto& stateRef = refState();
294+
SigintInterruptsQuery sigint(stateRef.ref().getClientContext());
202295
py::gil_scoped_release release;
203296
auto queryResult = stateRef.ref().query(statement);
204297
py::gil_scoped_acquire acquire;
298+
sigint.raiseIfInterrupted();
205299
return checkAndWrapQueryResult(queryResult, state);
206300
}
207301

208302
std::unique_ptr<PyQueryResult> PyConnection::queryAsArrow(const std::string& statement,
209303
int64_t chunkSize) {
210304
auto& stateRef = refState();
305+
SigintInterruptsQuery sigint(stateRef.ref().getClientContext());
211306
py::gil_scoped_release release;
212307
auto queryResult = stateRef.ref().queryAsArrow(statement, chunkSize);
213308
py::gil_scoped_acquire acquire;
309+
sigint.raiseIfInterrupted();
214310
return checkAndWrapQueryResult(queryResult, state);
215311
}
216312

‎test/capi_xfails.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@
6969
# C API scan rewriting uses temporary Arrow-backed tables, which cannot be
7070
# created through a read-only connection.
7171
"test/test_scan_pyarrow.py::test_pyarrow_basic",
72+
# Ctrl-C interrupts a running query only on the pybind backend.
73+
"test/test_keyboard_interrupt.py::test_keyboard_interrupt_stops_the_query[execute_with_parameters]",
74+
"test/test_keyboard_interrupt.py::test_keyboard_interrupt_stops_the_query[query]",
75+
"test/test_keyboard_interrupt.py::test_keyboard_interrupt_stops_the_query[query_as_arrow]",
7276
# UDF registration is still routed through pybind.
7377
"test/test_blob_parameter.py::test_bytes_param_udf",
7478
"test/test_udf.py::test_udf",

‎test/test_keyboard_interrupt.py‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
from __future__ import annotations
2+
3+
import subprocess
4+
import sys
5+
import textwrap
6+
from typing import TYPE_CHECKING
7+
8+
import pytest
9+
10+
if TYPE_CHECKING:
11+
from pathlib import Path
12+
13+
# Each scenario sends SIGINT to itself one second into a query that runs for many
14+
# seconds, and reports how the query ended.
15+
PRELUDE = """
16+
import os, signal, sys, threading, time
17+
import ladybug as lb
18+
19+
db = lb.Database(sys.argv[1])
20+
conn = lb.Connection(db)
21+
LONG_QUERY = "WITH range(1, $n) AS r UNWIND r AS x UNWIND r AS y RETURN sum(x * y)"
22+
threading.Timer(1.0, os.kill, (os.getpid(), signal.SIGINT)).start()
23+
"""
24+
25+
SCENARIOS = {
26+
"query": "conn.execute(LONG_QUERY.replace('$n', '40000'))",
27+
"execute_with_parameters": "conn.execute(LONG_QUERY, {'n': 40000})",
28+
"query_as_arrow": "conn.query_as_arrow(LONG_QUERY.replace('$n', '40000'), 1000)",
29+
}
30+
31+
RUN = """
32+
started = time.monotonic()
33+
try:
34+
{call}
35+
print("OUTCOME finished")
36+
except KeyboardInterrupt:
37+
print("OUTCOME KeyboardInterrupt")
38+
print("ELAPSED", time.monotonic() - started)
39+
print("AFTER", conn.execute("RETURN 1").get_next()[0])
40+
"""
41+
42+
CUSTOM_HANDLER = """
43+
received = []
44+
signal.signal(signal.SIGINT, lambda signum, frame: received.append(signum))
45+
result = conn.execute(LONG_QUERY.replace("$n", "20000")).get_next()[0]
46+
time.sleep(0.2)
47+
print("OUTCOME", "finished" if result else "empty", "handled" if received else "missed")
48+
"""
49+
50+
51+
def run_script(tmp_path: Path, body: str) -> str:
52+
script = textwrap.dedent(PRELUDE) + textwrap.dedent(body)
53+
try:
54+
proc = subprocess.run(
55+
[sys.executable, "-c", script, str(tmp_path / "db")],
56+
capture_output=True,
57+
text=True,
58+
timeout=120,
59+
)
60+
except subprocess.TimeoutExpired:
61+
pytest.fail("the query was not interrupted")
62+
assert proc.returncode == 0, proc.stderr[-2000:]
63+
return proc.stdout
64+
65+
66+
@pytest.mark.skipif(sys.platform == "win32", reason="SIGINT is POSIX only")
67+
@pytest.mark.parametrize("scenario", sorted(SCENARIOS))
68+
def test_keyboard_interrupt_stops_the_query(tmp_path: Path, scenario: str) -> None:
69+
stdout = run_script(tmp_path, RUN.format(call=SCENARIOS[scenario]))
70+
assert "OUTCOME KeyboardInterrupt" in stdout, stdout
71+
assert float(stdout.split("ELAPSED")[1].split()[0]) < 10, stdout
72+
assert "AFTER 1" in stdout, stdout
73+
74+
75+
@pytest.mark.skipif(sys.platform == "win32", reason="SIGINT is POSIX only")
76+
def test_custom_sigint_handler_still_runs(tmp_path: Path) -> None:
77+
stdout = run_script(tmp_path, CUSTOM_HANDLER)
78+
# A custom handler is left alone: the query finishes, then the handler runs.
79+
assert "OUTCOME finished handled" in stdout, stdout

0 commit comments

Comments
 (0)