From d315a1082ef27e3ff8aa75df229945943dfe2974 Mon Sep 17 00:00:00 2001 From: Vitor Rodrigues Date: Fri, 1 Dec 2023 13:05:50 +0100 Subject: [PATCH] Fixed potential NuCache file lock causing unusable website (#14940) * Fixed potential NuCache file lock causing unusable website * Remove unnecessary volatile read --- .../PublishedSnapshotService.cs | 59 ++++++++----------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs index 68b50cfd91..38165ee12a 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs @@ -58,11 +58,13 @@ internal class PublishedSnapshotService : IPublishedSnapshotService private long _domainGen; private SnapDictionary _domainStore = null!; private IAppCache? _elementsCache; - private bool _isReadSet; + private bool _isReadSet; private bool _isReady; private object? _isReadyLock; + private bool _mainDomRegistered; + private BPlusTree? _localContentDb; private bool _localContentDbExists; private BPlusTree? _localMediaDb; @@ -490,9 +492,20 @@ internal class PublishedSnapshotService : IPublishedSnapshotService // it will not be able to close the stores until we are done populating (if the store is empty) lock (_storesLock) { + SyncBootState bootState = _syncBootStateAccessor.GetSyncBootState(); + if (!_options.IgnoreLocalDb) { - _mainDom.Register(MainDomRegister, MainDomRelease); + if (!_mainDomRegistered) + { + _mainDom.Register(MainDomRegister, MainDomRelease); + } + else + { + // MainDom is already registered, so we must be retrying to load cache data + // We can't trust the localdb state, so always perform a cold boot + bootState = SyncBootState.ColdBoot; + } // stores are created with a db so they can write to it, but they do not read from it, // stores need to be populated, happens in OnResolutionFrozen which uses _localDbExists to @@ -541,8 +554,6 @@ internal class PublishedSnapshotService : IPublishedSnapshotService var okContent = false; var okMedia = false; - SyncBootState bootState = _syncBootStateAccessor.GetSyncBootState(); - try { if (bootState != SyncBootState.ColdBoot && _localContentDbExists) @@ -614,6 +625,8 @@ internal class PublishedSnapshotService : IPublishedSnapshotService _localContentDb = BTree.GetTree(localContentDbPath, _localContentDbExists, _config, _contentDataSerializer); _localMediaDb = BTree.GetTree(localMediaDbPath, _localMediaDbExists, _config, _contentDataSerializer); + _mainDomRegistered = true; + _logger.LogInformation( "Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", _localContentDbExists, @@ -699,21 +712,10 @@ internal class PublishedSnapshotService : IPublishedSnapshotService _localContentDb?.Clear(); // IMPORTANT GetAllContentSources sorts kits by level + parentId + sortOrder - try - { - IEnumerable kits = _publishedContentService.GetAllContentSources(); - return onStartup - ? _contentStore.SetAllFastSortedLocked(kits, _config.KitBatchSize, true) - : _contentStore.SetAllLocked(kits, _config.KitBatchSize, true); - } - catch (ThreadAbortException tae) - { - // Caught a ThreadAbortException, most likely from a database timeout. - // If we don't catch it here, the whole local cache can remain locked causing widespread panic (see above comment). - _logger.LogWarning(tae, tae.Message); - } - - return false; + IEnumerable kits = _publishedContentService.GetAllContentSources(); + return onStartup + ? _contentStore.SetAllFastSortedLocked(kits, _config.KitBatchSize, true) + : _contentStore.SetAllLocked(kits, _config.KitBatchSize, true); } } @@ -762,21 +764,10 @@ internal class PublishedSnapshotService : IPublishedSnapshotService } // IMPORTANT GetAllMediaSources sorts kits by level + parentId + sortOrder - try - { - IEnumerable kits = _publishedContentService.GetAllMediaSources(); - return onStartup - ? _mediaStore.SetAllFastSortedLocked(kits, _config.KitBatchSize, true) - : _mediaStore.SetAllLocked(kits, _config.KitBatchSize, true); - } - catch (ThreadAbortException tae) - { - // Caught a ThreadAbortException, most likely from a database timeout. - // If we don't catch it here, the whole local cache can remain locked causing widespread panic (see above comment). - _logger.LogWarning(tae, tae.Message); - } - - return false; + IEnumerable kits = _publishedContentService.GetAllMediaSources(); + return onStartup + ? _mediaStore.SetAllFastSortedLocked(kits, _config.KitBatchSize, true) + : _mediaStore.SetAllLocked(kits, _config.KitBatchSize, true); } }