Clear _closed_event in pool.close's finally so the signalled wakeup event is GC'd alongside the pool's other once-used loop primitives

The pool's close path nulls every other once-used loop-bound
field after use (``_async_conn``, ``_protocol``,
``_connect_lock``, ``_op_lock``, ``_loop_ref``);
``_closed_event`` was the lone holdout. Once
``set()`` runs in the close finally, the at-capacity
wakeup event has no remaining waiters (the pool is
``_closed=True``; new acquirers short-circuit before
parking) — yet the reference stayed pinned for the
lifetime of the pool object, keeping the event's
captured loop reachable through any SA engine
diagnostic accessor / module-level cache that
outlives ``close()``.

``_close_done`` deliberately stays — the second-caller
``if self._closed:`` arm awaits it, and clearing in
that arm would be TOCTOU. Clearing only ``_closed_event``
in the first caller's finally matches the project's
"null every once-used loop-bound field" discipline
without touching the second-caller drain visibility.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: antoineleclair

src/dqliteclient/pool.py

@@ -1432,6 +1432,19 @@ async def close(self) -> None:
             await self._drain_idle()
         finally:
             self._close_done.set()
+            # Drop the signalled-and-now-useless wakeup event
+            # so it can be GC'd alongside the pool's other
+            # once-used loop-bound primitives (``_async_conn``,
+            # ``_protocol``, ``_connect_lock``, ``_op_lock``,
+            # ``_loop_ref`` are all nulled at the end of their
+            # close paths). ``_close_done`` stays — the
+            # second-caller arm above awaits it; clearing in
+            # the second-caller arm would be TOCTOU and clearing
+            # here while siblings might still be parked is wrong.
+            # ``_closed_event`` has no remaining waiters once
+            # ``set()`` runs (the pool is closed; new acquirers
+            # short-circuit before parking) so it is safe to drop.
+            self._closed_event = None
 
         # In-use connections are closed by acquire()'s cleanup when they
         # return — the else branch checks _closed and closes instead of

tests/test_pool_close_clears_closed_event_after_drain.py

@@ -0,0 +1,57 @@
+"""Pin: ``ConnectionPool.close()`` clears the at-capacity wakeup
+event ``self._closed_event`` after the drain completes.
+
+The pool's close path nulls every other once-used loop-bound
+field after use (``_async_conn``, ``_protocol``, ``_connect_lock``,
+``_op_lock``, ``_loop_ref``); ``_closed_event`` is the lone
+holdout. A signalled-and-now-useless ``asyncio.Event`` bound to
+the pool's loop is held until the pool object itself is GC'd,
+keeping the loop reference alive past the user's intended
+lifetime when the pool object outlives ``close()`` (SA engine
+diagnostic accessors, module-level state, etc.).
+
+``_close_done`` is deliberately NOT cleared — see the issue's
+"Subtle" paragraph: clearing in the second-caller branch is
+TOCTOU; clearing only in the first caller's finally is what's
+needed, but the second-caller arm awaits the same event so we
+keep its lifetime tied to the pool's own.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from dqliteclient.cluster import ClusterClient
+from dqliteclient.pool import ConnectionPool
+
+
+@pytest.mark.asyncio
+async def test_pool_close_clears_closed_event_after_drain() -> None:
+    cluster = MagicMock(spec=ClusterClient)
+    pool = ConnectionPool(
+        addresses=["localhost:9001"],
+        min_size=0,
+        max_size=2,
+        timeout=1.0,
+        cluster=cluster,
+    )
+    # Force the wakeup event to exist (acquire normally lazy-creates
+    # it via ``_get_closed_event`` when a parker is going to sleep).
+    pool._get_closed_event()
+    assert pool._closed_event is not None
+
+    await pool.close()
+
+    # Mirror the discipline applied to every other once-used
+    # loop-bound field on the close path: drop the now-useless
+    # event reference so it can be GC'd alongside the pool's
+    # other loop primitives. ``_close_done`` stays — the
+    # second-caller arm awaits it.
+    assert pool._closed_event is None, (
+        "ConnectionPool.close() must clear _closed_event after "
+        "set() — every other loop-bound field on the close path "
+        "is nulled; this is the lone exception, leaving a "
+        "signalled Event pinned to the pool's loop."
+    )