Skip to content

Commit b9c7106

Browse files
Clean up raw_conn if eager connect fails in the async dialect
DqliteDialect_aio.connect() calls self.loaded_dbapi.connect() to build an AsyncConnection, then drives its eager TCP connect via await_only(raw_conn.connect()). If that eager step raised, raw_conn was already constructed (holding references to loop locks and partially-initialised state) and leaked until GC. In SQLAlchemy's connection-pool context a handful of those per leader flap added up fast. Wrap the eager connect in try/except BaseException, close raw_conn inside a narrow suppress(Exception), then re-raise. Using BaseException means cancellation during connect (parent asyncio.timeout(), TaskGroup cancel) also triggers cleanup and still propagates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6560ea5 commit b9c7106

2 files changed

Lines changed: 78 additions & 2 deletions

File tree

src/sqlalchemydqlite/aio.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,25 @@ def connect(self, *cargs: Any, **cparams: Any) -> Any:
253253
"""Create and wrap an async connection.
254254
255255
Eagerly establishes the TCP connection so errors surface at
256-
connect-time rather than on the first query.
256+
connect-time rather than on the first query. If that eager
257+
connect raises, the ``raw_conn`` object is already constructed
258+
and holds references to loop locks / partially-initialised
259+
state — without explicit cleanup it leaks until GC and can
260+
linger on the event loop it was bound to. Call ``close()`` on
261+
``BaseException`` so cancellation (e.g. a parent
262+
``asyncio.timeout()`` firing during connect) also cleans up,
263+
then re-raise unchanged. ``close()`` is documented as
264+
idempotent and safe to call even when no TCP connection
265+
landed, so the suppression around it is narrow
266+
(``Exception`` only; cancellation signals still propagate).
257267
"""
258268
raw_conn = self.loaded_dbapi.connect(*cargs, **cparams)
259-
await_only(raw_conn.connect())
269+
try:
270+
await_only(raw_conn.connect())
271+
except BaseException:
272+
with contextlib.suppress(Exception):
273+
await_only(raw_conn.close())
274+
raise
260275
return AsyncAdaptedConnection(raw_conn)
261276

262277
def get_driver_connection(self, connection: Any) -> Any:

tests/test_async_adapter.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,67 @@ def test_close_propagates_programming_error_from_rollback(self) -> None:
242242
adapted.close()
243243

244244

245+
class TestAioDialectConnectCleanup:
246+
"""DqliteDialect_aio.connect() eagerly establishes the TCP
247+
connection. If that raises, raw_conn (already constructed) must
248+
be closed — otherwise the partial connection leaks. Cleanup must
249+
also fire on cancellation so a parent asyncio.timeout() firing
250+
mid-connect doesn't leave the raw_conn dangling.
251+
"""
252+
253+
def test_close_is_called_when_eager_connect_fails(self) -> None:
254+
import pytest
255+
256+
from dqliteclient.exceptions import DqliteConnectionError
257+
from sqlalchemydqlite.aio import DqliteDialect_aio
258+
259+
dialect = DqliteDialect_aio()
260+
261+
raw_conn = MagicMock()
262+
raw_conn.connect = MagicMock(side_effect=DqliteConnectionError("probe failed"))
263+
raw_conn.close = MagicMock()
264+
265+
dbapi = MagicMock()
266+
dbapi.connect = MagicMock(return_value=raw_conn)
267+
dialect.loaded_dbapi = dbapi
268+
269+
with (
270+
patch("sqlalchemydqlite.aio.await_only", side_effect=_run_sync),
271+
pytest.raises(DqliteConnectionError, match="probe failed"),
272+
):
273+
dialect.connect()
274+
275+
raw_conn.close.assert_called_once()
276+
277+
def test_close_is_called_on_cancellation(self) -> None:
278+
"""CancelledError during eager connect must trigger cleanup
279+
AND still propagate — callers rely on structured-concurrency
280+
signal fidelity."""
281+
import asyncio
282+
283+
import pytest
284+
285+
from sqlalchemydqlite.aio import DqliteDialect_aio
286+
287+
dialect = DqliteDialect_aio()
288+
289+
raw_conn = MagicMock()
290+
raw_conn.connect = MagicMock(side_effect=asyncio.CancelledError())
291+
raw_conn.close = MagicMock()
292+
293+
dbapi = MagicMock()
294+
dbapi.connect = MagicMock(return_value=raw_conn)
295+
dialect.loaded_dbapi = dbapi
296+
297+
with (
298+
patch("sqlalchemydqlite.aio.await_only", side_effect=_run_sync),
299+
pytest.raises(asyncio.CancelledError),
300+
):
301+
dialect.connect()
302+
303+
raw_conn.close.assert_called_once()
304+
305+
245306
class TestAioAllExports:
246307
"""Adapter classes are part of the supported public surface."""
247308

0 commit comments

Comments
 (0)