Compare commits

...
24 Commits
Author SHA1 Message Date
weaselbot dee3a8f640 Return 0 instead of -1 from hash_table getBytes()
The hash_table implementation cannot accurately track memory usage and was
returning -1 from ConflictSet::getBytes() and ConflictSet_getBytes(). That
violates the API contract that getBytes() returns a non-negative byte count.

Change both entry points to return 0 and document in ConflictSet.h that
implementations which do not track memory usage may return 0.

Add a regression test in test_conflict_set.py that loads the hash_table
implementation and verifies getBytes() is non-negative.

Closes #62
2026-07-07 11:00:52 -04:00
andrew 732d19efa1 Merge pull request 'Fix RealDataBench subspan and version API contract violations' (#61) from weaselbot/conflict-set:weaselbot/issue-58 into main
Reviewed-on: weaselab/conflict-set#61
2026-06-29 18:49:58 +00:00
andrew 9449190d02 Merge pull request 'Fix undefined behavior on empty input in strinc() and prefixRange()' (#60) from weaselbot/conflict-set:weaselbot/issue-59 into main
Reviewed-on: weaselab/conflict-set#60
2026-06-29 18:26:04 +00:00
weaselbot 4fcdc5d7e9 Fix RealDataBench subspan and version API contract violations
- Use subspan count `line.size() - 2` instead of `line.size()` to avoid reading past the line bounds, and guard lines shorter than the two-byte prefix.
- Clamp `readVersion` and `setOldestVersion` arguments to `0` so the conflict set never receives negative, non-monotonic versions during the warmup phase.

Fixes #58.
2026-06-29 14:01:23 -04:00
weaselbot 63f9a139da Fix undefined behavior on empty input in strinc() and prefixRange()
strinc() in ConflictSet.cpp used std::string_view::size() (size_t) and subtracted 1 without first checking for an empty string. For the root node, getSearchPath() returns the empty string, so every debug correctness check underflowed size_t and relied on implementation-defined conversion to signed int.

prefixRange() in Bench.cpp had the same loop shape. Although TrivialSpan::size() returns int, on an empty (or all-0xff) key the function then asserted and continued executing, allocating a zero-length buffer and writing before its start.

Changes:
- In strinc(), initialize index as signed int(str.size()) - 1 so the loop is skipped for empty input, and return ok=false cleanly.
- In prefixRange(), initialize index the same way and call std::abort() after the assert so invalid input cannot fall through to an out-of-bounds write.
- Replace C-style uint8_t casts with explicit static_casts.

Closes #59
2026-06-29 13:52:02 -04:00
andrew 8f9f345c64 Merge pull request 'Keep Python wrapper key buffers alive in WriteRange/ReadRange' (#43) from weaselbot/conflict-set:weaselbot/issue-42 into main
Reviewed-on: weaselab/conflict-set#43
Reviewed-by: andrew <andrew@weaselab.dev>
2026-06-23 01:17:45 +00:00
andrew b9b2d69dd5 Merge pull request 'Disallow copying ConflictSet in C++98/C++03' (#51) from weaselbot/conflict-set:weaselbot/issue-48 into main
Reviewed-on: weaselab/conflict-set#51
2026-06-22 23:44:59 +00:00
andrew d3c8f4afc6 Merge pull request 'Include <sys/syscall.h> in ServerBench.cpp for SYS_perf_event_open' (#44) from weaselbot/conflict-set:weaselbot/issue-41 into main
Reviewed-on: weaselab/conflict-set#44
2026-06-22 20:01:43 +00:00
andrew 549724f09e Merge pull request 'Assert return values in test_update_zero_should_commit / conflict' (#50) from weaselbot/conflict-set:weaselbot/issue-49 into main
Reviewed-on: weaselab/conflict-set#50
2026-06-22 19:29:55 +00:00
andrew 52eb13cc0b Merge pull request 'Fix CMake Unix Makefiles parallel build race for conflict-set.o' (#52) from weaselbot/conflict-set:weaselbot/issue-47 into main
Reviewed-on: weaselab/conflict-set#52
2026-06-22 19:08:09 +00:00
andrew f30887f280 Merge pull request 'Fix move-assignment leak and self-assignment in ConflictSet' (#55) from weaselbot/conflict-set:weaselbot/issue-54 into main
Reviewed-on: weaselab/conflict-set#55
2026-06-22 18:40:25 +00:00
andrew ccd637deab We already know __cplusplus is defined 2026-06-22 14:18:33 -04:00
weaselbotandandrew 971deb477c 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
2026-06-22 14:09:18 -04:00
weaselbotandandrew ff0722728a Include <sys/syscall.h> in ServerBench.cpp for SYS_perf_event_open
`ServerBench.cpp` calls `syscall(SYS_perf_event_open, ...)` but did
not include `<sys/syscall.h>`, relying on `<unistd.h>` to transitively
provide the `SYS_*` constants. On toolchains where that does not happen,
the build fails with `SYS_perf_event_open` not declared.

Add the missing include so `server_bench` compiles on any platform that
provides `perf_event_open`.

Resolves weaselab/conflict-set#41.
2026-06-22 14:09:14 -04:00
weaselbotandandrew 60881419b8 Assert return values in test_update_zero_should_commit / conflict
Add the missing [Result.COMMIT] / [Result.CONFLICT] assertions so
that the two regression tests actually verify the expected conflict-set
outcome, not just that the calls do not crash.
2026-06-22 14:09:10 -04:00
weaselbotandandrew 4515af3662 CMake: add target-level dependencies for conflict-set-object
The custom command that links conflict-set.o depends on
$<TARGET_OBJECTS:conflict-set-object>, but that generator expression
does not create a target-level dependency.  With the Unix Makefiles
generator, parallel builds can start building the consuming libraries
before the object library's build rule is available, producing:

  gmake[3]: *** No rule to make target
  'CMakeFiles/conflict-set-object.dir/ConflictSet.cpp.o', needed by
  'conflict-set.o'.  Stop.

Add add_dependencies() so that conflict-set and conflict-set-static
cannot build until conflict-set-object has produced its object files.

Closes #47
2026-06-22 14:09:04 -04:00
weaselbotandandrew 4dc5f7f75c Fix self-move-assignment and leak in ConflictSet move-assignment
The user-declared move-assignment operator overwrote `impl` without first
destroying the existing implementation object, leaking all memory and resources
owned by the left-hand side. Self-move-assignment also set `impl` to nullptr,
leaving the object invalid and leaking the old state.

Fix all three implementations (ConflictSet.cpp, SkipList.cpp, HashTable.cpp)
to guard against self-assignment and to destroy/free the old `impl` before
taking ownership of `other.impl`.
2026-06-22 14:08:59 -04:00
weaselbot 7eaac2a184 Make ConflictSet non-copyable in C++98/C++03
`ConflictSet(const ConflictSet&)` and `operator=(const ConflictSet&)` were
only deleted for C++11 and later. In C++98/C++03 the compiler implicitly
generated public copy operations, so copying a ConflictSet shared the opaque
`Impl*` and caused a double-free on destruction.

Declare both operations private and leave them undefined when
`__cplusplus <= 199711L`, matching the standard pre-C++11 idiom for
move-only types. Guard the declarations with `defined(__cplusplus)` so
they are not exposed to C90 compilation units.

Closes #48
2026-06-22 13:43:04 -04:00
andrew e9c904a86b CMakeLists.txt: don't require hardening-check --help to exit 0
hardening-check --help returns exit code 1, so the previous
hardening_check_help_result EQUAL 0 guard skipped parsing the help
output entirely. As a result, architecture-specific flags such as
--nobranchprotection on x86_64 were never added to the test command.

Parse the help output regardless of exit code; the option-string regex
checks are sufficient.
2026-06-22 13:05:33 -04:00
andrew 789ae8cbb9 ci: install clang/LLVM 21 in Gitea Actions workflows
Switch all CI jobs from the distro-packaged clang to the apt.llvm.org
clang-21 / llvm-21 toolchain, and register the versioned binaries as
alternatives so that CC/CXX=clang/clang++ and tools like llvm-cov and
llvm-objcopy use the newer release automatically.
2026-06-22 12:26:19 -04:00
andrew d70e6a2455 Merge pull request 'Set restype=None for void-returning C functions' (#53) from weaselbot/conflict-set:weaselbot/issue-46 into main
Reviewed-on: weaselab/conflict-set#53
2026-06-22 01:12:47 +00:00
andrew 8a5168f232 CMakeLists: only pass hardening-check arch flags the tool supports
The amd64 CI runner's hardening-check does not recognize
--nobranchprotection, causing the hardening_check test to fail at
configure time. Query the tool's help output and only include the
architecture-specific skip flags when they are advertised.
2026-06-21 21:07:26 -04:00
weaselbotandandrew 742d920aa1 Set restype=None for void-returning C functions in conflict_set.py
ConflictSet_check, ConflictSet_addWrites, ConflictSet_setOldestVersion,
and ConflictSet_destroy return void in C, but the Python wrapper left
their ctypes restype at the default c_int. Set restype = None for each
to match the C API contract and avoid undefined behavior from reading
the return register of void functions.
2026-06-21 19:25:27 -04:00
andrew 6d8b939a81 Replace CI Docker image with inline apt installs
Drops the build-image job and the private registry dependency entirely.
Each job now installs only the packages it needs, caching /var/cache/apt/archives
keyed on the workflow file and ~/.cache/pre-commit keyed on .pre-commit-config.yaml.
2026-06-21 19:21:04 -04:00
11 changed files with 237 additions and 100 deletions
+88 -64
View File
@@ -2,62 +2,44 @@ name: CI
on: [push, pull_request] on: [push, pull_request]
env:
CC: clang
CXX: clang++
jobs: 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: pre-commit:
needs: build-image
runs-on: ubuntu-latest-amd64 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: steps:
- uses: actions/checkout@v4 - 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: |
. /etc/os-release
wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc
echo "deb http://apt.llvm.org/${VERSION_CODENAME}/ llvm-toolchain-${VERSION_CODENAME}-21 main" | sudo tee /etc/apt/sources.list.d/llvm.list
sudo apt-get update -qq
sudo apt-get install -y \
clang-21 git nodejs pre-commit
for tool in clang clang++; do
sudo update-alternatives --install /usr/bin/${tool} ${tool} /usr/bin/${tool}-21 100
done
- uses: actions/cache@v4
with:
path: ~/.cache/pre-commit
key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
- name: Run pre-commit - name: Run pre-commit
env:
# use the hooks pre-installed in the image
HOME: /tmp
run: | run: |
git config --global --add safe.directory "$PWD" git config --global --add safe.directory "$PWD"
pre-commit run --all-files --show-diff-on-failure pre-commit run --all-files --show-diff-on-failure
test: test:
needs: build-image
strategy: strategy:
fail-fast: false fail-fast: false
matrix: matrix:
@@ -71,14 +53,29 @@ jobs:
- name: gcc - name: gcc
cmake_args: -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ cmake_args: -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
runs-on: ubuntu-latest-amd64 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: steps:
- uses: actions/checkout@v4 - 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: |
. /etc/os-release
wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc
echo "deb http://apt.llvm.org/${VERSION_CODENAME}/ llvm-toolchain-${VERSION_CODENAME}-21 main" | sudo tee /etc/apt/sources.list.d/llvm.list
sudo apt-get update -qq
sudo apt-get install -y \
build-essential ccache clang-21 cmake gcc g++ \
libc6-dbg llvm-21 lld-21 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
for tool in clang clang++ llvm-ar llvm-nm llvm-ranlib llvm-objcopy llvm-cov llvm-symbolizer lld ld.lld; do
sudo update-alternatives --install /usr/bin/${tool} ${tool} /usr/bin/${tool}-21 100
done
- uses: actions/cache@v4 - uses: actions/cache@v4
with: with:
path: .ccache path: .ccache
@@ -120,7 +117,6 @@ jobs:
| tee -a "$GITHUB_STEP_SUMMARY" | tee -a "$GITHUB_STEP_SUMMARY"
release: release:
needs: build-image
strategy: strategy:
fail-fast: false fail-fast: false
matrix: matrix:
@@ -130,14 +126,31 @@ jobs:
- runner: ubuntu-latest-arm64 - runner: ubuntu-latest-arm64
arch: arm64 arch: arm64
runs-on: ${{ matrix.runner }} 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: steps:
- uses: actions/checkout@v4 - 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: |
. /etc/os-release
wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc
echo "deb http://apt.llvm.org/${VERSION_CODENAME}/ llvm-toolchain-${VERSION_CODENAME}-21 main" | sudo tee /etc/apt/sources.list.d/llvm.list
sudo apt-get update -qq
sudo apt-get install -y \
biber build-essential ccache clang-21 cmake devscripts \
latexmk libc6-dbg llvm-21 lld-21 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
for tool in clang clang++ llvm-ar llvm-nm llvm-ranlib llvm-objcopy llvm-cov llvm-symbolizer lld ld.lld; do
sudo update-alternatives --install /usr/bin/${tool} ${tool} /usr/bin/${tool}-21 100
done
- uses: actions/cache@v4 - uses: actions/cache@v4
with: with:
path: .ccache path: .ccache
@@ -188,9 +201,6 @@ jobs:
dest="minio/jenkins/conflict-set/${{ gitea.run_number }}/release-${{ matrix.arch }}/" dest="minio/jenkins/conflict-set/${{ gitea.run_number }}/release-${{ matrix.arch }}/"
zstd build/Testing/*/Test.xml zstd build/Testing/*/Test.xml
mc cp build/Testing/*/Test.xml.zst "$dest" 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 if compgen -G "build/*.deb" > /dev/null; then
mc cp build/*.deb "$dest" mc cp build/*.deb "$dest"
fi fi
@@ -209,16 +219,30 @@ jobs:
| tee -a "$GITHUB_STEP_SUMMARY" | tee -a "$GITHUB_STEP_SUMMARY"
coverage: coverage:
needs: build-image
runs-on: ubuntu-latest-amd64 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: steps:
- uses: actions/checkout@v4 - 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: |
. /etc/os-release
wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc
echo "deb http://apt.llvm.org/${VERSION_CODENAME}/ llvm-toolchain-${VERSION_CODENAME}-21 main" | sudo tee /etc/apt/sources.list.d/llvm.list
sudo apt-get update -qq
sudo apt-get install -y \
build-essential ccache clang-21 cmake gcovr \
libc6-dbg llvm-21 lld-21 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
for tool in clang clang++ llvm-ar llvm-nm llvm-ranlib llvm-objcopy llvm-cov llvm-symbolizer lld ld.lld; do
sudo update-alternatives --install /usr/bin/${tool} ${tool} /usr/bin/${tool}-21 100
done
- uses: actions/cache@v4 - uses: actions/cache@v4
with: with:
path: .ccache path: .ccache
+5 -4
View File
@@ -37,15 +37,16 @@ ConflictSet::ReadRange singleton(Arena &arena, TrivialSpan key) {
} }
ConflictSet::ReadRange prefixRange(Arena &arena, TrivialSpan key) { ConflictSet::ReadRange prefixRange(Arena &arena, TrivialSpan key) {
int index; int index = key.size() - 1;
for (index = key.size() - 1; index >= 0; index--) for (; index >= 0; index--)
if ((key[index]) != 255) if (key[index] != 255)
break; break;
// Must not be called with a string that consists only of zero or more '\xff' // Must not be called with a string that consists only of zero or more '\xff'
// bytes. // bytes, or with an empty string (which has no finite upper bound).
if (index < 0) { if (index < 0) {
assert(false); assert(false);
std::abort();
} }
uint8_t *buf = new (arena) uint8_t[index + 1]; uint8_t *buf = new (arena) uint8_t[index + 1];
+16 -2
View File
@@ -139,6 +139,7 @@ add_custom_command(
COMMAND_EXPAND_LISTS) COMMAND_EXPAND_LISTS)
add_library(${PROJECT_NAME} SHARED ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o) add_library(${PROJECT_NAME} SHARED ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o)
add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-object)
set_target_properties( set_target_properties(
${PROJECT_NAME} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${PROJECT_NAME} PROPERTIES LIBRARY_OUTPUT_DIRECTORY
"${CMAKE_CURRENT_BINARY_DIR}/radix_tree") "${CMAKE_CURRENT_BINARY_DIR}/radix_tree")
@@ -155,6 +156,7 @@ if(HAS_VERSION_SCRIPT)
endif() endif()
add_library(${PROJECT_NAME}-static STATIC ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o) add_library(${PROJECT_NAME}-static STATIC ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o)
add_dependencies(${PROJECT_NAME}-static ${PROJECT_NAME}-object)
if(CMAKE_BUILD_TYPE STREQUAL Debug) if(CMAKE_BUILD_TYPE STREQUAL Debug)
set_target_properties(${PROJECT_NAME}-static PROPERTIES LINKER_LANGUAGE CXX) set_target_properties(${PROJECT_NAME}-static PROPERTIES LINKER_LANGUAGE CXX)
else() else()
@@ -383,13 +385,25 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
if(NOT CMAKE_CROSSCOMPILING) if(NOT CMAKE_CROSSCOMPILING)
find_program(HARDENING_CHECK hardening-check) find_program(HARDENING_CHECK hardening-check)
if(HARDENING_CHECK) if(HARDENING_CHECK)
# Not all versions of hardening-check support the same options, so query
# the help output before using architecture-specific skips.
execute_process(
COMMAND ${HARDENING_CHECK} --help
OUTPUT_VARIABLE hardening_check_help
ERROR_VARIABLE hardening_check_help
OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_STRIP_TRAILING_WHITESPACE)
set(hardening_check_arch_flags "")
# Control flow integrity (CET) is x86-only and branch protection (PAC/BTI) # Control flow integrity (CET) is x86-only and branch protection (PAC/BTI)
# is arm64-only, so ignore whichever doesn't apply. # is arm64-only, so ignore whichever doesn't apply.
if(CMAKE_SYSTEM_PROCESSOR STREQUAL aarch64 OR CMAKE_SYSTEM_PROCESSOR if(CMAKE_SYSTEM_PROCESSOR STREQUAL aarch64 OR CMAKE_SYSTEM_PROCESSOR
STREQUAL arm64) STREQUAL arm64)
set(hardening_check_arch_flags --nocfprotection) if(hardening_check_help MATCHES "nocfprotection")
list(APPEND hardening_check_arch_flags --nocfprotection)
endif()
else() else()
set(hardening_check_arch_flags --nobranchprotection) if(hardening_check_help MATCHES "nobranchprotection")
list(APPEND hardening_check_arch_flags --nobranchprotection)
endif()
endif() endif()
add_test( add_test(
NAME hardening_check NAME hardening_check
+11 -5
View File
@@ -5589,7 +5589,12 @@ ConflictSet::ConflictSet(ConflictSet &&other) noexcept
: impl(std::exchange(other.impl, nullptr)) {} : impl(std::exchange(other.impl, nullptr)) {}
ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept { ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
if (this != &other) {
if (impl) {
internal_destroy(impl);
}
impl = std::exchange(other.impl, nullptr); impl = std::exchange(other.impl, nullptr);
}
return *this; return *this;
} }
@@ -5674,13 +5679,13 @@ std::string getPartialKeyPrintable(Node *n) {
} }
std::string strinc(std::string_view str, bool &ok) { std::string strinc(std::string_view str, bool &ok) {
int index; int index = static_cast<int>(str.size()) - 1;
for (index = str.size() - 1; index >= 0; index--) for (; index >= 0; index--)
if ((uint8_t &)(str[index]) != 255) if (static_cast<uint8_t>(str[index]) != 255)
break; break;
// Must not be called with a string that consists only of zero or more // Must not be called with a string that consists only of zero or more
// '\xff' bytes. // '\xff' bytes, and the empty string has no successor.
if (index < 0) { if (index < 0) {
ok = false; ok = false;
return {}; return {};
@@ -5688,7 +5693,8 @@ std::string strinc(std::string_view str, bool &ok) {
ok = true; ok = true;
auto r = std::string(str.substr(0, index + 1)); auto r = std::string(str.substr(0, index + 1));
((uint8_t &)r[r.size() - 1])++; auto &last = r[r.size() - 1];
last = static_cast<char>(static_cast<uint8_t>(last) + 1);
return r; return r;
} }
+14 -3
View File
@@ -96,7 +96,9 @@ void ConflictSet::setOldestVersion(int64_t oldestVersion) {
return impl->setOldestVersion(oldestVersion); return impl->setOldestVersion(oldestVersion);
} }
int64_t ConflictSet::getBytes() const { return -1; } // The hash_table implementation does not track memory usage, so return 0 to
// satisfy the API contract that getBytes() returns a non-negative value.
int64_t ConflictSet::getBytes() const { return 0; }
void ConflictSet::getMetricsV1(MetricsV1 **metrics, int *count) const { void ConflictSet::getMetricsV1(MetricsV1 **metrics, int *count) const {
*metrics = nullptr; *metrics = nullptr;
@@ -119,7 +121,13 @@ ConflictSet::ConflictSet(ConflictSet &&other) noexcept
: impl(std::exchange(other.impl, nullptr)) {} : impl(std::exchange(other.impl, nullptr)) {}
ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept { ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
if (this != &other) {
if (impl) {
impl->~Impl();
safe_free(impl, sizeof(Impl));
}
impl = std::exchange(other.impl, nullptr); impl = std::exchange(other.impl, nullptr);
}
return *this; return *this;
} }
@@ -155,7 +163,10 @@ __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
} }
__attribute__((__visibility__("default"))) int64_t __attribute__((__visibility__("default"))) int64_t
ConflictSet_getBytes(void *cs) { ConflictSet_getBytes(void *cs) {
using Impl = ConflictSet::Impl; (void)cs;
return -1; // The hash_table implementation does not track memory usage, so return 0 to
// satisfy the API contract that ConflictSet_getBytes returns a non-negative
// value.
return 0;
} }
} }
+7 -6
View File
@@ -1,5 +1,6 @@
#include <ConflictSet.h> #include <ConflictSet.h>
#include <algorithm>
#include <cerrno> #include <cerrno>
#include <chrono> #include <chrono>
#include <cstdio> #include <cstdio>
@@ -77,10 +78,10 @@ int main(int argc, const char **argv) {
begin = end + 1; begin = end + 1;
end = (uint8_t *)memchr(begin, '\n', size); end = (uint8_t *)memchr(begin, '\n', size);
if (line.size() > 0 && line[0] == 'P') { if (line.size() >= 2 && line[0] == 'P') {
write = line.subspan(2, line.size()); write = line.subspan(2, line.size() - 2);
} else if (line.size() > 0 && line[0] == 'L') { } else if (line.size() >= 2 && line[0] == 'L') {
reads.push_back(line.subspan(2, line.size())); reads.push_back(line.subspan(2, line.size() - 2));
} else if (line.empty()) { } else if (line.empty()) {
{ {
readRanges.resize(reads.size()); readRanges.resize(reads.size());
@@ -90,7 +91,7 @@ int main(int argc, const char **argv) {
iter->begin.len = read.size(); iter->begin.len = read.size();
checkBytes += read.size(); checkBytes += read.size();
iter->end.len = 0; iter->end.len = 0;
iter->readVersion = version - 100; iter->readVersion = std::max<int64_t>(0, version - 100);
++iter; ++iter;
} }
} }
@@ -121,7 +122,7 @@ int main(int argc, const char **argv) {
} }
timer = now(); timer = now();
cs.setOldestVersion(version - 10000); cs.setOldestVersion(std::max<int64_t>(0, version - 10000));
gcTime += now() - timer; gcTime += now() - timer;
} }
} }
+1
View File
@@ -12,6 +12,7 @@
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <sys/resource.h> #include <sys/resource.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/syscall.h>
#include <sys/types.h> #include <sys/types.h>
#include <sys/uio.h> #include <sys/uio.h>
#include <thread> #include <thread>
+5
View File
@@ -981,7 +981,12 @@ ConflictSet::ConflictSet(ConflictSet &&other) noexcept
: impl(std::exchange(other.impl, nullptr)) {} : impl(std::exchange(other.impl, nullptr)) {}
ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept { ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
if (this != &other) {
if (impl) {
internal_destroy(impl);
}
impl = std::exchange(other.impl, nullptr); impl = std::exchange(other.impl, nullptr);
}
return *this; return *this;
} }
+27 -9
View File
@@ -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:
@@ -88,6 +102,7 @@ class ConflictSet:
ctypes.POINTER(ctypes.c_int), ctypes.POINTER(ctypes.c_int),
ctypes.c_int, ctypes.c_int,
) )
self._lib.ConflictSet_check.restype = None
self._lib.ConflictSet_addWrites.argtypes = ( self._lib.ConflictSet_addWrites.argtypes = (
ctypes.c_void_p, ctypes.c_void_p,
@@ -95,13 +110,16 @@ class ConflictSet:
ctypes.c_int, ctypes.c_int,
ctypes.c_int64, ctypes.c_int64,
) )
self._lib.ConflictSet_addWrites.restype = None
self._lib.ConflictSet_setOldestVersion.argtypes = ( self._lib.ConflictSet_setOldestVersion.argtypes = (
ctypes.c_void_p, ctypes.c_void_p,
ctypes.c_int64, ctypes.c_int64,
) )
self._lib.ConflictSet_setOldestVersion.restype = None
self._lib.ConflictSet_destroy.argtypes = (ctypes.c_void_p,) self._lib.ConflictSet_destroy.argtypes = (ctypes.c_void_p,)
self._lib.ConflictSet_destroy.restype = None
self._lib.ConflictSet_getBytes.argtypes = (ctypes.c_void_p,) self._lib.ConflictSet_getBytes.argtypes = (ctypes.c_void_p,)
self._lib.ConflictSet_getBytes.restype = ctypes.c_int64 self._lib.ConflictSet_getBytes.restype = ctypes.c_int64
+11 -2
View File
@@ -88,7 +88,8 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
~ConflictSet(); ~ConflictSet();
/** Returns the total bytes in use by this ConflictSet */ /** Returns the total bytes in use by this ConflictSet. Implementations that
* do not track memory usage return 0. */
int64_t getBytes() const; int64_t getBytes() const;
/** Experimental! */ /** Experimental! */
@@ -132,6 +133,13 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
private: private:
Impl *impl; Impl *impl;
#if __cplusplus <= 199711L
/* Declared private and left undefined to prevent copying in C++98/C++03.
The compiler would otherwise implicitly generate public copy operations,
which share the opaque Impl* and cause a double-free. */
ConflictSet(const ConflictSet &);
ConflictSet &operator=(const ConflictSet &);
#endif
}; };
} /* namespace weaselab */ } /* namespace weaselab */
@@ -211,7 +219,8 @@ ConflictSet *ConflictSet_create(int64_t oldestVersion);
void ConflictSet_destroy(ConflictSet *cs); void ConflictSet_destroy(ConflictSet *cs);
/** Returns the total bytes in use by this ConflictSet */ /** Returns the total bytes in use by this ConflictSet. Implementations that
* do not track memory usage return 0. */
int64_t ConflictSet_getBytes(const ConflictSet *cs); int64_t ConflictSet_getBytes(const ConflictSet *cs);
#endif #endif
+49 -2
View File
@@ -57,6 +57,53 @@ 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_hash_table_getBytes():
# Regression test for issue #62: the hash_table implementation is
# point-query only and does not track memory usage, but getBytes() must
# still return a non-negative value rather than -1.
with ConflictSet(0, build_dir=build_dir, implementation="hash_table") as cs:
assert cs.getBytes() == 0
cs.addWrites(1, write(b"key"))
assert cs.getBytes() >= 0
assert cs.check(read(0, b"key")) == [Result.CONFLICT]
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:
@@ -68,7 +115,7 @@ def test_update_zero_should_commit():
for i in range(256 - 17, 256): for i in range(256 - 17, 256):
cs2.addWrites(int(1), write(bytes([i]))) cs2.addWrites(int(1), write(bytes([i])))
# Scan until first point write # Scan until first point write
cs2.check(read(0, b"\x00", bytes([256 - 17]))) assert cs2.check(read(0, b"\x00", bytes([256 - 17]))) == [Result.COMMIT]
def test_update_zero_should_conflict(): def test_update_zero_should_conflict():
@@ -81,7 +128,7 @@ def test_update_zero_should_conflict():
# "zero" is now 2**31 + 100 # "zero" is now 2**31 + 100
cs1.addWrites(2**32 + 101, write(b"", b"\x02"), write(b"\x01")) cs1.addWrites(2**32 + 101, write(b"", b"\x02"), write(b"\x01"))
# rangeVersion of \x01 is now 2**31 + 100 ("max" of (2**31 + 100, 2**32 + 101)) # rangeVersion of \x01 is now 2**31 + 100 ("max" of (2**31 + 100, 2**32 + 101))
cs1.check(read(2**32 + 1, b"\x00")) assert cs1.check(read(2**32 + 1, b"\x00")) == [Result.CONFLICT]
# but 2**32 + 1 ">" 2**31 + 100 , and it incorrectly commits # but 2**32 + 1 ">" 2**31 + 100 , and it incorrectly commits