Skip to content

Commit 2c2cd80

Browse files
Mirror in-lock messages clear in sync commit/rollback async bodies
The async commit() / rollback() pair clears Connection.messages twice: once pre-lock to honour PEP 249 §6.1.1 on the closed-raise and never-connected fast paths, and once inside the op_lock so a sibling task that appends to messages between the pre-lock clear and the COMMIT round-trip is wiped before the wire call. The sync counterparts only had the pre-lock clear; the same sibling-thread window was undefended. Add a second del self.messages[:] inside _commit_async and _rollback_async (which run under _op_lock via _run_sync) and pin both branches with direct asyncio.run tests of the inner coroutines, including the exception-from-execute path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cb81cc8 commit 2c2cd80

2 files changed

Lines changed: 89 additions & 0 deletions

File tree

src/dqlitedbapi/connection.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,14 @@ async def _commit_async(self) -> None:
814814
"""Async implementation of commit."""
815815
if self._async_conn is None:
816816
raise InterfaceError("Connection is closed")
817+
# Clear ``messages`` under the lock so the PEP 249 contract
818+
# "messages cleared by every method call" is atomic with the
819+
# operation. ``_run_sync`` holds ``_op_lock`` across this
820+
# coroutine; the pre-lock clear in ``commit()`` leaves a
821+
# window where a sibling thread could write directly to
822+
# ``messages`` between that clear and the lock acquire.
823+
# Mirror the async sibling's defense-in-depth shape.
824+
del self.messages[:]
817825
try:
818826
# Route through ``_call_client`` so client-layer errors
819827
# (including ``DqliteConnectionError`` for an externally
@@ -849,6 +857,9 @@ async def _rollback_async(self) -> None:
849857
"""Async implementation of rollback."""
850858
if self._async_conn is None:
851859
raise InterfaceError("Connection is closed")
860+
# In-lock messages clear; see ``_commit_async`` for the
861+
# rationale.
862+
del self.messages[:]
852863
try:
853864
# See ``_commit_async``: route through ``_call_client`` so
854865
# client-layer failures surface as PEP 249 ``Error``
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
"""Pin: sync ``Connection.commit`` / ``rollback`` clear ``messages``
2+
both pre-lock AND inside ``_op_lock`` (mirroring the async sibling).
3+
4+
PEP 249 §6.1.1 says ``Connection.messages`` is cleared by every
5+
method call on the connection. Clearing only pre-lock leaves a
6+
window where a sibling thread can append directly to
7+
``conn.messages`` between the pre-lock clear and the COMMIT round-
8+
trip; the in-lock clear in ``_commit_async`` / ``_rollback_async``
9+
(which run on the loop thread inside ``_run_sync``'s ``_op_lock``
10+
critical section) defends that window and matches the
11+
already-hardened async surface.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import asyncio
17+
import contextlib
18+
from unittest.mock import AsyncMock, MagicMock
19+
20+
from dqlitedbapi.connection import Connection
21+
22+
23+
def _build_conn_with_mocked_async() -> Connection:
24+
"""Build a Connection with no real socket: bypass ``__init__`` and
25+
wire only the fields ``_commit_async`` / ``_rollback_async`` touch.
26+
"""
27+
conn = Connection.__new__(Connection)
28+
conn.messages = []
29+
fake = MagicMock()
30+
fake.execute = AsyncMock(return_value=(0, 0))
31+
fake.in_transaction = True
32+
conn._async_conn = fake
33+
return conn
34+
35+
36+
class TestCommitAsyncClearsMessagesInLock:
37+
def test_commit_async_clears_messages(self) -> None:
38+
conn = _build_conn_with_mocked_async()
39+
conn.messages.append((RuntimeError, "synthetic"))
40+
asyncio.run(conn._commit_async())
41+
assert conn.messages == []
42+
43+
def test_commit_async_clears_messages_before_execute(self) -> None:
44+
"""The clear runs before ``execute("COMMIT")`` so even an
45+
exception from the wire round-trip leaves ``messages`` empty
46+
— consistent with PEP 249 "cleared prior to executing the
47+
call"."""
48+
conn = _build_conn_with_mocked_async()
49+
conn.messages.append((RuntimeError, "stale"))
50+
# Make the COMMIT raise; messages should still be cleared.
51+
from dqlitedbapi import OperationalError
52+
53+
conn._async_conn.execute = AsyncMock( # type: ignore[union-attr]
54+
side_effect=OperationalError("boom")
55+
)
56+
with contextlib.suppress(OperationalError):
57+
asyncio.run(conn._commit_async())
58+
assert conn.messages == []
59+
60+
61+
class TestRollbackAsyncClearsMessagesInLock:
62+
def test_rollback_async_clears_messages(self) -> None:
63+
conn = _build_conn_with_mocked_async()
64+
conn.messages.append((RuntimeError, "synthetic"))
65+
asyncio.run(conn._rollback_async())
66+
assert conn.messages == []
67+
68+
def test_rollback_async_clears_messages_before_execute(self) -> None:
69+
conn = _build_conn_with_mocked_async()
70+
conn.messages.append((RuntimeError, "stale"))
71+
from dqlitedbapi import OperationalError
72+
73+
conn._async_conn.execute = AsyncMock( # type: ignore[union-attr]
74+
side_effect=OperationalError("boom")
75+
)
76+
with contextlib.suppress(OperationalError):
77+
asyncio.run(conn._rollback_async())
78+
assert conn.messages == []

0 commit comments

Comments
 (0)