Skip to content

Commit ec2bf58

Browse files
fix: make ReadBuffer.clear() unpoison the buffer
ReadBuffer.clear() and ReadBuffer.reset() were silently asymmetric: reset() (added alongside the poison concept in issue 026) cleared _poisoned, while clear() (which predated the poison concept) did not. A caller reaching for clear() as "the simpler recovery primitive" got a half-fresh buffer: bytes gone, offset reset, but still raising ProtocolError from the poison gate on the next operation. Make clear() a thin alias for reset(). Behaviourally equivalent for a clean buffer, correct for a poisoned one. This is also a prerequisite for gating the ReadBuffer public API on the poison flag (issue 038), since otherwise any caller who used clear() as their recovery path would be locked out. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4315493 commit ec2bf58

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

src/dqlitewire/buffer.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,14 @@ def is_skipping(self) -> bool:
282282
return self._skip_remaining > 0
283283

284284
def clear(self) -> None:
285-
"""Clear the buffer."""
286-
self._data.clear()
287-
self._pos = 0
288-
self._skip_remaining = 0
285+
"""Clear buffer state and un-poison.
286+
287+
Equivalent to ``reset()``. Kept as a convenience alias because
288+
``clear()`` predates the poison concept (issue 026) and was
289+
briefly inconsistent with ``reset()`` — it used to leave the
290+
``_poisoned`` flag intact, which meant a caller who reached
291+
for ``clear()`` as a recovery primitive got a half-fresh
292+
buffer that still raised ``ProtocolError`` on the next
293+
operation (issue 040).
294+
"""
295+
self.reset()

tests/test_buffer.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,35 @@ def test_clear(self) -> None:
187187
buf.clear()
188188
assert buf.available() == 0
189189

190+
def test_clear_also_unpoisons(self) -> None:
191+
"""Regression for issue 040.
192+
193+
``ReadBuffer`` has two very similar methods: ``clear()`` and
194+
``reset()``. ``reset()`` was introduced by issue 026 as the
195+
recovery primitive for the poison flag. ``clear()`` predates
196+
the poison concept and was never updated — it still resets
197+
``_data``/``_pos``/``_skip_remaining`` but leaves
198+
``_poisoned`` intact. A caller who reaches for ``clear()``
199+
expecting it to be "the simpler recovery method" gets a
200+
half-fresh buffer that still raises ``ProtocolError`` on the
201+
next operation. The asymmetry is invisible at the API level
202+
(one-line docstrings, similar names), so the only safe
203+
behaviour is for both methods to unpoison.
204+
"""
205+
from dqlitewire.exceptions import DecodeError
206+
207+
buf = ReadBuffer()
208+
buf.feed(b"\x00" * 16)
209+
buf.poison(DecodeError("boom"))
210+
assert buf.is_poisoned
211+
212+
buf.clear()
213+
214+
assert not buf.is_poisoned
215+
# And the buffer must be usable after the clear+reconnect.
216+
buf.feed(b"\x01\x00\x00\x00\x00\x00\x00\x00")
217+
assert buf.available() == 8
218+
190219
def test_has_message_is_total_on_oversized(self) -> None:
191220
"""has_message() must not raise — oversized headers surface at consume time.
192221

0 commit comments

Comments
 (0)