From 971deb477c404338ef7ff76c6cbd34a39d236b37 Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Thu, 18 Jun 2026 22:21:56 -0400 Subject: [PATCH] Keep Python key buffers alive in WriteRange/ReadRange `write()` and `read()` created _Key objects from ephemeral ctypes arrays backed by local bytearray objects. Once the helpers returned, those local variables were freed, leaving the C library with dangling pointers when addWrites()/check() later read the keys. Store the backing bytearray on the returned WriteRange/ReadRange objects as private `_begin_buf` / `_end_buf` attributes. Python keeps them alive for the lifetime of the range object, so the C pointer is always valid. Closes #42 --- conflict_set.py | 32 +++++++++++++++++++++++--------- test_conflict_set.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/conflict_set.py b/conflict_set.py index f645343..82fe3fd 100644 --- a/conflict_set.py +++ b/conflict_set.py @@ -27,23 +27,37 @@ class Result(enum.Enum): TOO_OLD = 2 -def write(begin: bytes, end: Optional[bytes] = None) -> WriteRange: - b = (ctypes.c_ubyte * len(begin)).from_buffer(bytearray(begin)) +def _make_key(buf: bytes) -> tuple[_Key, bytearray]: + """Create a _Key and a backing bytearray that must be kept alive.""" + backing = bytearray(buf) + array = (ctypes.c_ubyte * len(backing)).from_buffer(backing) + return _Key(array, len(array)), backing + +def write(begin: bytes, end: Optional[bytes] = None) -> WriteRange: + begin_key, begin_buf = _make_key(begin) if end is None: - e = (ctypes.c_ubyte * 0)() + end_key = _Key((ctypes.c_ubyte * 0)(), 0) + end_buf = None else: - e = (ctypes.c_ubyte * len(end)).from_buffer(bytearray(end)) - return WriteRange(_Key(b, len(b)), _Key(e, len(e))) + end_key, end_buf = _make_key(end) + result = WriteRange(begin_key, end_key) + result._begin_buf = begin_buf + result._end_buf = end_buf + return result def read(version: int, begin: bytes, end: Optional[bytes] = None) -> ReadRange: - b = (ctypes.c_ubyte * len(begin)).from_buffer(bytearray(begin)) + begin_key, begin_buf = _make_key(begin) if end is None: - e = (ctypes.c_ubyte * 0)() + end_key = _Key((ctypes.c_ubyte * 0)(), 0) + end_buf = None else: - e = (ctypes.c_ubyte * len(end)).from_buffer(bytearray(end)) - return ReadRange(_Key(b, len(b)), _Key(e, len(e)), version) + end_key, end_buf = _make_key(end) + result = ReadRange(begin_key, end_key, version) + result._begin_buf = begin_buf + result._end_buf = end_buf + return result class ConflictSet: diff --git a/test_conflict_set.py b/test_conflict_set.py index 0b4b156..308dc04 100644 --- a/test_conflict_set.py +++ b/test_conflict_set.py @@ -57,6 +57,42 @@ def test_conflict_set(): assert cs.check(read(0, key), read(1, key)) == [Result.TOO_OLD, Result.COMMIT] +def test_write_read_without_outer_reference(): + # Regression test for issue #42: WriteRange/ReadRange must keep their + # backing key buffers alive, because the C library reads the pointer + # stored in _Key while addWrites/check run. + with DebugConflictSet() as cs: + # The bytes literal is not referenced after this expression. + cs.addWrites(1, write(b"key")) + assert cs.check(read(0, b"key")) == [Result.CONFLICT] + + cs.addWrites(2, write(b"a", b"z")) + assert cs.check(read(1, b"a", b"z")) == [Result.CONFLICT] + assert cs.check(read(1, b"b")) == [Result.CONFLICT] + assert cs.check(read(1, b"0")) == [Result.COMMIT] + + +def test_range_keeps_key_buffers_alive(): + # Verify the fix for issue #42: returned range objects must retain a + # reference to the backing bytearray so the C pointer stays valid after + # the helper returns. + w = write(b"key") + assert w._begin_buf == bytearray(b"key") + assert w._end_buf is None + + w2 = write(b"a", b"z") + assert w2._begin_buf == bytearray(b"a") + assert w2._end_buf == bytearray(b"z") + + r = read(0, b"key") + assert r._begin_buf == bytearray(b"key") + assert r._end_buf is None + + r2 = read(1, b"a", b"z") + assert r2._begin_buf == bytearray(b"a") + assert r2._end_buf == bytearray(b"z") + + def test_update_zero_should_commit(): with DebugConflictSet() as cs1: with DebugConflictSet() as cs2: