Skip to content

Fix non-atomic refcount decrement - #859

Merged
lemire merged 1 commit into
RoaringBitmap:masterfrom
aviggiano:agent/fix-non-atomic-refcount-decrement
Aug 8, 2026
Merged

Fix non-atomic refcount decrement#859
lemire merged 1 commit into
RoaringBitmap:masterfrom
aviggiano:agent/fix-non-atomic-refcount-decrement

Conversation

@aviggiano

Copy link
Copy Markdown
Contributor

Summary

  • compare the decremented reference count, rather than its pointer, in the non-atomic fallback
  • add a dedicated regression test that forces CROARING_ATOMIC_IMPL_NONE

Root cause and impact

croaring_refcount_dec() decremented *val but returned val == 0. Since callers pass a valid pointer, the function could never report that the final reference had been released. This prevents shared containers from being freed and makes extraction take an unnecessary copy path when the non-atomic fallback is selected.

Fixes #858.

This was found while reviewing the vendored CRoaring integration in redis/redis#15331.

Validation

  • confirmed the new regression test fails on unmodified master
  • cmake -S . -B build-release -DCMAKE_BUILD_TYPE=Release -DENABLE_ROARING_TESTS=ON
  • cmake --build build-release -j2
  • ctest --test-dir build-release --output-on-failure — 27/27 passed
  • forced-fallback test compiled as C++ with Clang 18 and passed
  • git diff --check

The Docker-based formatting helper was unavailable locally; formatting remains covered by CI.

Prepared with Codex assistance under contributor direction; the bug was reproduced and the complete test suite above was run before opening this draft.

@lemire

lemire commented Aug 8, 2026

Copy link
Copy Markdown
Member

Seems like an obvious defect to me. Merging.

@lemire
lemire marked this pull request as ready for review August 8, 2026 01:05
@lemire
lemire merged commit 335b188 into RoaringBitmap:master Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-atomic refcount decrement never reports zero

2 participants