Skip to content

Commit 0cfb00e

Browse files
Narrow pool _release drain shield to not swallow KI / SystemExit
The shielded pending-drain await previously used contextlib.suppress(BaseException), which silently consumed KeyboardInterrupt, SystemExit, and any other BaseException — including a fresh CancelledError that an outer asyncio.timeout might land on _release while the drain is in flight. Operators expect Ctrl+C to escape; suppressing it at this boundary made the cleanup a silent no-op past the next checkpoint. Narrow to (CancelledError, Exception): CancelledError covers the awaiter-side cancel that the shield's protect-the-inner-task contract relies on; Exception covers a synthetic drain-failure that the existing pin asserts; KeyboardInterrupt and SystemExit now propagate cleanly. Mirrors the adjacent _release_reservation shield's narrower CancelledError-only suppression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e6a6323 commit 0cfb00e

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

src/dqliteclient/pool.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1161,7 +1161,25 @@ async def _release(self, conn: DqliteConnection) -> None:
11611161
# "Task was destroyed but it is pending" on shutdown).
11621162
pending = getattr(conn, "_pending_drain", None)
11631163
if pending is not None and not pending.done():
1164-
with contextlib.suppress(BaseException):
1164+
# Absorb the await-side CancelledError so a fresh
1165+
# outer cancel during release does not tear down
1166+
# the bounded drain (the shield itself protects
1167+
# the inner task; this suppress covers the
1168+
# awaiter-side raise delivered when the
1169+
# surrounding scope is cancelled). Do NOT suppress
1170+
# KeyboardInterrupt / SystemExit — those must
1171+
# propagate. Mirrors the ``_release_reservation``
1172+
# shield two lines below.
1173+
#
1174+
# Also suppress Exception to cover the drain task
1175+
# itself raising — production drains run inside
1176+
# ``_bounded_drain`` which already wraps in
1177+
# ``suppress(Exception)``, but the
1178+
# belt-and-braces inner suppress here keeps the
1179+
# cleanup path immune to a synthetic drain failure
1180+
# (and to a future refactor that drops
1181+
# ``_bounded_drain``'s wrap).
1182+
with contextlib.suppress(asyncio.CancelledError, Exception):
11651183
await asyncio.shield(pending)
11661184
conn._pool_released = True
11671185
with contextlib.suppress(asyncio.CancelledError):
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
"""Pin: ``_release``'s shielded ``_pending_drain`` await suppresses
2+
``CancelledError`` and ``Exception``, but MUST NOT suppress arbitrary
3+
``BaseException``. The previous shape used
4+
``contextlib.suppress(BaseException)`` which silently consumed
5+
control-plane signals like ``KeyboardInterrupt`` / ``SystemExit``.
6+
7+
Operators expect Ctrl+C to escape the cleanup path; suppressing
8+
those signals at this boundary turns them into silent no-ops until
9+
the next checkpoint that re-raises (none, in this code path —
10+
``_release`` returns normally).
11+
12+
The test uses a custom ``BaseException`` subclass instead of
13+
``KeyboardInterrupt`` / ``SystemExit`` so pytest's own top-level
14+
handling does not intercept the signal.
15+
"""
16+
17+
from __future__ import annotations
18+
19+
import asyncio
20+
21+
import pytest
22+
23+
from dqliteclient import DqliteConnection
24+
from dqliteclient.pool import ConnectionPool
25+
26+
27+
class _SyntheticControlPlaneSignal(BaseException):
28+
"""A BaseException subclass that pytest does not intercept."""
29+
30+
31+
@pytest.mark.asyncio
32+
async def test_drain_base_exception_propagates_through_release() -> None:
33+
pool = ConnectionPool(["localhost:9001"], min_size=0, max_size=1)
34+
pool._size = 1
35+
conn = DqliteConnection("localhost:9001")
36+
conn._protocol = object() # type: ignore[assignment]
37+
conn._db_id = 1
38+
conn._in_transaction = True
39+
40+
async def signaling_drain() -> None:
41+
# Start the task on the loop, then raise the BaseException.
42+
await asyncio.sleep(0)
43+
raise _SyntheticControlPlaneSignal("control-plane signal")
44+
45+
conn._pending_drain = asyncio.create_task(signaling_drain())
46+
47+
async def fake_reset(c: DqliteConnection) -> bool:
48+
return False
49+
50+
pool._reset_connection = fake_reset # type: ignore[assignment]
51+
52+
async def noop() -> None:
53+
return
54+
55+
pool._release_reservation = noop
56+
57+
async def fake_close() -> None:
58+
return
59+
60+
conn.close = fake_close
61+
62+
with pytest.raises(_SyntheticControlPlaneSignal):
63+
await pool._release(conn)

0 commit comments

Comments
 (0)