Background
PR #3128 (merged as 4fd27f68) added reference counting to content-addressed blobs so that deleting one owner's blob (or a shared chunk) no longer destroys content another owner still references.
The refcount update is currently a non-atomic read-modify-write: BlobManager::{release_ref, persist_ref} in crates/store/blobs/src/lib.rs get the metadata row, adjust refs, and put/delete it back as separate store operations. This is documented as an explicit INVARIANT on both functions: callers must serialize add/delete for the same blob id, which the node does today (blob lifecycle is driven serially per id).
Multiple review passes on #3128 flagged the non-atomicity. It was intentionally deferred rather than pulled into that PR.
Problem
If two operations on the same blob id ever interleave between the read and the write, the invariant breaks:
- Two concurrent adds both read
refs == N, both write N + 1 → one increment lost (blob can be freed while an owner still references it → data loss).
- Two concurrent deletes both read
refs == 1, both decrement to 0, both delete the row + file → double-free.
No enforcement mechanism exists today (no per-id lock, no transaction) — only the documented convention. A future caller that introduces concurrent add/delete for the same id would silently reintroduce the loss.
Proposed fix
Fold the refcount read-modify-write onto the store's atomic transaction path, calimero_store::Store::apply (a RocksDB WriteBatch), so the read-decrement/increment-write is atomic with respect to the store. Alternatively (or additionally) introduce a per-blob-id lock inside BlobManager to serialize add/delete for the same id.
Care needed: persist_ref runs mid-stream inside put_sized (per chunk + root), so threading transaction semantics through the streaming path needs a small design pass.
Acceptance
- Concurrent add/delete of the same blob id cannot lose an increment or double-free.
- The
INVARIANT notes in crates/store/blobs/src/lib.rs are updated/removed to reflect the enforced guarantee.
- Regression test exercising concurrent add + delete on the same content id.
References
Background
PR #3128 (merged as
4fd27f68) added reference counting to content-addressed blobs so that deleting one owner's blob (or a shared chunk) no longer destroys content another owner still references.The refcount update is currently a non-atomic read-modify-write:
BlobManager::{release_ref, persist_ref}incrates/store/blobs/src/lib.rsgetthe metadata row, adjustrefs, andput/deleteit back as separate store operations. This is documented as an explicitINVARIANTon both functions: callers must serialize add/delete for the same blob id, which the node does today (blob lifecycle is driven serially per id).Multiple review passes on #3128 flagged the non-atomicity. It was intentionally deferred rather than pulled into that PR.
Problem
If two operations on the same blob id ever interleave between the read and the write, the invariant breaks:
refs == N, both writeN + 1→ one increment lost (blob can be freed while an owner still references it → data loss).refs == 1, both decrement to 0, both delete the row + file → double-free.No enforcement mechanism exists today (no per-id lock, no transaction) — only the documented convention. A future caller that introduces concurrent add/delete for the same id would silently reintroduce the loss.
Proposed fix
Fold the refcount read-modify-write onto the store's atomic transaction path,
calimero_store::Store::apply(a RocksDBWriteBatch), so the read-decrement/increment-write is atomic with respect to the store. Alternatively (or additionally) introduce a per-blob-id lock insideBlobManagerto serialize add/delete for the same id.Care needed:
persist_refruns mid-stream insideput_sized(per chunk + root), so threading transaction semantics through the streaming path needs a small design pass.Acceptance
INVARIANTnotes incrates/store/blobs/src/lib.rsare updated/removed to reflect the enforced guarantee.References
crates/store/blobs/src/lib.rs(delete,release_ref,persist_ref)