Skip to content

Release shared_mutex with unlock_shared() in Chunked3d::chunk_safe (UB)#1084

Open
robert-kofler42 wants to merge 1 commit into
ScrollPrize:mainfrom
robert-kofler42:fix/chunkedtensor-shared-mutex
Open

Release shared_mutex with unlock_shared() in Chunked3d::chunk_safe (UB)#1084
robert-kofler42 wants to merge 1 commit into
ScrollPrize:mainfrom
robert-kofler42:fix/chunkedtensor-shared-mutex

Conversation

@robert-kofler42

Copy link
Copy Markdown

Bug

Chunked3d::chunk_safe() is the fast path that every volume voxel access in the tracer and renderer goes through (Chunked3dAccessor::get_chunk_safechunk_safe). It takes the chunk-cache std::shared_mutex with lock_shared() but releases it with unlock() on both branches:

_mutex.lock_shared();
if (_chunks.count(id)) { chunk = _chunks[id]; _mutex.unlock(); }   // <- should be unlock_shared()
else                   { _mutex.unlock(); chunk = cache_chunk_safe(id); }  // <- should be unlock_shared()

Per [thread.sharedmutex.requirements], a lock acquired with lock_shared() must be released with unlock_shared(). Calling unlock() while holding shared ownership is undefined behavior. libstdc++ tolerates it (shared/exclusive ownership share one pthread_rwlock), which is why it went unnoticed on Linux, but it is broken on libc++ (macOS) and MSVC — and the repo targets macOS + Ubuntu. The other _mutex.lock()/unlock() pairs in the file are correct exclusive locks and are unchanged.

Fix

Release the shared lock with unlock_shared() on both branches (2 lines).

How it was found

Surfaced while running vc_grow_seg_from_seed on an open-data scroll volume (s3://vesuvius-challenge-open-data/, streamed) under ThreadSanitizer; reading the flagged chunk-cache hot path revealed the mismatched unlock. Compile-verified (vc_core + vc_tracer); correctness is by the std::shared_mutex contract — same class as the GrowSurface unlock_shared fix (#1077), in a much hotter path.

🤖 Generated with Claude Code

Chunked3d::chunk_safe() — the fast path that every volume voxel access in the
tracer and renderer goes through (Chunked3dAccessor::get_chunk_safe ->
chunk_safe) — takes the chunk-cache std::shared_mutex with lock_shared() but
releases it with unlock() on both branches (ChunkedTensor.hpp:422, 425).

Per [thread.sharedmutex.requirements], a lock acquired with lock_shared()
must be released with unlock_shared(); calling unlock() while holding shared
ownership is undefined behavior. libstdc++ happens to tolerate it (shared and
exclusive ownership share a pthread_rwlock), which is why it went unnoticed on
Linux, but it is broken on libc++ (macOS) and MSVC, and the repo targets
macOS + Ubuntu. The other _mutex.lock()/unlock() pairs in the file are correct
exclusive locks and are unchanged.

Surfaced while running vc_grow_seg_from_seed on an open-data scroll volume
under ThreadSanitizer; reading the flagged chunk-cache hot path revealed the
mismatched unlock. Compile-verified; correctness is by the std::shared_mutex
contract (same class as the GrowSurface unlock_shared fix).
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the scroll Team on Vercel.

A member of the Team first needs to authorize it.

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.

1 participant