Skip to content

Commit 60d8c75

Browse files
zahariashclaude
authored andcommitted
fix(connection): make close() safe while a query is running
Closing a connection while another thread was executing a query on it destroyed the native connection under the running query. Depending on the query this hung the process, aborted it with heap corruption, or segfaulted (a write). AsyncConnection.close() hit the same path, since it closes its connections before shutting down its executor. Connection now counts calls in flight in every method that reaches the native connection. close() refuses new calls, interrupts a running query (repeating the interrupt until the call returns, because an interrupt that arrives while the query is still compiling is cleared when execution starts), and waits for in-flight calls to leave the engine before releasing the native connection. Calls made after close() raise "Connection is closed." The guard lives in the Python layer, so it covers both backends. close() called from inside one of the connection's own calls on the same thread raises instead of waiting for itself. A Python UDF that calls into a connection while it runs a query (close(), execute() or any other call) raises too: UDFs run inside the query in flight, so such calls used to wait for themselves forever, on main as well. A per-thread stack of active calls and UDFs tells these apart from the connection's own nested calls, and covers UDFs folded on the calling thread while binding. UDFs are shared by all connections of a database and the driver cannot tell which query runs one, so a UDF may not call any connection that has a call in flight; create_function() documents this. interrupt() also reaches the pybind connection used as a fallback by the C-API backend, and is a no-op on a closed connection. The scan-object lookup for LOAD FROM no longer assumes a fixed call depth and walks up to the first frame outside this module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 429a9d7 commit 60d8c75

5 files changed

Lines changed: 381 additions & 8 deletions

File tree

‎src_py/async_connection.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,8 @@ def close(self) -> None:
275275
"""
276276
Close all connections and shutdown the thread pool.
277277
278+
Queries still running are interrupted and fail with "Interrupted.".
279+
278280
Note: Call to this method is optional. The connections and thread pool
279281
will be closed automatically when the instance is garbage collected.
280282
"""

‎src_py/connection.py‎

Lines changed: 124 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
from __future__ import annotations
22

33
import contextlib
4+
import functools
45
import inspect
56
import json
67
import re
78
import threading
89
import uuid
910
import warnings
1011
import weakref
11-
from typing import TYPE_CHECKING, Any
12+
from typing import TYPE_CHECKING, Any, TypeVar, cast
1213
from weakref import WeakSet
1314

1415
from ._backend import get_capi_module, get_pybind_module
@@ -308,6 +309,82 @@ def _pybind_param_signature(parameters: dict[str, Any]) -> tuple:
308309
)
309310

310311

312+
_Method = TypeVar("_Method", bound="Callable[..., Any]")
313+
314+
# Per thread: the connections whose calls are running, innermost last, and a
315+
# marker for each Python UDF entered on top of them.
316+
_active = threading.local()
317+
_UDF = object()
318+
319+
320+
def _active_stack() -> list[object]:
321+
stack: list[object] | None = getattr(_active, "stack", None)
322+
if stack is None:
323+
stack = _active.stack = []
324+
return stack
325+
326+
327+
def _called_from_udf(conn: Connection) -> bool:
328+
"""Whether this thread is in a UDF entered after its last call on conn."""
329+
for entry in reversed(_active_stack()):
330+
if entry is conn:
331+
return False
332+
if entry is _UDF:
333+
return True
334+
return False
335+
336+
337+
def _tracks_call(method: _Method) -> _Method:
338+
"""Count the call as in flight so that close() can wait for it."""
339+
340+
@functools.wraps(method)
341+
def wrapper(self: Connection, *args: Any, **kwargs: Any) -> Any:
342+
with self._calls:
343+
if self._closing:
344+
msg = "Connection is closed."
345+
raise RuntimeError(msg)
346+
if self._calls_in_flight and _called_from_udf(self):
347+
# UDFs run inside the query in flight, which holds the
348+
# connection, so the call could wait for itself forever.
349+
msg = "A UDF cannot use a connection while it runs a query."
350+
raise RuntimeError(msg)
351+
self._calls_in_flight += 1
352+
stack = _active_stack()
353+
stack.append(self)
354+
try:
355+
return method(self, *args, **kwargs)
356+
finally:
357+
stack.pop()
358+
with self._calls:
359+
self._calls_in_flight -= 1
360+
self._calls.notify_all()
361+
362+
return cast("_Method", wrapper)
363+
364+
365+
def _stacklevel_outside_module() -> int:
366+
"""Return the warnings.warn() stacklevel of the first caller outside this module."""
367+
frame, level = inspect.currentframe(), 0
368+
while frame is not None and frame.f_code.co_filename == __file__:
369+
frame, level = frame.f_back, level + 1
370+
return level
371+
372+
373+
def _marks_udf_call(udf: Callable[..., Any]) -> Callable[..., Any]:
374+
"""Mark the thread as running a UDF so that calls into a busy connection raise."""
375+
376+
@functools.wraps(udf)
377+
def wrapper(*args: Any, **kwargs: Any) -> Any:
378+
stack = _active_stack()
379+
stack.append(_UDF)
380+
try:
381+
return udf(*args, **kwargs)
382+
finally:
383+
stack.pop()
384+
385+
return wrapper
386+
387+
311388
class Connection:
312389
"""Connection to a database."""
313390

