Keep Python key buffers alive in WriteRange/ReadRange
CI / build-image (arm64, ubuntu-latest-arm64) (pull_request) Failing after 20s
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Has been skipped
CI / build-image (amd64, ubuntu-latest-amd64) (pull_request) Failing after 37s
CI / pre-commit (pull_request) Has been skipped
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Has been skipped
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Has been skipped
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Has been skipped
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Has been skipped
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Has been skipped
CI / coverage (pull_request) Has been skipped
CI / build-image (arm64, ubuntu-latest-arm64) (pull_request) Failing after 20s
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Has been skipped
CI / build-image (amd64, ubuntu-latest-amd64) (pull_request) Failing after 37s
CI / pre-commit (pull_request) Has been skipped
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Has been skipped
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Has been skipped
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Has been skipped
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Has been skipped
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Has been skipped
CI / coverage (pull_request) Has been skipped
`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
|
||||
|
||||
|
||||
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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user