diff --git a/src/Umbraco.Core/MainDom.cs b/src/Umbraco.Core/MainDom.cs index 5da1062275..a21afd6ffb 100644 --- a/src/Umbraco.Core/MainDom.cs +++ b/src/Umbraco.Core/MainDom.cs @@ -15,7 +15,7 @@ namespace Umbraco.Core /// When an AppDomain starts, it tries to acquire the main domain status. /// When an AppDomain stops (eg the application is restarting) it should release the main domain status. /// - internal class MainDom : IMainDom, IRegisteredObject + internal class MainDom : IMainDom, IRegisteredObject, IDisposable { #region Vars @@ -25,8 +25,8 @@ namespace Umbraco.Core private readonly object _locko = new object(); // async lock representing the main domain lock - private readonly AsyncLock _asyncLock; - private IDisposable _asyncLocker; + private readonly SystemLock _systemLock; + private IDisposable _systemLocker; // event wait handle used to notify current main domain that it should // release the lock because a new domain wants to be the main domain @@ -48,13 +48,13 @@ namespace Umbraco.Core // initializes a new instance of MainDom public MainDom(ILogger logger) { + HostingEnvironment.RegisterObject(this); + _logger = logger; - var appId = string.Empty; // HostingEnvironment.ApplicationID is null in unit tests, making ReplaceNonAlphanumericChars fail - if (HostingEnvironment.ApplicationID != null) - appId = HostingEnvironment.ApplicationID.ReplaceNonAlphanumericChars(string.Empty); - + var appId = HostingEnvironment.ApplicationID?.ReplaceNonAlphanumericChars(string.Empty) ?? string.Empty; + // combining with the physical path because if running on eg IIS Express, // two sites could have the same appId even though they are different. // @@ -64,11 +64,11 @@ namespace Umbraco.Core // we *cannot* use the process ID here because when an AppPool restarts it is // a new process for the same application path - var appPath = HostingEnvironment.ApplicationPhysicalPath; + var appPath = HostingEnvironment.ApplicationPhysicalPath?.ToLowerInvariant() ?? string.Empty; var hash = (appId + ":::" + appPath).GenerateHash(); var lockName = "UMBRACO-" + hash + "-MAINDOM-LCK"; - _asyncLock = new AsyncLock(lockName); + _systemLock = new SystemLock(lockName); var eventName = "UMBRACO-" + hash + "-MAINDOM-EVT"; _signal = new EventWaitHandle(false, EventResetMode.AutoReset, eventName); @@ -99,6 +99,12 @@ namespace Umbraco.Core lock (_locko) { if (_signaled) return false; + if (_isMainDom == false) + { + _logger.Warn("Register called when MainDom has not been acquired"); + return false; + } + install?.Invoke(); if (release != null) _callbacks.Add(new KeyValuePair(weight, release)); @@ -118,32 +124,32 @@ namespace Umbraco.Core if (_signaled) return; if (_isMainDom == false) return; // probably not needed _signaled = true; - } - try - { - _logger.Info("Stopping ({SignalSource})", source); - foreach (var callback in _callbacks.OrderBy(x => x.Key).Select(x => x.Value)) + try { - try + _logger.Info("Stopping ({SignalSource})", source); + foreach (var callback in _callbacks.OrderBy(x => x.Key).Select(x => x.Value)) { - callback(); // no timeout on callbacks + try + { + callback(); // no timeout on callbacks + } + catch (Exception e) + { + _logger.Error(e, "Error while running callback"); + continue; + } } - catch (Exception e) - { - _logger.Error(e, "Error while running callback, remaining callbacks will not run."); - throw; - } - + _logger.Debug("Stopped ({SignalSource})", source); } - _logger.Debug("Stopped ({SignalSource})", source); - } - finally - { - // in any case... - _isMainDom = false; - _asyncLocker.Dispose(); - _logger.Info("Released ({SignalSource})", source); + finally + { + // in any case... + _isMainDom = false; + _systemLocker?.Dispose(); + _logger.Info("Released ({SignalSource})", source); + } + } } @@ -172,25 +178,29 @@ namespace Umbraco.Core // if more than 1 instance reach that point, one will get the lock // and the other one will timeout, which is accepted - //TODO: This can throw a TimeoutException - in which case should this be in a try/finally to ensure the signal is always reset? - _asyncLocker = _asyncLock.Lock(LockTimeoutMilliseconds); + //This can throw a TimeoutException - in which case should this be in a try/finally to ensure the signal is always reset. + try + { + _systemLocker = _systemLock.Lock(LockTimeoutMilliseconds); + } + finally + { + // we need to reset the event, because otherwise we would end up + // signaling ourselves and committing suicide immediately. + // only 1 instance can reach that point, but other instances may + // have started and be trying to get the lock - they will timeout, + // which is accepted + + _signal.Reset(); + } _isMainDom = true; - - // we need to reset the event, because otherwise we would end up - // signaling ourselves and committing suicide immediately. - // only 1 instance can reach that point, but other instances may - // have started and be trying to get the lock - they will timeout, - // which is accepted - - _signal.Reset(); + //WaitOneAsync (ext method) will wait for a signal without blocking the main thread, the waiting is done on a background thread _signal.WaitOneAsync() .ContinueWith(_ => OnSignal("signal")); - HostingEnvironment.RegisterObject(this); - _logger.Info("Acquired."); return true; } @@ -204,14 +214,39 @@ namespace Umbraco.Core // IRegisteredObject void IRegisteredObject.Stop(bool immediate) { - try + OnSignal("environment"); // will run once + + // The web app is stopping, need to wind down + Dispose(true); + + HostingEnvironment.UnregisterObject(this); + } + + #region IDisposable Support + + // This code added to correctly implement the disposable pattern. + + private bool disposedValue = false; // To detect redundant calls + + protected virtual void Dispose(bool disposing) + { + if (!disposedValue) { - OnSignal("environment"); // will run once - } - finally - { - HostingEnvironment.UnregisterObject(this); + if (disposing) + { + _signal?.Close(); + _signal?.Dispose(); + } + + disposedValue = true; } } + + public void Dispose() + { + Dispose(true); + } + + #endregion } } diff --git a/src/Umbraco.Core/Runtime/CoreRuntime.cs b/src/Umbraco.Core/Runtime/CoreRuntime.cs index 5b069641c4..5839ba6cfd 100644 --- a/src/Umbraco.Core/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Core/Runtime/CoreRuntime.cs @@ -157,6 +157,8 @@ namespace Umbraco.Core.Runtime // create & initialize the components _components = _factory.GetInstance(); _components.Initialize(); + + } catch (Exception e) { diff --git a/src/Umbraco.Core/AsyncLock.cs b/src/Umbraco.Core/SystemLock.cs similarity index 77% rename from src/Umbraco.Core/AsyncLock.cs rename to src/Umbraco.Core/SystemLock.cs index 6dd866705e..4eaae7082b 100644 --- a/src/Umbraco.Core/AsyncLock.cs +++ b/src/Umbraco.Core/SystemLock.cs @@ -21,18 +21,18 @@ namespace Umbraco.Core // been closed, the Semaphore system object is destroyed - so in any case // an iisreset should clean up everything // - internal class AsyncLock + internal class SystemLock { private readonly SemaphoreSlim _semaphore; private readonly Semaphore _semaphore2; private readonly IDisposable _releaser; private readonly Task _releaserTask; - public AsyncLock() - : this (null) + public SystemLock() + : this(null) { } - public AsyncLock(string name) + public SystemLock(string name) { // WaitOne() waits until count > 0 then decrements count // Release() increments count @@ -67,35 +67,6 @@ namespace Umbraco.Core : new NamedSemaphoreReleaser(_semaphore2); } - //NOTE: We don't use the "Async" part of this lock at all - //TODO: Remove this and rename this class something like SystemWideLock, then we can re-instate this logic if we ever need an Async lock again - - //public Task LockAsync() - //{ - // var wait = _semaphore != null - // ? _semaphore.WaitAsync() - // : _semaphore2.WaitOneAsync(); - - // return wait.IsCompleted - // ? _releaserTask ?? Task.FromResult(CreateReleaser()) // anonymous vs named - // : wait.ContinueWith((_, state) => (((AsyncLock) state).CreateReleaser()), - // this, CancellationToken.None, - // TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); - //} - - //public Task LockAsync(int millisecondsTimeout) - //{ - // var wait = _semaphore != null - // ? _semaphore.WaitAsync(millisecondsTimeout) - // : _semaphore2.WaitOneAsync(millisecondsTimeout); - - // return wait.IsCompleted - // ? _releaserTask ?? Task.FromResult(CreateReleaser()) // anonymous vs named - // : wait.ContinueWith((_, state) => (((AsyncLock)state).CreateReleaser()), - // this, CancellationToken.None, - // TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); - //} - public IDisposable Lock() { if (_semaphore != null) @@ -121,14 +92,18 @@ namespace Umbraco.Core private class NamedSemaphoreReleaser : CriticalFinalizerObject, IDisposable { private readonly Semaphore _semaphore; - private GCHandle _handle; internal NamedSemaphoreReleaser(Semaphore semaphore) { _semaphore = semaphore; - _handle = GCHandle.Alloc(_semaphore); } + #region IDisposable Support + + // This code added to correctly implement the disposable pattern. + + private bool disposedValue = false; // To detect redundant calls + public void Dispose() { Dispose(true); @@ -137,10 +112,22 @@ namespace Umbraco.Core private void Dispose(bool disposing) { - // critical - _handle.Free(); - _semaphore.Release(); - _semaphore.Dispose(); + if (!disposedValue) + { + try + { + _semaphore.Release(); + } + finally + { + try + { + _semaphore.Dispose(); + } + catch { } + } + disposedValue = true; + } } // we WANT to release the semaphore because it's a system object, ie a critical @@ -171,6 +158,9 @@ namespace Umbraco.Core // we do NOT want the finalizer to throw - never ever } } + + #endregion + } private class SemaphoreSlimReleaser : IDisposable diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 50754fef0d..6f9ff1e783 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -128,7 +128,7 @@ --> - + diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/SafeXmlReaderWriter.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/SafeXmlReaderWriter.cs index c0b9383b57..aa88f28dc0 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/SafeXmlReaderWriter.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/SafeXmlReaderWriter.cs @@ -39,7 +39,7 @@ namespace Umbraco.Tests.LegacyXmlPublishedCache return scopeProvider?.Context?.GetEnlisted(EnlistKey); } - public static SafeXmlReaderWriter Get(IScopeProvider scopeProvider, AsyncLock xmlLock, XmlDocument xml, Action refresh, Action apply, bool writer) + public static SafeXmlReaderWriter Get(IScopeProvider scopeProvider, SystemLock xmlLock, XmlDocument xml, Action refresh, Action apply, bool writer) { var scopeContext = scopeProvider.Context; diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs index 447104b7cd..c452c4792a 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs @@ -305,7 +305,7 @@ namespace Umbraco.Tests.LegacyXmlPublishedCache private XmlDocument _xmlDocument; // supplied xml document (for tests) private volatile XmlDocument _xml; // master xml document - private readonly AsyncLock _xmlLock = new AsyncLock(); // protects _xml + private readonly SystemLock _xmlLock = new SystemLock(); // protects _xml // to be used by PublishedContentCache only // for non-preview content only diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs index 145a19872a..56c09b18ac 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs @@ -24,7 +24,7 @@ namespace Umbraco.Tests.LegacyXmlPublishedCache private bool _released; private Timer _timer; private DateTime _initialTouch; - private readonly AsyncLock _runLock = new AsyncLock(); // ensure we run once at a time + private readonly SystemLock _runLock = new SystemLock(); // ensure we run once at a time // note: // as long as the runner controls the runs, we know that we run once at a time, but diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 3664717af7..ea42cd2ea9 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Logging; +using Umbraco.Tests.TestHelpers; using Umbraco.Web.Scheduling; namespace Umbraco.Tests.Scheduling @@ -21,7 +22,7 @@ namespace Umbraco.Tests.Scheduling [OneTimeSetUp] public void InitializeFixture() { - _logger = new DebugDiagnosticsLogger(); + _logger = new ConsoleLogger(); } [Test] @@ -102,12 +103,12 @@ namespace Umbraco.Tests.Scheduling { using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) { - MyTask t; + MyTask t1, t2, t3; Assert.IsFalse(runner.IsRunning); // because AutoStart is false - runner.Add(new MyTask(5000)); - runner.Add(new MyTask()); - runner.Add(t = new MyTask()); + runner.Add(t1 = new MyTask(5000)); + runner.Add(t2 = new MyTask()); + runner.Add(t3 = new MyTask()); Assert.IsTrue(runner.IsRunning); // is running tasks // shutdown -force => run all queued tasks @@ -115,7 +116,7 @@ namespace Umbraco.Tests.Scheduling Assert.IsTrue(runner.IsRunning); // is running tasks await runner.StoppedAwaitable; // runner stops, within test's timeout - Assert.AreNotEqual(DateTime.MinValue, t.Ended); // t has run + Assert.AreNotEqual(DateTime.MinValue, t3.Ended); // t3 has run } } @@ -124,20 +125,25 @@ namespace Umbraco.Tests.Scheduling { using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) { - MyTask t; + MyTask t1, t2, t3; Assert.IsFalse(runner.IsRunning); // because AutoStart is false - runner.Add(new MyTask(5000)); - runner.Add(new MyTask()); - runner.Add(t = new MyTask()); + runner.Add(t1 = new MyTask(5000)); + runner.Add(t2 = new MyTask()); + runner.Add(t3 = new MyTask()); Assert.IsTrue(runner.IsRunning); // is running tasks + Thread.Sleep(1000); // since we are forcing shutdown, we need to give it a chance to start, else it will be canceled before the queue is started + // shutdown +force => tries to cancel the current task, ignores queued tasks runner.Shutdown(true, false); // +force -wait Assert.IsTrue(runner.IsRunning); // is running that long task it cannot cancel - await runner.StoppedAwaitable; // runner stops, within test's timeout - Assert.AreEqual(DateTime.MinValue, t.Ended); // t has *not* run + await runner.StoppedAwaitable; // runner stops, within test's timeout (no cancelation token used, no need to catch OperationCanceledException) + + Assert.AreNotEqual(DateTime.MinValue, t1.Ended); // t1 *has* run + Assert.AreEqual(DateTime.MinValue, t2.Ended); // t2 has *not* run + Assert.AreEqual(DateTime.MinValue, t3.Ended); // t3 has *not* run } } @@ -163,7 +169,15 @@ namespace Umbraco.Tests.Scheduling // shutdown +force => tries to cancel the current task, ignores queued tasks runner.Shutdown(true, false); // +force -wait - await runner.StoppedAwaitable; // runner stops, within test's timeout + try + { + await runner.StoppedAwaitable; // runner stops, within test's timeout ... maybe + } + catch (OperationCanceledException) + { + // catch exception, this can occur because we are +force shutting down which will + // cancel a pending task if the queue hasn't completed in time + } } } @@ -183,25 +197,20 @@ namespace Umbraco.Tests.Scheduling runner.Terminated += (sender, args) => { terminated = true; }; Assert.IsFalse(runner.IsRunning); // because AutoStart is false - runner.Add(new MyTask(5000)); - runner.Add(new MyTask()); - runner.Add(t = new MyTask()); + runner.Add(new MyTask()); // sleeps 500 ms + runner.Add(new MyTask()); // sleeps 500 ms + runner.Add(t = new MyTask()); // sleeps 500 ms ... total = 1500 ms until it's done Assert.IsTrue(runner.IsRunning); // is running the task - runner.Stop(false); // -immediate = -force, -wait + runner.Stop(false); // -immediate = -force, -wait (max 2000 ms delay before +immediate) + await runner.TerminatedAwaitable; + + Assert.IsTrue(stopped); // raised that one Assert.IsTrue(terminating); // has raised that event - Assert.IsFalse(terminated); // but not terminated yet + Assert.IsTrue(terminated); // and that event - // all this before we await because -wait Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner - Assert.IsTrue(runner.IsRunning); // still running the task - - await runner.StoppedAwaitable; // runner stops, within test's timeout - Assert.IsFalse(runner.IsRunning); - Assert.IsTrue(stopped); - - await runner.TerminatedAwaitable; // runner terminates, within test's timeout - Assert.IsTrue(terminated); // has raised that event + Assert.IsFalse(runner.IsRunning); // done running Assert.AreNotEqual(DateTime.MinValue, t.Ended); // t has run } @@ -222,23 +231,21 @@ namespace Umbraco.Tests.Scheduling runner.Terminated += (sender, args) => { terminated = true; }; Assert.IsFalse(runner.IsRunning); // because AutoStart is false - runner.Add(new MyTask(5000)); - runner.Add(new MyTask()); - runner.Add(t = new MyTask()); + runner.Add(new MyTask()); // sleeps 500 ms + runner.Add(new MyTask()); // sleeps 500 ms + runner.Add(t = new MyTask()); // sleeps 500 ms ... total = 1500 ms until it's done Assert.IsTrue(runner.IsRunning); // is running the task - runner.Stop(true); // +immediate = +force, +wait + runner.Stop(true); // +immediate = +force, +wait (no delay) + await runner.TerminatedAwaitable; + + Assert.IsTrue(stopped); // raised that one Assert.IsTrue(terminating); // has raised that event Assert.IsTrue(terminated); // and that event - Assert.IsTrue(stopped); // and that one - - // and all this before we await because +wait + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner Assert.IsFalse(runner.IsRunning); // done running - await runner.StoppedAwaitable; // runner stops, within test's timeout - await runner.TerminatedAwaitable; // runner terminates, within test's timeout - Assert.AreEqual(DateTime.MinValue, t.Ended); // t has *not* run } } @@ -264,8 +271,7 @@ namespace Umbraco.Tests.Scheduling }, _logger)) { Assert.IsTrue(runner.IsRunning); // because AutoStart is true - runner.Stop(false); // keepalive = must be stopped - await runner.StoppedAwaitable; // runner stops, within test's timeout + await runner.StopInternal(false); // keepalive = must be stopped } } @@ -291,13 +297,9 @@ namespace Umbraco.Tests.Scheduling // dispose will stop it } - await runner.StoppedAwaitable; // runner stops, within test's timeout - //await runner.TerminatedAwaitable; // NO! see note below + await runner.StoppedAwaitable; Assert.Throws(() => runner.Add(new MyTask())); - // but do NOT await on TerminatedAwaitable - disposing just shuts the runner down - // so that we don't have a runaway task in tests, etc - but it does NOT terminate - // the runner - it really is NOT a nice way to end a runner - it's there for tests } [Test] @@ -564,7 +566,7 @@ namespace Umbraco.Tests.Scheduling Thread.Sleep(1000); Assert.IsTrue(runner.IsRunning); // still waiting for the task to release Assert.IsFalse(task.HasRun); - task.Release(); + task.Release(); // unlatch var runnerTask = runner.CurrentThreadingTask; // may be null if things go fast enough if (runnerTask != null) await runnerTask; // wait for current task to complete @@ -574,7 +576,7 @@ namespace Umbraco.Tests.Scheduling } [Test] - public async Task LatchedTaskStops() + public async Task LatchedTaskStops_Runs_On_Shutdown() { using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) { @@ -584,7 +586,7 @@ namespace Umbraco.Tests.Scheduling Thread.Sleep(5000); Assert.IsTrue(runner.IsRunning); // still waiting for the task to release Assert.IsFalse(task.HasRun); - runner.Shutdown(false, false); + runner.Shutdown(false, false); // -force, -wait await runner.StoppedAwaitable; // wait for the entire runner operation to complete Assert.IsTrue(task.HasRun); } @@ -880,7 +882,9 @@ namespace Umbraco.Tests.Scheduling public override void PerformRun() { + Console.WriteLine($"Sleeping {_milliseconds}..."); Thread.Sleep(_milliseconds); + Console.WriteLine("Wake up!"); } } @@ -997,7 +1001,9 @@ namespace Umbraco.Tests.Scheduling public DateTime Ended { get; set; } public virtual void Dispose() - { } + { + + } } } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 64bffc7a49..550fd507d5 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -235,11 +235,24 @@ namespace Umbraco.Web.PublishedCache.NuCache var lockInfo = new WriteLockInfo(); try { - Lock(lockInfo); + try + { + // Trying to lock could throw exceptions so always make sure to clean up. + Lock(lockInfo); + } + finally + { + try + { + _localDb?.Dispose(); + } + catch { /* TBD: May already be throwing so don't throw again */} + finally + { + _localDb = null; + } + } - if (_localDb == null) return; - _localDb.Dispose(); - _localDb = null; } finally { diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 80662f5db0..a866297d72 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -57,7 +57,8 @@ namespace Umbraco.Web.PublishedCache.NuCache private BPlusTree _localContentDb; private BPlusTree _localMediaDb; - private bool _localDbExists; + private bool _localContentDbExists; + private bool _localMediaDbExists; // define constant - determines whether to use cache when previewing // to store eg routes, property converted values, anything - caching @@ -127,9 +128,9 @@ namespace Umbraco.Web.PublishedCache.NuCache // 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}", _localDbExists); + _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDbExists); _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); - _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localDbExists); + _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDbExists); _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); } else @@ -170,14 +171,15 @@ namespace Umbraco.Web.PublishedCache.NuCache 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); + + _localContentDbExists = File.Exists(localContentDbPath); + _localMediaDbExists = File.Exists(localMediaDbPath); - _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", localContentDbExists, localMediaDbExists); + // if both local databases exist then GetTree will open them, else new databases will be created + _localContentDb = BTree.GetTree(localContentDbPath, _localContentDbExists); + _localMediaDb = BTree.GetTree(localMediaDbPath, _localMediaDbExists); + + _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", _localContentDbExists, _localMediaDbExists); } /// @@ -210,11 +212,15 @@ namespace Umbraco.Web.PublishedCache.NuCache try { - if (_localDbExists) + if (_localContentDbExists) { okContent = LockAndLoadContent(scope => LoadContentFromLocalDbLocked(true)); if (!okContent) - _logger.Warn("Loading content from local db raised warnings, will reload from database."); + _logger.Warn("Loading content from local db raised warnings, will reload from database."); + } + + if (_localMediaDbExists) + { okMedia = LockAndLoadMedia(scope => LoadMediaFromLocalDbLocked(true)); if (!okMedia) _logger.Warn("Loading media from local db raised warnings, will reload from database."); diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index 47e97593f0..c0475b1f79 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -199,7 +199,7 @@ namespace Umbraco.Web.Scheduling { lock (_locker) { - var task = _runningTask ?? Task.FromResult(0); + var task = _runningTask ?? Task.CompletedTask; return new ThreadingTaskImmutable(task); } } @@ -211,8 +211,9 @@ namespace Umbraco.Web.Scheduling /// An awaitable object. /// /// Used to wait until the runner has terminated. - /// This is for unit tests and should not be used otherwise. In most cases when the runner - /// has terminated, the application domain is going down and it is not the right time to do things. + /// + /// The only time the runner will be terminated is by the Hosting Environment when the application is being shutdown. + /// /// internal ThreadingTaskImmutable TerminatedAwaitable { @@ -338,29 +339,37 @@ namespace Umbraco.Web.Scheduling if (_isRunning == false) return; // done already } + var hasTasks = TaskCount > 0; + + if (!force && hasTasks) + _logger.Info("{LogPrefix} Waiting for tasks to complete", _logPrefix); + // complete the queue // will stop waiting on the queue or on a latch _tasks.Complete(); if (force) { - // we must bring everything down, now - Thread.Sleep(100); // give time to Complete() + // we must bring everything down, now lock (_locker) { // was Complete() enough? - if (_isRunning == false) return; + // if _tasks.Complete() ended up triggering code to stop the runner and reset + // the _isRunning flag, then there's no need to initiate a cancel on the cancelation token. + if (_isRunning == false) + return; } + // try to cancel running async tasks (cannot do much about sync tasks) // break latched tasks // stop processing the queue - _shutdownTokenSource.Cancel(false); // false is the default - _shutdownTokenSource.Dispose(); + _shutdownTokenSource?.Cancel(false); // false is the default + _shutdownTokenSource?.Dispose(); _shutdownTokenSource = null; } // tasks in the queue will be executed... - if (wait == false) return; + if (!wait) return; _runningTask?.Wait(CancellationToken.None); // wait for whatever is running to end... } @@ -428,7 +437,7 @@ namespace Umbraco.Web.Scheduling lock (_locker) { // deal with race condition - if (_shutdownToken.IsCancellationRequested == false && _tasks.Count > 0) continue; + if (_shutdownToken.IsCancellationRequested == false && TaskCount > 0) continue; // if we really have nothing to do, stop _logger.Debug("{LogPrefix} Stopping", _logPrefix); @@ -453,7 +462,7 @@ namespace Umbraco.Web.Scheduling // if KeepAlive is false then don't block, exit if there is // no task in the buffer - yes, there is a race condition, which // we'll take care of - if (_options.KeepAlive == false && _tasks.Count == 0) + if (_options.KeepAlive == false && TaskCount == 0) return null; try @@ -503,15 +512,19 @@ namespace Umbraco.Web.Scheduling // returns the task that completed // - latched.Latch completes when the latch releases // - _tasks.Completion completes when the runner completes - // - tokenTaskSource.Task completes when this task, or the whole runner, is cancelled + // - tokenTaskSource.Task completes when this task, or the whole runner is cancelled var task = await Task.WhenAny(latched.Latch, _tasks.Completion, tokenTaskSource.Task); // ok to run now if (task == latched.Latch) return bgTask; + // we are shutting down if the _tasks.Complete(); was called or the shutdown token was cancelled + var isShuttingDown = _shutdownToken.IsCancellationRequested || task == _tasks.Completion; + // if shutting down, return the task only if it runs on shutdown - if (_shutdownToken.IsCancellationRequested == false && latched.RunsOnShutdown) return bgTask; + if (isShuttingDown && latched.RunsOnShutdown) + return bgTask; // else, either it does not run on shutdown or it's been cancelled, dispose latched.Dispose(); @@ -578,17 +591,18 @@ namespace Umbraco.Web.Scheduling // triggers when the hosting environment requests that the runner terminates internal event TypedEventHandler, EventArgs> Terminating; - // triggers when the runner has terminated (no task can be added, no task is running) + // triggers when the hosting environment has terminated (no task can be added, no task is running) internal event TypedEventHandler, EventArgs> Terminated; private void OnEvent(TypedEventHandler, EventArgs> handler, string name) { - if (handler == null) return; OnEvent(handler, name, EventArgs.Empty); } private void OnEvent(TypedEventHandler, TArgs> handler, string name, TArgs e) { + _logger.Debug("{LogPrefix} OnEvent {EventName}", _logPrefix, name); + if (handler == null) return; try @@ -664,17 +678,16 @@ namespace Umbraco.Web.Scheduling #endregion + #region IRegisteredObject.Stop + /// - /// Requests a registered object to un-register. + /// Used by IRegisteredObject.Stop and shutdown on threadpool threads to not block shutdown times. /// - /// true to indicate the registered object should un-register from the hosting - /// environment before returning; otherwise, false. - /// - /// "When the application manager needs to stop a registered object, it will call the Stop method." - /// The application manager will call the Stop method to ask a registered object to un-register. During - /// processing of the Stop method, the registered object must call the HostingEnvironment.UnregisterObject method. - /// - public void Stop(bool immediate) + /// + /// + /// An awaitable Task that is used to handle the shutdown. + /// + internal Task StopInternal(bool immediate) { // the first time the hosting environment requests that the runner terminates, // raise the Terminating event - that could be used to prevent any process that @@ -693,33 +706,90 @@ namespace Umbraco.Web.Scheduling if (onTerminating) OnEvent(Terminating, "Terminating"); - if (immediate == false) + // Run the Stop commands on another thread since IRegisteredObject.Stop calls are called sequentially + // with a single aspnet thread during shutdown and we don't want to delay other calls to IRegisteredObject.Stop. + if (!immediate) { - // The Stop method is first called with the immediate parameter set to false. The object can either complete - // processing, call the UnregisterObject method, and then return or it can return immediately and complete - // processing asynchronously before calling the UnregisterObject method. + return Task.Run(StopInitial, CancellationToken.None); + } + else + { + lock (_locker) + { + if (_terminated) return Task.CompletedTask; + return Task.Run(StopImmediate, CancellationToken.None); + } + } + } - _logger.Info("{LogPrefix} Waiting for tasks to complete", _logPrefix); + /// + /// Requests a registered object to un-register. + /// + /// true to indicate the registered object should un-register from the hosting + /// environment before returning; otherwise, false. + /// + /// "When the application manager needs to stop a registered object, it will call the Stop method." + /// The application manager will call the Stop method to ask a registered object to un-register. During + /// processing of the Stop method, the registered object must call the HostingEnvironment.UnregisterObject method. + /// + public void Stop(bool immediate) => StopInternal(immediate); + + /// + /// Called when immediate == false for IRegisteredObject.Stop(bool immediate) + /// + /// + /// Called on a threadpool thread + /// + private void StopInitial() + { + // immediate == false when the app is trying to wind down, immediate == true will be called either: + // after a call with immediate == false or if the app is not trying to wind down and needs to immediately stop. + // So Stop may be called twice or sometimes only once. + + try + { Shutdown(false, false); // do not accept any more tasks, flush the queue, do not wait - + } + finally + { // raise the completed event only after the running threading task has completed lock (_locker) { if (_runningTask != null) - _runningTask.ContinueWith(_ => Terminate(false)); + _runningTask.ContinueWith(_ => StopImmediate()); else - Terminate(false); + StopImmediate(); } } - else - { - // If the registered object does not complete processing before the application manager's time-out - // period expires, the Stop method is called again with the immediate parameter set to true. When the - // immediate parameter is true, the registered object must call the UnregisterObject method before returning; - // otherwise, its registration will be removed by the application manager. - _logger.Info("{LogPrefix} Canceling tasks", _logPrefix); + // If the shutdown token was not canceled in the Shutdown call above, it means there was still tasks + // being processed, in which case we'll give it a couple seconds + if (!_shutdownToken.IsCancellationRequested) + { + // If we are called with immediate == false, wind down above and then shutdown within 2 seconds, + // we want to shut down the app as quick as possible, if we wait until immediate == true, this can + // take a very long time since immediate will only be true when a new request is received on the new + // appdomain (or another iis timeout occurs ... which can take some time). + Thread.Sleep(2000); //we are already on a threadpool thread + StopImmediate(); + } + } + + /// + /// Called when immediate == true for IRegisteredObject.Stop(bool immediate) + /// + /// + /// Called on a threadpool thread + /// + private void StopImmediate() + { + _logger.Info("{LogPrefix} Canceling tasks", _logPrefix); + try + { Shutdown(true, true); // cancel all tasks, wait for the current one to end + } + finally + { Terminate(true); } } @@ -732,7 +802,13 @@ namespace Umbraco.Web.Scheduling // raise the Terminated event // complete the awaitable completion source, if any - HostingEnvironment.UnregisterObject(this); + if (immediate) + { + //only unregister when it's the final call, else we won't be notified of the final call + HostingEnvironment.UnregisterObject(this); + } + + if (_terminated) return; // already taken care of TaskCompletionSource terminatedSource; lock (_locker) @@ -747,7 +823,9 @@ namespace Umbraco.Web.Scheduling OnEvent(Terminated, "Terminated"); - terminatedSource.SetResult(0); + terminatedSource.TrySetResult(0); } + + #endregion } }