From aff4814e0bcdd4c24d38ccd4716578f6d115cd03 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2019 16:06:43 +1100 Subject: [PATCH] Ensures that PublishedSnapshotService cannot close the local cache files after they are created but before they are populated --- .../NuCache/PublishedSnapshotService.cs | 188 ++++++++++-------- 1 file changed, 101 insertions(+), 87 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index d7480baf67..276498ec93 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -113,65 +113,37 @@ namespace Umbraco.Web.PublishedCache.NuCache if (runtime.Level != RuntimeLevel.Run) return; - if (options.IgnoreLocalDb == false) + // lock this entire call, we only want a single thread to be accessing the stores at once and within + // the call below to mainDom.Register, a callback may occur on a threadpool thread to MainDomRelease + // at the same time as we are trying to write to the stores. MainDomRelease also locks on _storesLock so + // it will not be able to close the stores until we are done populating (if the store is empty) + lock (_storesLock) { - var registered = mainDom.Register( - () => - { - //"install" phase of MainDom - //this is inside of a lock in MainDom so this is guaranteed to run if MainDom was acquired and guaranteed - //to not run if MainDom wasn't acquired. - //If MainDom was not acquired, then _localContentDb and _localMediaDb will remain null which means this appdomain - //will load in published content via the DB and in that case this appdomain will probably not exist long enough to - //serve more than a page of content. + if (options.IgnoreLocalDb == false) + { + var registered = mainDom.Register(MainDomRegister, MainDomRelease); - var path = GetLocalFilesPath(); - var localContentDbPath = Path.Combine(path, "NuCache.Content.db"); - var localMediaDbPath = Path.Combine(path, "NuCache.Media.db"); - var localContentDbExists = File.Exists(localContentDbPath); - var localMediaDbExists = File.Exists(localMediaDbPath); - _localDbExists = localContentDbExists && localMediaDbExists; - // if both local databases exist then GetTree will open them, else new databases will be created - _localContentDb = BTree.GetTree(localContentDbPath, _localDbExists); - _localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists); + // 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 + // figure out whether it can read the databases or it should populate them from sql - _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", localContentDbExists, localMediaDbExists); - }, - () => - { - //"release" phase of MainDom + _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDb != null); + _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); + _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDb != null); + _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); + } + else + { + _logger.Info("Creating the content store (local db ignored)"); + _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); + _logger.Info("Creating the media store (local db ignored)"); + _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); + } - lock (_storesLock) - { - _contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned - _localContentDb = null; - _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned - _localMediaDb = null; + _domainStore = new SnapDictionary(); - _logger.Info("Released from MainDom"); - } - }); - - // 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 - // figure out whether it can read the databases or it should populate them from sql - - _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDb != null); - _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); - _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDb != null); - _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); + LoadCachesOnStartup(); } - else - { - _logger.Info("Creating the content store (local db ignored)"); - _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); - _logger.Info("Creating the media store (local db ignored)"); - _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); - } - - _domainStore = new SnapDictionary(); - - LoadCachesOnStartup(); Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? default; int GetId(ContentStore store, Guid uid) => store.LiveSnapshot.Get(uid)?.Id ?? default; @@ -183,47 +155,89 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - private void LoadCachesOnStartup() + /// + /// Install phase of + /// + /// + /// This is inside of a lock in MainDom so this is guaranteed to run if MainDom was acquired and guaranteed + /// to not run if MainDom wasn't acquired. + /// If MainDom was not acquired, then _localContentDb and _localMediaDb will remain null which means this appdomain + /// will load in published content via the DB and in that case this appdomain will probably not exist long enough to + /// serve more than a page of content. + /// + private void MainDomRegister() + { + var path = GetLocalFilesPath(); + var localContentDbPath = Path.Combine(path, "NuCache.Content.db"); + var localMediaDbPath = Path.Combine(path, "NuCache.Media.db"); + var localContentDbExists = File.Exists(localContentDbPath); + var localMediaDbExists = File.Exists(localMediaDbPath); + _localDbExists = localContentDbExists && localMediaDbExists; + // if both local databases exist then GetTree will open them, else new databases will be created + _localContentDb = BTree.GetTree(localContentDbPath, _localDbExists); + _localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists); + + _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", localContentDbExists, localMediaDbExists); + } + + /// + /// Release phase of MainDom + /// + /// + /// This will execute on a threadpool thread + /// + private void MainDomRelease() { lock (_storesLock) { - // populate the stores + _contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned + _localContentDb = null; + _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned + _localMediaDb = null; - - var okContent = false; - var okMedia = false; - - try - { - if (_localDbExists) - { - okContent = LockAndLoadContent(scope => LoadContentFromLocalDbLocked(true)); - if (!okContent) - _logger.Warn("Loading content from local db raised warnings, will reload from database."); - okMedia = LockAndLoadMedia(scope => LoadMediaFromLocalDbLocked(true)); - if (!okMedia) - _logger.Warn("Loading media from local db raised warnings, will reload from database."); - } - - if (!okContent) - LockAndLoadContent(scope => LoadContentFromDatabaseLocked(scope, true)); - - if (!okMedia) - LockAndLoadMedia(scope => LoadMediaFromDatabaseLocked(scope, true)); - - LockAndLoadDomains(); - } - catch (Exception ex) - { - _logger.Fatal(ex, "Panic, exception while loading cache data."); - throw; - } - - // finally, cache is ready! - _isReady = true; + _logger.Info("Released from MainDom"); } } + /// + /// Populates the stores + /// + /// This is called inside of a lock for _storesLock + private void LoadCachesOnStartup() + { + var okContent = false; + var okMedia = false; + + try + { + if (_localDbExists) + { + okContent = LockAndLoadContent(scope => LoadContentFromLocalDbLocked(true)); + if (!okContent) + _logger.Warn("Loading content from local db raised warnings, will reload from database."); + okMedia = LockAndLoadMedia(scope => LoadMediaFromLocalDbLocked(true)); + if (!okMedia) + _logger.Warn("Loading media from local db raised warnings, will reload from database."); + } + + if (!okContent) + LockAndLoadContent(scope => LoadContentFromDatabaseLocked(scope, true)); + + if (!okMedia) + LockAndLoadMedia(scope => LoadMediaFromDatabaseLocked(scope, true)); + + LockAndLoadDomains(); + } + catch (Exception ex) + { + _logger.Fatal(ex, "Panic, exception while loading cache data."); + throw; + } + + // finally, cache is ready! + _isReady = true; + } + private void InitializeRepositoryEvents() { // TODO: The reason these events are in the repository is for legacy, the events should exist at the service