Skip to content

Commit 44c7ecd

Browse files
Pin pool acquire late-winner and initialize unqueued-survivor paths
Two timing-sensitive coverage gaps: - _put_back_or_release_late_winner: the recovery hook for two race paths in acquire (outer cancel arrived just after a sibling _release put a conn on the queue, post-wait demux else-arm). Pin both arms (queue-has-room → put_nowait, queue-full → close + release). - Initialize unqueued-survivor close-failure log: a successful conn that fails put_nowait then fails its own close() must emit a DEBUG log without disrupting the rest of the cleanup loop. Both paths were tested only via integration race scenarios. Adding focused unit tests on the helpers / specific arms catches a regression at PR time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2446d38 commit 44c7ecd

2 files changed

Lines changed: 109 additions & 0 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
"""``ConnectionPool._put_back_or_release_late_winner`` is the recovery
2+
hook for two race paths in ``acquire``: an outer cancel that arrives
3+
just after a sibling ``_release`` puts a conn on the queue, and the
4+
post-wait demux's else-arm where a timeout snapshot raced a winning
5+
``get_task``. Both paths route through this helper so the conn is
6+
not silently dropped.
7+
8+
The helper has two arms:
9+
- queue not full → ``put_nowait`` and the conn is reusable.
10+
- queue full → close + release reservation (invariant-violation
11+
recovery).
12+
13+
Pin both arms.
14+
"""
15+
16+
from unittest.mock import AsyncMock, MagicMock, patch
17+
18+
import pytest
19+
20+
from dqliteclient.pool import ConnectionPool
21+
22+
23+
@pytest.mark.asyncio
24+
async def test_late_winner_put_back_when_queue_has_room() -> None:
25+
"""The happy path: queue has room, conn is put back and the
26+
pool's free-conn counter reflects it."""
27+
pool = ConnectionPool(addresses=["h:9001"], min_size=0, max_size=4)
28+
fake_conn = MagicMock()
29+
30+
await pool._put_back_or_release_late_winner(fake_conn)
31+
32+
assert pool._pool.qsize() == 1
33+
queued = pool._pool.get_nowait()
34+
assert queued is fake_conn
35+
36+
37+
@pytest.mark.asyncio
38+
async def test_late_winner_close_and_release_on_queue_full() -> None:
39+
"""The recovery path: queue is full, conn is closed and the
40+
reservation released so the pool shrinks rather than leaks."""
41+
pool = ConnectionPool(addresses=["h:9001"], min_size=0, max_size=1)
42+
# Saturate the queue so put_nowait raises QueueFull.
43+
filler = MagicMock()
44+
pool._pool.put_nowait(filler)
45+
46+
fake_conn = MagicMock()
47+
fake_conn.close = AsyncMock()
48+
49+
with patch.object(pool, "_release_reservation", new=AsyncMock()) as rel:
50+
await pool._put_back_or_release_late_winner(fake_conn)
51+
52+
fake_conn.close.assert_awaited_once()
53+
rel.assert_awaited_once()
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
"""``ConnectionPool.initialize`` walks ``unqueued_survivors`` after a
2+
mid-init cancel and closes each. If the close itself raises
3+
``_POOL_CLEANUP_EXCEPTIONS`` (OSError / DqliteConnectionError), the
4+
``logger.debug`` emit at ``pool.py:623-627`` is the only forensic
5+
trail.
6+
7+
Without coverage, a regression that drops the ``except`` clause and
8+
re-raises during cleanup would supplant the user-visible original
9+
error and the unqueued-survivor cleanup loop would abort partway
10+
through, leaking subsequent survivors.
11+
"""
12+
13+
import asyncio
14+
from unittest.mock import AsyncMock, MagicMock, patch
15+
16+
import pytest
17+
18+
from dqliteclient.pool import ConnectionPool
19+
20+
21+
@pytest.mark.asyncio
22+
async def test_unqueued_survivor_close_failure_debug_logged(
23+
caplog: pytest.LogCaptureFixture,
24+
) -> None:
25+
"""A successful conn whose subsequent close() raises must
26+
emit a DEBUG log without disrupting the rest of the cleanup
27+
loop."""
28+
pool = ConnectionPool(addresses=["h:9001"], min_size=2, max_size=4)
29+
30+
# Two conns: one that closes cleanly, one that raises OSError.
31+
clean_conn = MagicMock()
32+
clean_conn.close = AsyncMock()
33+
failing_conn = MagicMock()
34+
failing_conn.close = AsyncMock(side_effect=OSError("simulated close failure"))
35+
36+
# _create_connection returns the two conns in order. After they
37+
# succeed, force put_nowait to raise so they end up on
38+
# unqueued_survivors.
39+
create_calls = iter([clean_conn, failing_conn])
40+
41+
async def stub_create() -> MagicMock:
42+
return next(create_calls)
43+
44+
with (
45+
patch.object(pool, "_create_connection", new=stub_create),
46+
patch.object(pool._pool, "put_nowait", side_effect=asyncio.QueueFull),
47+
caplog.at_level("DEBUG"),
48+
pytest.raises(asyncio.QueueFull),
49+
):
50+
await pool.initialize()
51+
52+
# Both close() calls must have been attempted; the OSError on
53+
# the second is logged but does not stop the loop.
54+
clean_conn.close.assert_awaited()
55+
failing_conn.close.assert_awaited()
56+
assert any("unqueued-survivor close error" in r.message for r in caplog.records)

0 commit comments

Comments
 (0)