@@ -329,6 +406,9 @@ def __init__(self, database: Database, num_threads: int = 0):
329406
self.database = database
330407
self.num_threads = num_threads
331408
self.is_closed = False
409+
self._closing = False
410+
self._calls = threading.Condition()
411+
self._calls_in_flight = 0
332412
self._prefer_pybind = False
333413
self._query_timeout_ms = 0
334414
self._query_results: WeakSet[QueryResult] = WeakSet()
@@ -379,6 +459,7 @@ def _using_pybind_backend(self) -> bool:
379459
self.database._use_pybind_backend and get_pybind_module() is not None
380460
)
381461

462+
@_tracks_call
382463
def set_max_threads_for_exec(self, num_threads: int) -> None:
383464
"""
384465
Set the maximum number of threads for executing queries.
@@ -404,10 +485,32 @@ def close(self) -> None:
404485
405486
Note: Call to this method is optional. The connection will be closed
406487
automatically when the object goes out of scope.
488+
489+
A query running on this connection in another thread is interrupted,
490+
and close() returns once it has left the engine. Called from a UDF
491+
while the connection runs a query, close() raises instead.
407492
"""
408493
if self.is_closed:
409494
return
410495

496+
if self in _active_stack():
497+
msg = "close() cannot be called from a call on the same connection."
498+
raise RuntimeError(msg)
499+
500+
with self._calls:
501+
if self._calls_in_flight and _called_from_udf(self):
502+
# UDFs run inside the query in flight, so waiting for it would
503+
# never end.
504+
msg = "close() cannot be called from a UDF while the connection runs a query."
505+
raise RuntimeError(msg)
506+
self._closing = True
507+
while self._calls_in_flight:
508+
# An interrupt that arrives while the query is still compiling is
509+
# cleared when execution starts, so repeat it until the call returns.
510+
with contextlib.suppress(RuntimeError):
511+
self.interrupt()
512+
self._calls.wait(timeout=0.05)
513+
411514
for query_result in list(self._query_results):
412515
query_result.close()
413516
self._query_results.clear()
@@ -814,11 +917,9 @@ def _maybe_raise_scan_unsupported_object(self, query: str) -> None:
814917
return
815918

816919
var_name = match.group(1)
817-
frame = inspect.currentframe()
818-
if frame is None or frame.f_back is None:
819-
return
820-
821-
caller = frame.f_back.f_back
920+
caller = inspect.currentframe()
921+
while caller is not None and caller.f_code.co_filename == __file__:
922+
caller = caller.f_back
822923
if caller is None:
823924
return
824925

@@ -837,6 +938,7 @@ def _maybe_raise_scan_unsupported_object(self, query: str) -> None:
837938
)
838939
raise RuntimeError(msg)
839940

