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

.gitea/workflows/ci.yml

@@ -2,62 +2,38 @@ name: CI
 
 on: [push, pull_request]
 
+env:
+  CC: clang
+  CXX: clang++
+
 jobs:
-  build-image:
-    strategy:
-      fail-fast: false
-      matrix:
-        include:
-          - runner: ubuntu-latest-amd64
-            arch: amd64
-          - runner: ubuntu-latest-arm64
-            arch: arm64
-    runs-on: ${{ matrix.runner }}
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Log in to registry
-        env:
-          REGISTRY_USER: ${{ secrets.REGISTRY_USER }}
-          REGISTRY_TOKEN: ${{ secrets.REGISTRY_TOKEN }}
-        run: |
-          echo "$REGISTRY_TOKEN" \
-            | docker login -u "$REGISTRY_USER" --password-stdin git.weaselab.dev
-
-      - name: Build and push image if changed
-        run: |
-          image=git.weaselab.dev/weaselab/conflict-set-ci
-          hash="$(sha256sum Dockerfile .pre-commit-config.yaml | sha256sum | cut -c 1-16)"
-          latest="$image:latest-${{ matrix.arch }}"
-          current="$(docker buildx imagetools inspect "$latest" \
-            --format '{{index .Image.Config.Labels "dev.weaselab.ci-hash"}}' 2> /dev/null || true)"
-          if [ "$current" = "$hash" ]; then
-            echo "$latest is up to date"
-          else
-            docker build --push --label "dev.weaselab.ci-hash=$hash" -t "$latest" .
-          fi
-
   pre-commit:
-    needs: build-image
     runs-on: ubuntu-latest-amd64
-    container:
-      image: git.weaselab.dev/weaselab/conflict-set-ci:latest-amd64
-      credentials:
-        username: ${{ secrets.REGISTRY_USER }}
-        password: ${{ secrets.REGISTRY_TOKEN }}
     steps:
       - uses: actions/checkout@v4
 
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            clang git nodejs pre-commit
+
+      - uses: actions/cache@v4
+        with:
+          path: ~/.cache/pre-commit
+          key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
+
       - name: Run pre-commit
-        env:
-          # use the hooks pre-installed in the image
-          HOME: /tmp
         run: |
           git config --global --add safe.directory "$PWD"
           pre-commit run --all-files --show-diff-on-failure
 
   test:
-    needs: build-image
     strategy:
       fail-fast: false
       matrix:
@@ -71,14 +47,23 @@ jobs:
           - name: gcc
             cmake_args: -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
     runs-on: ubuntu-latest-amd64
-    container:
-      image: git.weaselab.dev/weaselab/conflict-set-ci:latest-amd64
-      credentials:
-        username: ${{ secrets.REGISTRY_USER }}
-        password: ${{ secrets.REGISTRY_TOKEN }}
     steps:
       - uses: actions/checkout@v4
 
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            build-essential ccache clang cmake gcc g++ \
+            libc6-dbg llvm mold ninja-build python3 valgrind zstd
+          sudo curl -Ls "https://dl.min.io/client/mc/release/linux-amd64/mc" \
+            -o /usr/local/bin/mc && sudo chmod +x /usr/local/bin/mc
+
       - uses: actions/cache@v4
         with:
           path: .ccache
@@ -120,7 +105,6 @@ jobs:
             | tee -a "$GITHUB_STEP_SUMMARY"
 
   release:
-    needs: build-image
     strategy:
       fail-fast: false
       matrix:
@@ -130,14 +114,25 @@ jobs:
           - runner: ubuntu-latest-arm64
             arch: arm64
     runs-on: ${{ matrix.runner }}
-    container:
-      image: git.weaselab.dev/weaselab/conflict-set-ci:latest-${{ matrix.arch }}
-      credentials:
-        username: ${{ secrets.REGISTRY_USER }}
-        password: ${{ secrets.REGISTRY_TOKEN }}
     steps:
       - uses: actions/checkout@v4
 
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-${{ matrix.arch }}-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            biber build-essential ccache clang cmake devscripts \
+            latexmk libc6-dbg llvm mold ninja-build rpm \
+            texlive-bibtex-extra texlive-fonts-recommended \
+            texlive-latex-extra texlive-pictures valgrind zstd
+          sudo curl -Ls "https://dl.min.io/client/mc/release/linux-$(dpkg --print-architecture)/mc" \
+            -o /usr/local/bin/mc && sudo chmod +x /usr/local/bin/mc
+
       - uses: actions/cache@v4
         with:
           path: .ccache
@@ -188,9 +183,6 @@ jobs:
           dest="minio/jenkins/conflict-set/${{ gitea.run_number }}/release-${{ matrix.arch }}/"
           zstd build/Testing/*/Test.xml
           mc cp build/Testing/*/Test.xml.zst "$dest"
-          # This step runs even when a previous step failed, to upload test
-          # results. The packages may never have been built though, so skip
-          # them if they're missing.
           if compgen -G "build/*.deb" > /dev/null; then
             mc cp build/*.deb "$dest"
           fi
@@ -209,16 +201,24 @@ jobs:
             | tee -a "$GITHUB_STEP_SUMMARY"
 
   coverage:
-    needs: build-image
     runs-on: ubuntu-latest-amd64
-    container:
-      image: git.weaselab.dev/weaselab/conflict-set-ci:latest-amd64
-      credentials:
-        username: ${{ secrets.REGISTRY_USER }}
-        password: ${{ secrets.REGISTRY_TOKEN }}
     steps:
       - uses: actions/checkout@v4
 
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            build-essential ccache clang cmake gcovr \
+            libc6-dbg llvm mold ninja-build python3 valgrind zstd
+          sudo curl -Ls "https://dl.min.io/client/mc/release/linux-amd64/mc" \
+            -o /usr/local/bin/mc && sudo chmod +x /usr/local/bin/mc
+
       - uses: actions/cache@v4
         with:
           path: .ccache

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:

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: