Skip to content

Commit 57b6799

Browse files
Fix do_ping to close cursor in finally block
Ensure cursor is always closed even when execute raises, preventing resource leaks on connection health checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f557db6 commit 57b6799

2 files changed

Lines changed: 55 additions & 2 deletions

File tree

src/sqlalchemydqlite/base.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,14 @@ def is_disconnect(self, e: Any, connection: Any, cursor: Any) -> bool:
134134

135135
def do_ping(self, dbapi_connection: Any) -> bool:
136136
"""Check if the connection is still alive."""
137+
cursor = dbapi_connection.cursor()
137138
try:
138-
cursor = dbapi_connection.cursor()
139139
cursor.execute("SELECT 1")
140-
cursor.close()
141140
return True
142141
except Exception:
143142
return False
143+
finally:
144+
cursor.close()
144145

145146
def _get_server_version_info(self, connection: Any) -> tuple[int, ...]:
146147
"""Return the server version as a tuple.

tests/test_dialect.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,58 @@ def test_set_isolation_level_silent_for_none(self) -> None:
326326
assert len(w) == 0
327327

328328

329+
class TestDoPing:
330+
def test_cursor_closed_in_finally(self) -> None:
331+
"""do_ping must close cursor in a finally block."""
332+
import ast
333+
import inspect
334+
import textwrap
335+
336+
source = textwrap.dedent(inspect.getsource(DqliteDialect.do_ping))
337+
tree = ast.parse(source)
338+
339+
has_finally_close = False
340+
for node in ast.walk(tree):
341+
if isinstance(node, ast.Try) and node.finalbody:
342+
for stmt in ast.walk(node):
343+
if isinstance(stmt, ast.Call):
344+
func_node = stmt.func
345+
if isinstance(func_node, ast.Attribute) and func_node.attr == "close":
346+
has_finally_close = True
347+
348+
assert has_finally_close, "cursor.close() should be in a finally block in do_ping"
349+
350+
def test_ping_returns_true_on_success(self) -> None:
351+
"""do_ping should return True when query succeeds."""
352+
from unittest.mock import MagicMock
353+
354+
dialect = DqliteDialect()
355+
mock_conn = MagicMock()
356+
assert dialect.do_ping(mock_conn) is True
357+
358+
def test_ping_returns_false_on_error(self) -> None:
359+
"""do_ping should return False when query fails."""
360+
from unittest.mock import MagicMock
361+
362+
dialect = DqliteDialect()
363+
mock_conn = MagicMock()
364+
mock_conn.cursor.return_value.execute.side_effect = RuntimeError("boom")
365+
assert dialect.do_ping(mock_conn) is False
366+
367+
def test_ping_closes_cursor_even_on_error(self) -> None:
368+
"""do_ping must close cursor even when execute fails."""
369+
from unittest.mock import MagicMock
370+
371+
dialect = DqliteDialect()
372+
mock_conn = MagicMock()
373+
mock_cursor = MagicMock()
374+
mock_cursor.execute.side_effect = RuntimeError("boom")
375+
mock_conn.cursor.return_value = mock_cursor
376+
377+
dialect.do_ping(mock_conn)
378+
mock_cursor.close.assert_called_once()
379+
380+
329381
class TestURLParsing:
330382
def test_parse_basic_url(self) -> None:
331383
url = URL.create("dqlite", host="localhost", port=9001, database="test")

0 commit comments

Comments
 (0)