From 713a074049416371b0c75aaf985dc23bccdeb125 Mon Sep 17 00:00:00 2001 From: Luciano Pucciarelli Date: Fri, 26 Sep 2025 11:38:30 -0600 Subject: [PATCH] fix(engine/memorycache): harden MemoryCache with on-demand cleanup - Add non-reentrant Cleanup() (SemaphoreSlim.Wait(0)) to prevent overlap - Introduce TryCleanupOnDemand(): cleans on demand (no background thread) - Triggered from DiscardPage (force when free > cap) - Triggered after Extend() requeues pages (signal + on next tick) - Lightweight tick every OPS_CLEANUP_STEP ops (default 256) - Eviction policy: idle + batch (DEFAULT_IDLE=60s, DEFAULT_BATCH=128) - Stamp freed timestamp (UTC ticks) when moving readable -> free - DiscardPage/Extend/Clear now set page.Timestamp - PageBuffer: atomic eviction guard (TryBeginEvict/MarkReusable) to avoid races - Clear(): requeue readable pages, return count, wake cleanup if over cap - Dispose(): block new triggers, wait any running cleanup, dispose lock - Conservative defaults: MaxFreePages=200, Idle=60s, Batch=128, Interval=30s - No public API changes Refs: #1756, #2619, RFC #2647 --- LiteDB.Tests/Internals/Cache_Tests.cs | 68 ++++++++ LiteDB/Engine/Disk/DiskService.cs | 3 + LiteDB/Engine/Disk/MemoryCache.cs | 206 ++++++++++++++++++++++++- LiteDB/Engine/Structures/PageBuffer.cs | 14 +- 4 files changed, 283 insertions(+), 8 deletions(-) diff --git a/LiteDB.Tests/Internals/Cache_Tests.cs b/LiteDB.Tests/Internals/Cache_Tests.cs index b5dd737e8..49626e917 100644 --- a/LiteDB.Tests/Internals/Cache_Tests.cs +++ b/LiteDB.Tests/Internals/Cache_Tests.cs @@ -173,5 +173,73 @@ private void ConsumeNewPages(int[] segmentSizes) p.ShareCounter = 0; } } + + [Fact] + public void Cache_ProfileConfiguration() + { + var m = new MemoryCache(new int[] { 10 }); + + // Test Mobile profile + m.ApplyProfile(MemoryCache.CacheProfile.Mobile); + // Mobile profile should be more aggressive (can't directly test private fields, but behavior should reflect it) + + // Test Desktop profile + m.ApplyProfile(MemoryCache.CacheProfile.Desktop); + + // Test Server profile + m.ApplyProfile(MemoryCache.CacheProfile.Server); + + // Profiles should not crash and cache should remain functional + var page = m.NewPage(); + page.ShareCounter.Should().Be(-1); + m.DiscardPage(page); + } + + [Fact] + public void Cache_CleanupDoesNotInterfereWithNormalOperations() + { + var m = new MemoryCache(new int[] { 5 }); + var pages = new List(); + + // Create many pages to potentially trigger cleanup + for (int i = 0; i < 100; i++) + { + var page = m.NewPage(); + page.Origin = FileOrigin.Log; + page.Position = i; + m.TryMoveToReadable(page); + pages.Add(page); + } + + // Read pages multiple times to trigger cleanup in GetReadablePage + for (int i = 0; i < 50; i++) + { + var page = m.GetReadablePage(i, FileOrigin.Log, (p, s) => { }); + page.Should().NotBeNull(); + page.Release(); + } + + // Cache should still be functional + m.ExtendSegments.Should().BeGreaterThan(0); + m.FreePages.Should().BeGreaterOrEqualTo(0); + } + + [Fact] + public void Cache_EvictionFlags() + { + var page = new PageBuffer(new byte[8192], 0, 1); + + // Initially should be able to begin eviction + page.TryBeginEvict().Should().BeTrue(); + + // Second attempt should fail (already evicting) + page.TryBeginEvict().Should().BeFalse(); + + // Mark as reusable + page.MarkReusable(); + + // Should be able to begin eviction again + page.TryBeginEvict().Should().BeTrue(); + } } } \ No newline at end of file diff --git a/LiteDB/Engine/Disk/DiskService.cs b/LiteDB/Engine/Disk/DiskService.cs index 73e7910b5..e91bcaa3c 100644 --- a/LiteDB/Engine/Disk/DiskService.cs +++ b/LiteDB/Engine/Disk/DiskService.cs @@ -34,6 +34,9 @@ public DiskService( int[] memorySegmentSizes) { _cache = new MemoryCache(memorySegmentSizes); + // TODO: Add engine parameter to configure cache profile selection + _cache.ApplyProfile(MemoryCache.CacheProfile.Desktop); + _state = state; diff --git a/LiteDB/Engine/Disk/MemoryCache.cs b/LiteDB/Engine/Disk/MemoryCache.cs index f82947dba..97695783f 100644 --- a/LiteDB/Engine/Disk/MemoryCache.cs +++ b/LiteDB/Engine/Disk/MemoryCache.cs @@ -40,6 +40,27 @@ internal class MemoryCache : IDisposable /// Get memory segment sizes /// private readonly int[] _segmentSizes; + + // On-demand cleanup system to prevent memory leaks + private readonly SemaphoreSlim _cleanLock = new SemaphoreSlim(1, 1); // Non-reentrant cleanup lock + private DateTime _lastCleanupRunUtc = DateTime.MinValue; // Last cleanup execution time + private volatile bool _cleanupSignal; // Signal to trigger cleanup + + // Conservative library defaults + private const int DEFAULT_MAX_FREE_PAGES = 200; + private static readonly TimeSpan DEFAULT_IDLE = TimeSpan.FromSeconds(60); + private static readonly TimeSpan DEFAULT_INT = TimeSpan.FromSeconds(30); + private const int DEFAULT_BATCH = 128; + private const int OPS_CLEANUP_STEP = 256; // Trigger cleanup every N operations + + // Instance-level overrides for tuning + private volatile int _maxFreePages = DEFAULT_MAX_FREE_PAGES; + private TimeSpan _idleBeforeEvict = DEFAULT_IDLE; + private TimeSpan _minCleanupInterval = DEFAULT_INT; + private volatile int _cleanupBatchSize = DEFAULT_BATCH; + private int _opsSinceLastCleanup; // Operations counter for periodic cleanup + + public enum CacheProfile { Mobile, Desktop, Server } public MemoryCache(int[] memorySegmentSizes) { @@ -79,6 +100,12 @@ public PageBuffer GetReadablePage(long position, FileOrigin origin, Action public PageBuffer NewPage() { - return this.NewPage(long.MaxValue, FileOrigin.None); + var page = this.NewPage(long.MaxValue, FileOrigin.None); + + return page; } /// @@ -256,7 +285,10 @@ public void DiscardPage(PageBuffer page) // or will be overwritten by ReadPage // added into free list + page.Timestamp = DateTime.UtcNow.Ticks; // PR-1 mark freed time _free.Enqueue(page); + if (_free.Count > _maxFreePages) _cleanupSignal = true; + TryCleanupOnDemand(force: _free.Count > _maxFreePages); } #endregion @@ -339,7 +371,9 @@ private void Extend() page.Position = long.MaxValue; page.Origin = FileOrigin.None; + page.Timestamp = DateTime.UtcNow.Ticks; _free.Enqueue(page); + if (_free.Count > _maxFreePages) _cleanupSignal = true; } } @@ -401,28 +435,186 @@ private void Extend() public int Clear() { var counter = 0; + var now = DateTime.UtcNow; + var overCap = false; ENSURE(this.PagesInUse == 0, "must have no pages in use when call Clear() cache"); - foreach (var page in _readable.Values) + // Thread-safe enumeration of ConcurrentDictionary + foreach (var kv in _readable) { - page.Position = long.MaxValue; - page.Origin = FileOrigin.None; + if (_readable.TryRemove(kv.Key, out var page)) + { + // Reset page controls + page.Position = long.MaxValue; + page.Origin = FileOrigin.None; + + // Mark as "freed at" for idle policy + page.Timestamp = now.Ticks; - _free.Enqueue(page); + _free.Enqueue(page); + counter++; - counter++; + // Check if we exceed free pages limit + if (_free.Count > _maxFreePages) + overCap = true; + } } - _readable.Clear(); + // Trigger cleanup if needed (outside any locks) + if (overCap) + { + _cleanupSignal = true; + TryCleanupOnDemand(force: true); + } return counter; } #endregion + #region Memory Cleanup & Tuning + + /// + /// Apply predefined cache tuning profiles for different environments + /// + public void ApplyProfile(CacheProfile profile) + { + switch (profile) + { + case CacheProfile.Mobile: + // Low-RAM / mobile devices - aggressive cleanup + _maxFreePages = 100; + _idleBeforeEvict = TimeSpan.FromSeconds(25); + _minCleanupInterval = TimeSpan.FromSeconds(2); + _cleanupBatchSize = 96; + break; + + case CacheProfile.Desktop: + // Balanced defaults + _maxFreePages = DEFAULT_MAX_FREE_PAGES; + _idleBeforeEvict = DEFAULT_IDLE; + _minCleanupInterval = DEFAULT_INT; + _cleanupBatchSize = DEFAULT_BATCH; + break; + + case CacheProfile.Server: + // High throughput - less frequent but larger cleanups + _maxFreePages = 360; + _idleBeforeEvict = TimeSpan.FromSeconds(90); + _minCleanupInterval = TimeSpan.FromSeconds(45); + _cleanupBatchSize = 192; + break; + } + } + + /// + /// Triggers cleanup if conditions are met: forced, cleanup signal set, + /// free pages exceed limit, or operation threshold reached + /// + private void TryCleanupOnDemand(bool force = false) + { + if (_disposed) + return; + + var now = DateTime.UtcNow; + var since = now - _lastCleanupRunUtc; + + if (!force && + since < _minCleanupInterval && + _free.Count <= _maxFreePages && + !_cleanupSignal && + _opsSinceLastCleanup < OPS_CLEANUP_STEP) + { + return; + } + + _cleanupSignal = false; + Interlocked.Exchange(ref _opsSinceLastCleanup, 0); + + Cleanup(); + } + + /// + /// Performs bounded, non-reentrant cleanup over the free-list. + /// Processes up to _cleanupBatchSize pages, evicting those idle longer than _idleBeforeEvict. + /// Uses atomic eviction flags to prevent race conditions during cleanup. + /// + private void Cleanup() + { + if (!_cleanLock.Wait(0)) return; + try + { + if (_free.IsEmpty) return; + + var now = DateTime.UtcNow; + _lastCleanupRunUtc = now; + + var keep = new List(_cleanupBatchSize); + int processed = 0; + + while (processed < _cleanupBatchSize && _free.TryDequeue(out var page)) + { + if (!page.TryBeginEvict()) + { + keep.Add(page); + processed++; + continue; + } + + var freedAtTicks = page.Timestamp > 0 ? page.Timestamp : now.Ticks; + var idle = now - new DateTime(freedAtTicks, DateTimeKind.Utc); + + if (idle < _idleBeforeEvict) + { + page.MarkReusable(); + keep.Add(page); + } + else + { + try + { + // Try to clear the page buffer to free memory + // If Clear() fails (e.g., corrupted buffer, access violation), + // we gracefully handle it by keeping the page for reuse instead of crashing + page.Clear(); + } + catch + { + page.MarkReusable(); + keep.Add(page); + } + } + + processed++; + } + + foreach (var p in keep) + _free.Enqueue(p); + } + finally + { + _cleanLock.Release(); + } + } + + #endregion + + private volatile bool _disposed; + public void Dispose() { + _disposed = true; + _cleanupSignal = false; + + try + { + _cleanLock.Wait(); + } + finally + { + _cleanLock.Dispose(); + } } } } \ No newline at end of file diff --git a/LiteDB/Engine/Structures/PageBuffer.cs b/LiteDB/Engine/Structures/PageBuffer.cs index d4b95c18f..0d3406e8e 100644 --- a/LiteDB/Engine/Structures/PageBuffer.cs +++ b/LiteDB/Engine/Structures/PageBuffer.cs @@ -17,6 +17,18 @@ namespace LiteDB.Engine /// internal class PageBuffer : BufferSlice { + private int _evicting; // atomic eviction flag (0/1) + + public bool TryBeginEvict() + { +#if DEBUG + ENSURE(this.ShareCounter == 0, "evicting a non-free page"); +#endif + return Interlocked.CompareExchange(ref _evicting, 1, 0) == 0; + } + + public void MarkReusable() => Interlocked.Exchange(ref _evicting, 0); + /// /// Get, on initialize, a unique ID in all database instance for this PageBufer. Is a simple global incremented counter /// @@ -38,7 +50,7 @@ internal class PageBuffer : BufferSlice public int ShareCounter; /// - /// Get/Set timestamp from last request + /// Last request timestamp; when moved to free-list, stores the "freed at" time (UTC ticks). /// public long Timestamp;