941+
@_tracks_call
840942
def execute(
841943
self,
842944
query: str | PreparedStatement,
@@ -932,7 +1034,7 @@ def execute(
9321034
warnings.warn(
9331035
drop_result.getErrorMessage(),
9341036
RuntimeWarning,
935-
stacklevel=2,
1037+
stacklevel=_stacklevel_outside_module(),
9361038
)
9371039
finally:
9381040
self._capi_scan_tables.discard(table_name)
@@ -950,6 +1052,7 @@ def execute(
9501052
all_query_results.append(next_query_result)
9511053
return all_query_results
9521054

1055+
@_tracks_call
9531056
def query_as_arrow(self, query: str, chunk_size: int) -> ArrowQueryResult:
9541057
"""
9551058
Execute a query with the native Arrow collector path.
@@ -971,6 +1074,7 @@ def query_as_arrow(self, query: str, chunk_size: int) -> ArrowQueryResult:
9711074
self._register_query_result(current_query_result)
9721075
return current_query_result
9731076

1077+
@_tracks_call
9741078
def _prepare(
9751079
self,
9761080
query: str,
@@ -1069,6 +1173,7 @@ def _get_rel_table_names(self) -> list[dict[str, Any]]:
10691173
results.append({"name": name, "src": src_node, "dst": dst_node})
10701174
return results
10711175

1176+
@_tracks_call
10721177
def set_query_timeout(self, timeout_in_ms: int) -> None:
10731178
"""
10741179
Set the query timeout value in ms for executing queries.
@@ -1089,8 +1194,11 @@ def interrupt(self) -> None:
10891194
10901195
If there is no currently executing query, this function does nothing.
10911196
"""
1092-
self._connection.interrupt()
1197+
for native in (self._connection, self._py_connection):
1198+
if native is not None:
1199+
native.interrupt()
10931200

1201+
@_tracks_call
10941202
def create_function(
10951203
self,
10961204
name: str,
@@ -1124,12 +1232,16 @@ def create_function(
11241232
catch_exceptions: Optional[bool]
11251233
if true, when an exception is thrown from python, the function output will be null
11261234
Otherwise, the exception will be rethrown
1235+
1236+
The UDF must not call a connection that is running a query: such calls raise
1237+
RuntimeError, because the query running the UDF may be on that connection.
11271238
"""
11281239
if params_type is None:
11291240
params_type = []
11301241
parsed_params_type = [x if type(x) is str else x.value for x in params_type]
11311242
if type(return_type) is not str:
11321243
return_type = return_type.value
1244+
udf = _marks_udf_call(udf)
11331245

11341246
try:
11351247
self._connection.create_function(
@@ -1154,6 +1266,7 @@ def create_function(
11541266
catch_exceptions=catch_exceptions,
11551267
)
11561268

1269+
@_tracks_call
11571270
def remove_function(self, name: str) -> None:
11581271
"""
11591272
Remove a User Defined Function (UDF).
@@ -1172,6 +1285,7 @@ def remove_function(self, name: str) -> None:
11721285
self._prefer_pybind = True
11731286
py_connection.remove_function(name)
11741287

1288+
@_tracks_call
11751289
def create_arrow_table(
11761290
self,
11771291
table_name: str,
@@ -1211,6 +1325,7 @@ def create_arrow_table(
12111325
raise RuntimeError(query_result_internal.getErrorMessage())
12121326
return QueryResult(self, query_result_internal)
12131327

1328+
@_tracks_call
12141329
def drop_arrow_table(self, table_name: str) -> QueryResult:
12151330
"""
12161331
Drop an Arrow memory-backed table.
@@ -1239,6 +1354,7 @@ def drop_arrow_table(self, table_name: str) -> QueryResult:
12391354
raise RuntimeError(query_result_internal.getErrorMessage())
12401355
return QueryResult(self, query_result_internal)
12411356

1357+
@_tracks_call
12421358
def create_arrow_rel_table(
12431359
self,
12441360
table_name: str,

‎test/capi_xfails.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@
7575
"test/test_udf.py::test_udf_null",
7676
"test/test_udf.py::test_udf_except",
7777
"test/test_udf.py::test_udf_remove",
78+
"test/test_connection_close.py::test_close_with_query_in_flight[close_from_a_udf]",
79+
"test/test_udf_reentrancy.py::test_udf_calling_a_connection[idle_other_connection]",
80+
"test/test_udf_reentrancy.py::test_udf_calling_a_connection[same_connection_on_a_worker]",
81+
"test/test_udf_reentrancy.py::test_udf_calling_a_connection[same_connection_while_binding]",
7882
# C API Arrow export for JSON types is not yet implemented. get_as_df()
7983
# tests that need the pybind NPArrayWrapper path are xfailed.
8084
"test/test_json.py::test_get_as_df_json_scalar",

0 commit comments

Comments
 (0)