Skip to content

Commit 141c3f2

Browse files
Fix resource leak when async connection close fails
Wrap close() in try/finally to ensure the event loop and thread are always cleaned up, even if closing the async connection raises. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3cad8b5 commit 141c3f2

2 files changed

Lines changed: 50 additions & 11 deletions

File tree

src/dqlitedbapi/connection.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""PEP 249 Connection implementation for dqlite."""
22

33
import asyncio
4+
import contextlib
45
import threading
56
from typing import Any
67

@@ -85,17 +86,20 @@ async def _get_async_connection(self) -> DqliteConnection:
8586

8687
def close(self) -> None:
8788
"""Close the connection."""
88-
if self._async_conn is not None:
89-
self._run_sync(self._async_conn.close())
90-
self._async_conn = None
91-
if self._loop is not None and not self._loop.is_closed():
92-
self._loop.call_soon_threadsafe(self._loop.stop)
93-
if self._thread is not None:
94-
self._thread.join(timeout=5)
95-
self._loop.close()
96-
self._loop = None
97-
self._thread = None
98-
self._closed = True
89+
try:
90+
if self._async_conn is not None:
91+
with contextlib.suppress(Exception):
92+
self._run_sync(self._async_conn.close())
93+
self._async_conn = None
94+
finally:
95+
if self._loop is not None and not self._loop.is_closed():
96+
self._loop.call_soon_threadsafe(self._loop.stop)
97+
if self._thread is not None:
98+
self._thread.join(timeout=5)
99+
self._loop.close()
100+
self._loop = None
101+
self._thread = None
102+
self._closed = True
99103

100104
def commit(self) -> None:
101105
"""Commit any pending transaction."""

tests/test_close_resource_leak.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
"""Tests that close() cleans up resources even if async close fails."""
2+
3+
from unittest.mock import AsyncMock, patch
4+
5+
from dqlitedbapi.connection import Connection
6+
7+
8+
class TestCloseResourceLeak:
9+
def test_close_cleans_up_loop_even_if_async_close_fails(self) -> None:
10+
"""Event loop and thread must be cleaned up even if async close raises."""
11+
conn = Connection("localhost:9001")
12+
13+
# Force the connection to create a loop and async connection
14+
with patch("dqlitedbapi.connection.DqliteConnection") as MockDqliteConn:
15+
mock_instance = AsyncMock()
16+
mock_instance.connect = AsyncMock()
17+
mock_instance._protocol = AsyncMock()
18+
mock_instance._db_id = 0
19+
# Make close() raise
20+
mock_instance.close = AsyncMock(side_effect=RuntimeError("close failed"))
21+
MockDqliteConn.return_value = mock_instance
22+
23+
# Establish connection
24+
conn._run_sync(conn._get_async_connection())
25+
assert conn._loop is not None
26+
assert conn._thread is not None
27+
assert conn._thread.is_alive()
28+
29+
# close() should not raise, and should clean up the loop
30+
conn.close()
31+
32+
assert conn._closed
33+
assert conn._loop is None
34+
assert conn._thread is None
35+
assert conn._async_conn is None

0 commit comments

Comments
 (0)