db: the catalog snapshot is read under each collection's lock
`write_catalog` walks every collection's slab extents, byte counters and index metadata while holding only the *shared catalog* lock -- which is the same lock a writer holds, taking the collection's lock exclusively. So the snapshot read structures their owners were free to mutate underneath it. `slab_extents` makes it more than a torn read: it is an ArrayList that `slab_reserve` appends to, and an append that reallocates leaves the serializer walking freed memory. What it writes from that walk is the catalog the next open trusts to find every extent the collection owns. Now under each collection's lock, shared, taken inside the catalog lock -- the same order `compact` uses, so no new ordering to reason about. Not the commit that found the concurrency bugs above; those needed a checkpoint racing writers, which this lock is orthogonal to. It is the one that makes the snapshot legal rather than merely lucky. Verified: `zig build test` in ReleaseFast and ReleaseSafe.
This commit is contained in:
@@ -1670,6 +1670,14 @@ pub const Engine = struct {
|
||||
var coll_it = colls.iterator();
|
||||
while (coll_it.next()) |ce| {
|
||||
const coll = ce.value_ptr.*;
|
||||
// Everything below this line is written by a collection's own
|
||||
// writer under its own lock, and `slab_extents` is an ArrayList
|
||||
// that `slab_reserve` appends to -- so reading it under only the
|
||||
// shared catalog lock could walk a slice a concurrent append had
|
||||
// already reallocated. Lock order is catalog then collection,
|
||||
// the same order `compact` uses.
|
||||
try coll.lock.lockShared(self.io);
|
||||
defer coll.lock.unlockShared(self.io);
|
||||
try put_bytes(gpa, out, ce.key_ptr.*);
|
||||
try put_u64(gpa, out, coll.slab_tail);
|
||||
try put_u64(gpa, out, coll.slab_end);
|
||||
|
||||
Reference in New Issue
Block a user