Keep Python key buffers alive in WriteRange/ReadRange
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m22s
CI / pre-commit (pull_request) Successful in 2m5s
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Successful in 3m38s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 3m32s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 3m40s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 3m28s
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Successful in 4m53s
CI / coverage (pull_request) Successful in 3m40s
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m22s
CI / pre-commit (pull_request) Successful in 2m5s
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Successful in 3m38s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 3m32s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 3m40s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 3m28s
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Successful in 4m53s
CI / coverage (pull_request) Successful in 3m40s
`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
This commit is contained in:
+23
-9
@@ -27,23 +27,37 @@ class Result(enum.Enum):
|
|||||||
TOO_OLD = 2
|
TOO_OLD = 2
|
||||||
|
|
||||||
|
|
||||||
def write(begin: bytes, end: Optional[bytes] = None) -> WriteRange:
|
def _make_key(buf: bytes) -> tuple[_Key, bytearray]:
|
||||||
b = (ctypes.c_ubyte * len(begin)).from_buffer(bytearray(begin))
|
"""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:
|
if end is None:
|
||||||
e = (ctypes.c_ubyte * 0)()
|
end_key = _Key((ctypes.c_ubyte * 0)(), 0)
|
||||||
|
end_buf = None
|
||||||
else:
|
else:
|
||||||
e = (ctypes.c_ubyte * len(end)).from_buffer(bytearray(end))
|
end_key, end_buf = _make_key(end)
|
||||||
return WriteRange(_Key(b, len(b)), _Key(e, len(e)))
|
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:
|
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:
|
if end is None:
|
||||||
e = (ctypes.c_ubyte * 0)()
|
end_key = _Key((ctypes.c_ubyte * 0)(), 0)
|
||||||
|
end_buf = None
|
||||||
else:
|
else:
|
||||||
e = (ctypes.c_ubyte * len(end)).from_buffer(bytearray(end))
|
end_key, end_buf = _make_key(end)
|
||||||
return ReadRange(_Key(b, len(b)), _Key(e, len(e)), version)
|
result = ReadRange(begin_key, end_key, version)
|
||||||
|
result._begin_buf = begin_buf
|
||||||
|
result._end_buf = end_buf
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
class ConflictSet:
|
class ConflictSet:
|
||||||
|
|||||||
@@ -57,6 +57,42 @@ def test_conflict_set():
|
|||||||
assert cs.check(read(0, key), read(1, key)) == [Result.TOO_OLD, Result.COMMIT]
|
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():
|
def test_update_zero_should_commit():
|
||||||
with DebugConflictSet() as cs1:
|
with DebugConflictSet() as cs1:
|
||||||
with DebugConflictSet() as cs2:
|
with DebugConflictSet() as cs2:
|
||||||
|
|||||||
Reference in New Issue
Block a user