From e8e1d25af08ebe84f76900781591ced1f272616c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Sat, 17 Oct 2026 16:10:35 +0200 Subject: [PATCH] fix(storage): make the collection slot map safe for lock-free readers (#312) In collection-per-file mode every read, write and free of a collection page resolves the page's slot to the collection name (GetPageFile -> GetCollectionNameBySlot) without _collectionSlotLock. The slot -> name map was a plain Dictionary, written under the lock when a collection got its slot. A lookup that ran while the dictionary resized (at the 4th, 8th, 18th and 38th collection) could miss, and GetPageFile then sent the page to the main file: "Page 3254779953 is outside the page file (file holds 65 pages): the file is corrupt". The map is now a ConcurrentDictionary, and a new slot is published slot -> name before name -> slot, so a thread that finds the slot by name (GetCollectionPageIds) also finds the file by the slot. The new test races slot lookups and page reads against all 63 slot assignments of an engine, 100 times; it fails in 10 of 10 runs on the old code (also with DOTNET_PROCESSOR_COUNT=2), within about a second. Co-Authored-By: Claude Opus 5.5 --- .../Storage/StorageEngine.Memory.cs | 6 +- src/BLite.Core/Storage/StorageEngine.cs | 8 ++- tests/BLite.Tests/MultiFileStorageTests.cs | 68 +++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/BLite.Core/Storage/StorageEngine.Memory.cs b/src/BLite.Core/Storage/StorageEngine.Memory.cs index a70bee1f..758411e5 100644 --- a/src/BLite.Core/Storage/StorageEngine.Memory.cs +++ b/src/BLite.Core/Storage/StorageEngine.Memory.cs @@ -675,9 +675,11 @@ private int GetOrAssignCollectionSlot(string collectionName) $"Maximum number of per-collection files ({MaxCollections}) reached. " + $"Use single-file mode (no CollectionDataDirectory) for more than {MaxCollections} collections."); + // Slot → name first: a thread that finds the slot through _collectionNameToSlot may + // route a page of the collection right away, and must find its file by the slot. var newSlot = _nextSlotIndex++; - _collectionNameToSlot[collectionName] = newSlot; _collectionSlotToName![newSlot] = collectionName; + _collectionNameToSlot[collectionName] = newSlot; SaveCollectionSlots(); return newSlot; } @@ -699,8 +701,8 @@ private void LoadCollectionSlots() #endif var name = line.Substring(sep + 1); if (string.IsNullOrEmpty(name)) continue; - _collectionNameToSlot![name] = slot; _collectionSlotToName![slot] = name; + _collectionNameToSlot![name] = slot; if (slot >= _nextSlotIndex) _nextSlotIndex = slot + 1; } diff --git a/src/BLite.Core/Storage/StorageEngine.cs b/src/BLite.Core/Storage/StorageEngine.cs index 4c5d256b..68c1b8dc 100644 --- a/src/BLite.Core/Storage/StorageEngine.cs +++ b/src/BLite.Core/Storage/StorageEngine.cs @@ -74,9 +74,11 @@ public sealed partial class StorageEngine : IDisposable private readonly ConcurrentDictionary>? _collectionFiles; // Collection slot registry — only populated in multi-file mode - // Maps collection name → slot index (0-63) and slot → name + // Maps collection name → slot index (0-63) and slot → name. Both are written under + // _collectionSlotLock but read without it: every collection page I/O resolves its slot + // (GetPageFile), so both must be safe for readers that run during a write (#312). private readonly ConcurrentDictionary? _collectionNameToSlot; - private readonly Dictionary? _collectionSlotToName; + private readonly ConcurrentDictionary? _collectionSlotToName; private int _nextSlotIndex; private readonly string? _slotsFilePath; private readonly object _collectionSlotLock = new(); @@ -309,7 +311,7 @@ public StorageEngine(string databasePath, PageFileConfig config) Directory.CreateDirectory(config.CollectionDataDirectory); _collectionFiles = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); _collectionNameToSlot = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - _collectionSlotToName = new Dictionary(); + _collectionSlotToName = new ConcurrentDictionary(); _slotsFilePath = Path.Combine(config.CollectionDataDirectory, ".slots"); LoadCollectionSlots(); } diff --git a/tests/BLite.Tests/MultiFileStorageTests.cs b/tests/BLite.Tests/MultiFileStorageTests.cs index 0da5a7a0..e8fa4b75 100644 --- a/tests/BLite.Tests/MultiFileStorageTests.cs +++ b/tests/BLite.Tests/MultiFileStorageTests.cs @@ -792,4 +792,72 @@ public async Task ConcurrentAllocateAndRead_NoCrash() await Task.WhenAll(tasks); Assert.Empty(errors); } + + /// + /// Every read or write of a collection page looks its slot up to find the collection file, + /// without the slot lock. Creating collections at the same time must not make that lookup + /// miss: a miss sent the page to the main file ("Page ... is outside the page file", #312). + /// Readers race the lookup against every resize of the slot map, up to + /// collections; one of them also reads the page + /// through the engine, so the end-to-end symptom is covered as well. The slots are assigned + /// without creating the collection files, which keeps a round cheap enough to run many. + /// + [Fact] + public void CollectionPageRouting_WhileCollectionsAreCreated_FindsTheCollectionFile() + { + const int rounds = 100; + var readers = Math.Max(2, Environment.ProcessorCount - 1); + const System.Reflection.BindingFlags Private = System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic; + var lookup = typeof(StorageEngine).GetMethod("GetCollectionNameBySlot", Private)!; + var assign = typeof(StorageEngine).GetMethod("GetOrAssignCollectionSlot", Private)!; + var errors = new System.Collections.Concurrent.ConcurrentQueue(); + + for (int round = 0; round < rounds && errors.IsEmpty; round++) + { + var dbPath = Path.Combine(_tempDir, $"slots_{round}.db"); + var config = PageFileConfig.Small with { CollectionDataDirectory = Path.Combine(_tempDir, $"slots_{round}") }; + using var engine = new StorageEngine(dbPath, config); + var nameBySlot = (Func)lookup.CreateDelegate(typeof(Func), engine); + var assignSlot = (Func)assign.CreateDelegate(typeof(Func), engine); + + var pageId = engine.AllocateCollectionPage("c0"); + var data = new byte[engine.PageSize]; + data[0] = 0x5A; + engine.WritePageImmediate(pageId, data); + + var creating = true; + var threads = Enumerable.Range(0, readers).Select(r => new Thread(() => + { + var buf = new byte[engine.PageSize]; + while (Volatile.Read(ref creating) && errors.IsEmpty) + { + try + { + if (r == 0) + { + engine.ReadPage(pageId, null, buf); + if (buf[0] != 0x5A) errors.Enqueue($"round {round}: page {pageId:X8} read back {buf[0]:X2}"); + } + else if (nameBySlot(0) != "c0") + { + errors.Enqueue($"round {round}: slot 0 not found while collections were created"); + } + } + catch (Exception ex) + { + errors.Enqueue($"round {round}: {ex.GetType().Name}: {ex.Message}"); + } + } + }) { IsBackground = true }).ToList(); + threads.ForEach(t => t.Start()); + + for (int c = 1; c < StorageEngine.MaxCollections; c++) + assignSlot($"c{c}"); + + Volatile.Write(ref creating, false); + threads.ForEach(t => t.Join()); + } + + Assert.True(errors.IsEmpty, string.Join("\n", errors.Take(5))); + } }