Skip to content

Commit 52eaeb6

Browse files
Gate AsyncConnection ResourceWarning on connected; mark closed in force_close_transport
Two cycle-21 ripple fixes for the AsyncConnection finalizer: - The "GC'd without await close()" warning fired on never-connected instances (``conn = AsyncConnection(...); del conn``, common in early-error and test-fixture flows). Add a companion ``_connected_flag`` that flips True only after ``_ensure_connection`` successfully builds the underlying client connection; the finalizer skips the warning until it's set. - ``force_close_transport`` (used by SA's ``terminate()`` outside- greenlet preflight, post-await RuntimeError catches, and direct user calls) reaped the writer transport but did not set ``closed_flag``. A subsequent GC sweep then emitted the warning even though the transport had been explicitly closed. Set the flag at the top of the method so any path through it counts as explicit cleanup. Test fixture that bypasses ``__init__`` via ``__new__`` updated to inject the new flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c28bbc8 commit 52eaeb6

2 files changed

Lines changed: 48 additions & 3 deletions

File tree

src/dqlitedbapi/aio/connection.py

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
logger = logging.getLogger(__name__)
4141

4242

43-
def _async_unclosed_warning(closed_flag: list[bool], address: str) -> None:
43+
def _async_unclosed_warning(
44+
closed_flag: list[bool], connected_flag: list[bool], address: str
45+
) -> None:
4446
"""Emit a ResourceWarning when an ``AsyncConnection`` is GC'd
4547
without ``await close()``.
4648
@@ -53,10 +55,27 @@ def _async_unclosed_warning(closed_flag: list[bool], address: str) -> None:
5355
own "Unclosed transport" warnings, which point at the StreamReader/
5456
StreamWriter rather than the dqlite layer they came from.
5557
58+
Three-flag gate:
59+
60+
- ``closed_flag[0]`` is True if ``close()`` ran or if the
61+
synchronous ``force_close_transport`` (terminate / SA outside-
62+
greenlet) ran.
63+
- ``connected_flag[0]`` is True only after ``_ensure_connection``
64+
successfully built the underlying ``DqliteConnection``. A
65+
never-connected instance has nothing to clean up — the warning
66+
would be a false positive.
67+
68+
Without the connected-flag gate, common patterns like
69+
``conn = AsyncConnection(...); del conn`` (early-error or
70+
test-fixture flow) would emit a misleading warning. Without
71+
``force_close_transport`` setting ``closed_flag``, the SA
72+
``terminate()`` path triggers the warning even though the
73+
transport was reaped.
74+
5675
Suppression-narrow ``RuntimeError`` mirrors the sync sibling's
5776
interpreter-shutdown race protection.
5877
"""
59-
if closed_flag[0]:
78+
if closed_flag[0] or not connected_flag[0]:
6079
return
6180
with contextlib.suppress(RuntimeError):
6281
warnings.warn(
@@ -224,7 +243,20 @@ def __init__(
224243
# finalizer's responsibility split (which does drive an
225244
# owned daemon thread; the async sibling has none).
226245
self._closed_flag: list[bool] = [False]
227-
weakref.finalize(self, _async_unclosed_warning, self._closed_flag, address)
246+
# Companion flag flipped True only after ``_ensure_connection``
247+
# successfully builds the underlying ``DqliteConnection``. The
248+
# finalizer skips the ResourceWarning when False so a
249+
# never-connected instance (e.g. ``conn = AsyncConnection(...);
250+
# del conn``, common in early-error and test-fixture flows)
251+
# doesn't emit a misleading "GC'd without close" warning.
252+
self._connected_flag: list[bool] = [False]
253+
weakref.finalize(
254+
self,
255+
_async_unclosed_warning,
256+
self._closed_flag,
257+
self._connected_flag,
258+
address,
259+
)
228260

229261
def _ensure_locks(self) -> tuple[asyncio.Lock, asyncio.Lock]:
230262
"""Lazy-create the asyncio locks on the currently-running loop.
@@ -339,6 +371,10 @@ async def _ensure_connection(self) -> DqliteConnection:
339371
await built.close()
340372
raise InterfaceError("Connection is closed")
341373
self._async_conn = built
374+
# Flip the finalizer's "anything to clean up" gate. From
375+
# this point GC without close emits the ResourceWarning;
376+
# before this point a never-connected instance is silent.
377+
self._connected_flag[0] = True
342378

343379
return self._async_conn
344380

@@ -546,6 +582,14 @@ def force_close_transport(self) -> None:
546582
Callers that need to wait for the async path to drain should
547583
await ``close()`` from the original loop instead.
548584
"""
585+
# Set the finalizer's closed_flag so a subsequent GC sweep
586+
# does not emit a misleading "GC'd without close()" warning
587+
# — the user (or SA's terminate()) explicitly cleaned up via
588+
# the synchronous force-close path. The flag is set
589+
# unconditionally (even on the no-op inner=None / fork-child
590+
# branches) so any path through this method counts as
591+
# explicit cleanup.
592+
self._closed_flag[0] = True
549593
inner = self._async_conn
550594
if inner is None:
551595
return

tests/test_async_close_finally_propagates_cancel.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ def _prime_connection_with_in_use_inner() -> AsyncConnection:
3939
conn._close_timeout = 0.5
4040
conn._creator_pid = _os.getpid()
4141
conn._closed_flag = [False]
42+
conn._connected_flag = [True]
4243
return conn
4344

4445

0 commit comments

Comments
 (0)