This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 8286d178f fix(python/adbc_driver_manager): close Cursors when closing
Connection (#3810)
8286d178f is described below
commit 8286d178f1863e7731910a0da2b84b07ef343489
Author: Philip Moore <[email protected]>
AuthorDate: Wed Dec 17 18:13:48 2025 -0500
fix(python/adbc_driver_manager): close Cursors when closing Connection
(#3810)
Closes #3713.
---------
Co-authored-by: Bryce Mecum <[email protected]>
---
.../adbc_driver_manager/dbapi.py | 20 ++-
python/adbc_driver_manager/tests/test_dbapi.py | 138 +++++++++++++++++++++
2 files changed, 157 insertions(+), 1 deletion(-)
diff --git a/python/adbc_driver_manager/adbc_driver_manager/dbapi.py
b/python/adbc_driver_manager/adbc_driver_manager/dbapi.py
index 91072768e..fb83b04f3 100644
--- a/python/adbc_driver_manager/adbc_driver_manager/dbapi.py
+++ b/python/adbc_driver_manager/adbc_driver_manager/dbapi.py
@@ -46,6 +46,7 @@ import threading
import time
import typing
import warnings
+import weakref
from typing import Any, Dict, List, Literal, Optional, Tuple, Union
try:
@@ -341,6 +342,7 @@ class Connection(_Closeable):
self._db = _SharedDatabase(db)
self._conn = conn
self._conn_kwargs = conn_kwargs
+ self._cursors: "weakref.WeakSet[Cursor]" = weakref.WeakSet()
if autocommit:
self._autocommit = True
@@ -364,6 +366,10 @@ class Connection(_Closeable):
"""
Close the connection.
+ Notes
+ -----
+ This will also close any open cursors associated with this connection.
+
Warnings
--------
Failure to close a connection may leak memory or database
@@ -372,6 +378,16 @@ class Connection(_Closeable):
if self._closed:
return
+ # Try to close all open cursors first to avoid RuntimeError from
+ # AdbcConnection about open children
+ for cursor in self._cursors:
+ try:
+ cursor.close()
+ except Exception as e:
+ warnings.warn(
+ f"Failed to close cursor: {e}", ResourceWarning,
stacklevel=2
+ )
+
self._conn.close()
self._db.close()
self._closed = True
@@ -394,7 +410,9 @@ class Connection(_Closeable):
adbc_stmt_kwargs : dict, optional
ADBC-specific options to pass to the underlying ADBC statement.
"""
- return Cursor(self, adbc_stmt_kwargs, dbapi_backend=self._backend)
+ cursor = Cursor(self, adbc_stmt_kwargs, dbapi_backend=self._backend)
+ self._cursors.add(cursor)
+ return cursor
def rollback(self) -> None:
"""Explicitly rollback."""
diff --git a/python/adbc_driver_manager/tests/test_dbapi.py
b/python/adbc_driver_manager/tests/test_dbapi.py
index 38c836a58..ed58a2905 100644
--- a/python/adbc_driver_manager/tests/test_dbapi.py
+++ b/python/adbc_driver_manager/tests/test_dbapi.py
@@ -569,6 +569,144 @@ def test_dbapi_extensions(sqlite):
assert cur.execute("SELECT 42").fetchall() == [(42,)]
[email protected]
+def test_close_connection_with_open_cursor():
+ """
+ Regression test for https://github.com/apache/arrow-adbc/issues/3713
+
+ Closing a connection should automatically close any open cursors
+ without raising RuntimeError about open AdbcStatement.
+
+ This test intentionally avoids context managers because the bug only
+ manifests when users manually call close() without first closing cursors.
+ """
+ conn = dbapi.connect(driver="adbc_driver_sqlite")
+ cursor = conn.cursor()
+ cursor.execute("SELECT 1")
+ # Fetch the result to ensure the cursor is "active"
+ cursor.fetch_arrow_table()
+
+ # This should NOT raise:
+ # RuntimeError: Cannot close AdbcConnection with open AdbcStatement
+ conn.close()
+
+ # Verify cursor is also closed
+ assert cursor._closed
+
+
[email protected]
+def test_close_connection_with_multiple_open_cursors():
+ """
+ Test that closing a connection closes all open cursors.
+
+ This test intentionally avoids context managers because the bug only
+ manifests when users manually call close() without first closing cursors.
+ """
+ conn = dbapi.connect(driver="adbc_driver_sqlite")
+ cursor1 = conn.cursor()
+ cursor2 = conn.cursor()
+ cursor3 = conn.cursor()
+
+ cursor1.execute("SELECT 1")
+ cursor2.execute("SELECT 2")
+ # cursor3 is not executed
+
+ # This should close all cursors
+ conn.close()
+
+ assert cursor1._closed
+ assert cursor2._closed
+ assert cursor3._closed
+
+
[email protected]
+def test_close_connection_cursor_already_closed():
+ """
+ Test that closing a connection works even if some cursors are already
closed.
+
+ This test intentionally avoids context managers because the bug only
+ manifests when users manually call close() without first closing cursors.
+ """
+ conn = dbapi.connect(driver="adbc_driver_sqlite")
+ cursor1 = conn.cursor()
+ cursor2 = conn.cursor()
+
+ cursor1.execute("SELECT 1")
+ cursor1.close() # Manually close cursor1
+
+ cursor2.execute("SELECT 2")
+ # cursor2 is still open
+
+ # This should work without issues
+ conn.close()
+
+ assert cursor1._closed
+ assert cursor2._closed
+
+
[email protected]
+def test_close_connection_via_context_manager_with_open_cursors():
+ """
+ Test that exiting a connection context manager closes open cursors.
+
+ This test uses a context manager for the connection but intentionally
+ avoids context managers for cursors to verify that Connection.__exit__
+ properly closes any open cursors.
+ """
+ with dbapi.connect(driver="adbc_driver_sqlite") as conn:
+ cursor1 = conn.cursor()
+ cursor2 = conn.cursor()
+
+ cursor1.execute("SELECT 1")
+ cursor2.execute("SELECT 2")
+
+ # Cursors are open at this point
+ assert not cursor1._closed
+ assert not cursor2._closed
+
+ # After exiting the context manager, cursors should be closed
+ assert cursor1._closed
+ assert cursor2._closed
+
+
[email protected]
+def test_close_connection_suppresses_cursor_close_error():
+ """
+ Test that closing a connection suppresses exceptions from cursor.close().
+
+ When a cursor's close() method raises an exception, the connection should
+ still close successfully without propagating the error, but should emit
+ a ResourceWarning.
+ """
+ from unittest.mock import MagicMock
+
+ conn = dbapi.connect(driver="adbc_driver_sqlite")
+ # Create a real cursor so we have something legitimate in _cursors
+ real_cursor = conn.cursor()
+ real_cursor.execute("SELECT 1")
+
+ # Create a mock cursor that raises on close
+ mock_cursor = MagicMock()
+ mock_cursor.close.side_effect = RuntimeError("Simulated cursor close
failure")
+
+ # Add the mock cursor to the connection's cursor set
+ conn._cursors.add(mock_cursor)
+
+ # This should NOT raise despite the mock cursor raising on close,
+ # but should emit a ResourceWarning
+ with pytest.warns(ResourceWarning, match="Failed to close cursor"):
+ conn.close()
+
+ # Verify the connection is closed
+ assert conn._closed
+
+ # Verify the mock cursor's close was called
+ mock_cursor.close.assert_called_once()
+
+ # Verify the real cursor is also closed
+ assert real_cursor._closed
+
+
@pytest.mark.sqlite
def test_connect(tmp_path: pathlib.Path, monkeypatch) -> None:
with dbapi.connect(driver="adbc_driver_sqlite") as conn:
