Serialise data-page writes across collections sharing one file

[mrdevrobot]

Fixes #135.

Problem

DocumentCollection serialises its own writes with _collectionLock
(DocumentCollection.cs:71), but page placement depends on state shared by every collection in
the file:

• the FreeSpaceIndex — FreeSpaceIndexProvider hands out a single instance whenever
  UsesSeparateCollectionFiles is false (FreeSpaceIndexProvider.cs:18-29)
• the page allocator in StorageEngine

FindPageWithSpace and InsertIntoPage are two separate calls with no lock spanning both, so it
is a check-then-act race between collections:

collection A: FindPageWithSpace(535)  -> enters FSI gate, reads, exits, returns pageId 2739
                                      <-- collection B fills 2739 here -->
collection A: InsertIntoPage(2739, …) -> re-reads the page: not enough space

The thrown InvalidOperationException is the lucky outcome. When the losing writer still fits,
the write goes through and lands over slots another collection's primary index still points at —
and since InsertIntoPage reads, modifies and writes back the page as a whole buffer, the other
collection's documents are dropped with nothing reported at write time. The damage surfaces much
later, on an unrelated read, as a BSON parse failure on a document that was never written that
way. Nothing is persisted corrupt, which is what makes it look like an encoding bug.

UpdateAsync reaches the same path through UpdateDataCore when a document outgrows its slot and
is relocated, so updates are affected too.

Fix

StorageEngine now owns the write lock and hands it to collections:

• single-file mode — every collection of the engine gets the same SemaphoreSlim, so the
  whole find-page → write-page sequence is serialised file-wide.
• collection-per-file mode (CollectionDataDirectory set) — nothing is shared, so each
  collection gets its own lock and write concurrency between collections is preserved.

DocumentCollection._collectionLock changes from a field initialiser to
_storage.CreateCollectionWriteLock(). Every existing acquisition site is unchanged, so the lock
scope (which already covers InsertCore/UpdateCore/DeleteCore and the auto-commit) is the
same — only its granularity changes. The collection never disposes it, since the instance may be
shared.

Two files, ~20 lines. No public API change.

Cost

Writes to different collections of a single-file database no longer overlap. That is the price of
correctness here: the state they contend on is file-wide, so a per-collection lock cannot make
those writes safe. Databases that want write concurrency across collections can use the
collection-per-file layout, which this PR leaves untouched.

Alternatives considered and rejected:

• Reserving space inside the FSI gate — closes the check-then-act window on the free-space
  index, but not the page read-modify-write: two collections holding disjoint reservations on the
  same page would still clobber each other on write-back.
• A FreeSpaceIndex per collection in single-file mode — removes the sharing but costs page
  reuse across collections and leaves the shared page allocator unprotected.

Test

tests/BLite.Tests/CrossCollectionWriteRaceTests.cs — three writers on three collections of one
TestDbContext, plain InsertAsync, single process, 5 seconds. Asserts no writer exception, then
that every inserted id is enumerable and every document intact.

Bisected on this machine (macOS arm64, .NET 10.0.6):

	result
with the fix	3/3 pass
fix reverted, test kept	3/3 fail

Sample failure without the fix:

2 writer failure(s); first: System.InvalidOperationException:
Not enough space: need 272, have 179 | PageId=2601 | SlotCount=21 | Start=192 | End=371 | FSI=281

Note FSI=281 against the page header's real AvailableFreeSpace of 179: the shared free-space
index is describing a page another collection has already consumed.

Full suite

dotnet test tests/BLite.Tests → 2323 passed, 3 failed, 4 skipped.

The 3 failures are MultiProcessWalSharedMemoryTests
(WriterLock_IsMutualExclusion_AcrossInstances,
BeginTransaction_Phase7_ReplaysCrossProcessCommits,
Phase7_ReplayDoesNotDuplicate_LocalCommits). Verified pre-existing: the same three fail on the
same machine with this branch's src/ changes stashed. They are the macOS flock/FileStream
issue tracked in #134 and do not show up on the ubuntu-latest release pipeline.

One thing I could not explain

While iterating on the test I saw a single run where one StringEntity out of ~34,500 was
inserted but not returned by FindAllAsync. It did not reproduce in the following 9 runs (6 of
them with per-id tracking rather than counting), and a single-writer single-collection probe of
35,000 inserts lost nothing across 3 runs. I could not pin it down and it is a different failure
mode from the one this PR fixes — flagging it rather than leaving it unsaid.

Performance

Measured on this branch vs the same tree with the two src/ files reverted to da9795a~1.
Release build, macOS arm64, .NET 10.0.6, single-file database, default PageFileConfig.
First rep of each scenario discarded as warm-up; run-to-run spread inside each set is roughly
±10%, so only the concurrent-insert delta is above noise.

scenario	baseline	with fix	delta
A — 1 collection, 1 writer, 20k inserts	~14.5k ops/s	~13.7k ops/s	within noise
B — 3 collections, 3 writers, 30k inserts	~19.5k ops/s, 4–9 errors/run	~16.1k ops/s, 0 errors	≈ −17%
C — FindAllAsync over 100k docs	~317k docs/s	~330k docs/s	within noise

A is unchanged by construction: the lock scope and acquisition sites are identical, only the
instance differs.

C is unchanged by construction too: reads never take the write lock.

B is the real cost, and it is the point of the change — writes to different collections of one
file no longer overlap. Note the baseline column is flattered by its own bug: every run lost 4–9
inserts to Not enough space, and a failed insert aborts at InsertIntoPage without doing the
rest of the work, so the baseline is doing measurably less work per "operation".

Databases that need write concurrency across collections can use the collection-per-file layout
(CollectionDataDirectory), where each collection keeps its own lock — that path is unchanged.

[Copilot]

Pull request overview

Adds storage-engine-managed write serialization for collections sharing a file, plus a concurrent write regression test.

Changes:

• Adds shared/per-file write-lock creation in StorageEngine.
• Uses the engine-provided lock in DocumentCollection.
• Adds cross-collection concurrent-write coverage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Review findings
tests/BLite.Tests/CrossCollectionWriteRaceTests.cs	Nit (3 votes): Validate exact IDs and payloads for all three writers, not only leading characters for two collections.
src/BLite.Core/Storage/StorageEngine.cs	Critical (1 vote): DynamicCollection still bypasses the shared lock. Critical (3 votes): Per-file locks must coordinate multiple wrappers for the same physical collection file.
src/BLite.Core/Collections/DocumentCollection.cs	Critical (1 vote): ForcePruneAsync can write and free pages without acquiring the file-wide lock.

Suppressed comments (1)

src/BLite.Core/Storage/StorageEngine.cs:220

• The engine now allocates this semaphore, but StorageEngine.Dispose does not dispose _dataWriteLock; the collection explicitly leaves it alive because it may be shared. Dispose this engine-owned semaphore after writers have stopped, alongside the other engine-owned synchronization objects, so repeated engine open/close cycles do not leave an undisposed wait handle.

            _dataWriteLock = _collectionFiles == null ? new SemaphoreSlim(1, 1) : null;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.