diff --git a/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs b/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs index 0543e58612..ae840d7b0e 100644 --- a/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs +++ b/src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs @@ -61,8 +61,15 @@ namespace Umbraco.Core.Cache if (lazy.Value is ExceptionHolder) return null; // we have a value and execution has not thrown so returning - // here does not throw - return lazy.Value; + // here does not throw - unless we're re-entering, take care of it + try + { + return lazy.Value; + } + catch (InvalidOperationException e) + { + throw new InvalidOperationException("The method that computes a value for the cache has tried to read that value from the cache.", e); + } } internal class ExceptionHolder diff --git a/src/Umbraco.Tests/Cache/CacheProviderTests.cs b/src/Umbraco.Tests/Cache/CacheProviderTests.cs index 8dd76102cf..384fbf2bb8 100644 --- a/src/Umbraco.Tests/Cache/CacheProviderTests.cs +++ b/src/Umbraco.Tests/Cache/CacheProviderTests.cs @@ -24,6 +24,30 @@ namespace Umbraco.Tests.Cache Provider.ClearAllCache(); } + [Test] + public void Throws_On_Reentry() + { + // don't run for StaticCacheProvider - not making sense + if (GetType() == typeof (StaticCacheProviderTests)) + Assert.Ignore("Do not run for StaticCacheProvider."); + + Exception exception = null; + var result = Provider.GetCacheItem("blah", () => + { + try + { + var result2 = Provider.GetCacheItem("blah"); + } + catch (Exception e) + { + exception = e; + } + return "value"; + }); + Assert.IsNotNull(exception); + Assert.IsAssignableFrom(exception); + } + [Test] public void Does_Not_Cache_Exceptions() { diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 072a370216..7051199bde 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -24,61 +24,156 @@ namespace Umbraco.Tests.Scheduling _logger = new DebugDiagnosticsLogger(); } + /* [Test] public async void ShutdownWaitWhenRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true }, _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { - Assert.IsTrue(runner.IsRunning); - Thread.Sleep(800); // for long - Assert.IsTrue(runner.IsRunning); + AutoStart = true, + KeepAlive = true + })) + { + var stopped = false; + runner.Stopped += (sender, args) => { stopped = true; }; + + Assert.IsTrue(runner.IsRunning); // because AutoStart is true + Thread.Sleep(500); // and because KeepAlive is true... + Assert.IsTrue(runner.IsRunning); // ...it keeps running + runner.Shutdown(false, true); // -force +wait - await runner; // wait for the entire runner operation to complete - Assert.IsTrue(runner.IsCompleted); + await runner.StoppedAwaitable; // runner stops, within test's timeout + + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner + Assert.IsFalse(runner.IsRunning); // no more running tasks + Assert.IsTrue(stopped); } } [Test] public async void ShutdownWhenRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { Console.WriteLine("Begin {0}", DateTime.Now); runner.TaskStarting += (sender, args) => Console.WriteLine("starting {0}", DateTime.Now); runner.TaskCompleted += (sender, args) => Console.WriteLine("completed {0}", DateTime.Now); + runner.Stopped += (sender, args) => Console.WriteLine("stopped {0}", DateTime.Now); - Assert.IsFalse(runner.IsRunning); + Assert.IsFalse(runner.IsRunning); // because AutoStart is false Console.WriteLine("Adding task {0}", DateTime.Now); runner.Add(new MyTask(5000)); + Thread.Sleep(500); Assert.IsTrue(runner.IsRunning); // is running the task + Console.WriteLine("Shutting down {0}", DateTime.Now); runner.Shutdown(false, false); // -force -wait - Assert.IsTrue(runner.IsCompleted); + + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner Assert.IsTrue(runner.IsRunning); // still running that task Thread.Sleep(3000); // wait slightly less than the task takes to complete Assert.IsTrue(runner.IsRunning); // still running that task - await runner; // wait for the entire runner operation to complete - + await runner.StoppedAwaitable; // runner stops, within test's timeout + Console.WriteLine("End {0}", DateTime.Now); } } + */ + + [Test] + public async void ShutdownWhenRunningWithWait() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var stopped = false; + runner.Stopped += (sender, args) => { stopped = true; }; + + Assert.IsFalse(runner.IsRunning); // because AutoStart is false + runner.Add(new MyTask(5000)); + Assert.IsTrue(runner.IsRunning); // is running the task + + runner.Shutdown(false, true); // -force +wait + + // all this before we await because +wait + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner + Assert.IsFalse(runner.IsRunning); // no more running tasks + Assert.IsTrue(stopped); + + await runner.StoppedAwaitable; // runner stops, within test's timeout + } + } + + [Test] + public async void ShutdownWhenRunningWithoutWait() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var stopped = false; + runner.Stopped += (sender, args) => { stopped = true; }; + + Assert.IsFalse(runner.IsRunning); // because AutoStart is false + runner.Add(new MyTask(5000)); + Assert.IsTrue(runner.IsRunning); // is running the task + + runner.Shutdown(false, false); // -force +wait + + // all this before we await because -wait + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner + Assert.IsTrue(runner.IsRunning); // still running the task + Assert.IsFalse(stopped); + + await runner.StoppedAwaitable; // runner stops, within test's timeout + + // and then... + Assert.IsFalse(runner.IsRunning); // no more running tasks + Assert.IsTrue(stopped); + } + } + + [Test] + public async void ShutdownCompletesTheRunner() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + Assert.IsFalse(runner.IsRunning); // because AutoStart is false + + // shutdown -force => run all queued tasks + runner.Shutdown(false, false); // -force -wait + await runner.StoppedAwaitable; // runner stops, within test's timeout + + Assert.IsFalse(runner.IsRunning); // still not running anything + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner + + // cannot add tasks to it anymore + Assert.IsFalse(runner.TryAdd(new MyTask())); + Assert.Throws(() => + { + runner.Add(new MyTask()); + }); + } + } [Test] public async void ShutdownFlushesTheQueue() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - Assert.IsFalse(runner.IsRunning); + MyTask t; + + Assert.IsFalse(runner.IsRunning); // because AutoStart is false runner.Add(new MyTask(5000)); runner.Add(new MyTask()); - var t = new MyTask(); - runner.Add(t); - Assert.IsTrue(runner.IsRunning); // is running the first task + runner.Add(t = new MyTask()); + Assert.IsTrue(runner.IsRunning); // is running tasks + + // shutdown -force => run all queued tasks runner.Shutdown(false, false); // -force -wait - await runner; // wait for the entire runner operation to complete + 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 } } @@ -86,44 +181,132 @@ namespace Umbraco.Tests.Scheduling [Test] public async void ShutdownForceTruncatesTheQueue() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - Assert.IsFalse(runner.IsRunning); + MyTask t; + + Assert.IsFalse(runner.IsRunning); // because AutoStart is false runner.Add(new MyTask(5000)); runner.Add(new MyTask()); - var t = new MyTask(); - runner.Add(t); - Assert.IsTrue(runner.IsRunning); // is running the first task + runner.Add(t = new MyTask()); + Assert.IsTrue(runner.IsRunning); // is running tasks + + // shutdown +force => tries to cancel the current task, ignores queued tasks runner.Shutdown(true, false); // +force -wait - await runner; // wait for the entire runner operation to complete - Assert.AreEqual(DateTime.MinValue, t.Ended); // t has not run + 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 } } [Test] public async void ShutdownThenForce() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - Assert.IsFalse(runner.IsRunning); + + Assert.IsFalse(runner.IsRunning); // because AutoStart is false runner.Add(new MyTask(5000)); runner.Add(new MyTask()); runner.Add(new MyTask()); - Assert.IsTrue(runner.IsRunning); // is running the task + Assert.IsTrue(runner.IsRunning); // is running tasks + + // shutdown -force => run all queued tasks runner.Shutdown(false, false); // -force -wait - Assert.IsTrue(runner.IsCompleted); - Assert.IsTrue(runner.IsRunning); // still running that task + + Assert.IsTrue(runner.IsCompleted); // shutdown completes the runner + Assert.IsTrue(runner.IsRunning); // still running a task Thread.Sleep(3000); - Assert.IsTrue(runner.IsRunning); // still running that task + Assert.IsTrue(runner.IsRunning); // still running a task + + // shutdown +force => tries to cancel the current task, ignores queued tasks runner.Shutdown(true, false); // +force -wait - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // runner stops, within test's timeout + } + } + + + [Test] + public async void HostingStopNonImmediate() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + MyTask t; + + var stopped = false; + runner.Stopped += (sender, args) => { stopped = true; }; + var terminating = false; + runner.Terminating += (sender, args) => { terminating = true; }; + var terminated = false; + 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()); + Assert.IsTrue(runner.IsRunning); // is running the task + + runner.Stop(false); // -immediate = -force, -wait + Assert.IsTrue(terminating); // has raised that event + Assert.IsFalse(terminated); // but not terminated yet + + // 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.AreNotEqual(DateTime.MinValue, t.Ended); // t has run } } [Test] - public void Create_IsRunning() + public async void HostingStopImmediate() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + MyTask t; + + var stopped = false; + runner.Stopped += (sender, args) => { stopped = true; }; + var terminating = false; + runner.Terminating += (sender, args) => { terminating = true; }; + var terminated = false; + 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()); + Assert.IsTrue(runner.IsRunning); // is running the task + + runner.Stop(true); // +immediate = +force, +wait + 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 + } + } + + + [Test] + public void Create_IsNotRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { Assert.IsFalse(runner.IsRunning); } @@ -133,19 +316,22 @@ namespace Umbraco.Tests.Scheduling [Test] public async void Create_AutoStart_IsRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true }, _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { - Assert.IsTrue(runner.IsRunning); - await runner; // wait for the entire runner operation to complete + AutoStart = true + })) + { + Assert.IsTrue(runner.IsRunning); // because AutoStart is true + await runner.StoppedAwaitable; // runner stops, within test's timeout } } [Test] public void Create_AutoStartAndKeepAlive_IsRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true }, _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true })) { - Assert.IsTrue(runner.IsRunning); + Assert.IsTrue(runner.IsRunning); // because AutoStart is true Thread.Sleep(800); // for long Assert.IsTrue(runner.IsRunning); // dispose will stop it @@ -156,20 +342,25 @@ namespace Umbraco.Tests.Scheduling public async void Dispose_IsRunning() { BackgroundTaskRunner runner; - using (runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true }, _logger)) + using (runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true })) { Assert.IsTrue(runner.IsRunning); // dispose will stop it } - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // runner stops, within test's timeout + //await runner.TerminatedAwaitable; // NO! see note below 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] public void Startup_KeepAlive_IsRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = true }, _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = true })) { Assert.IsFalse(runner.IsRunning); runner.StartUp(); @@ -181,7 +372,7 @@ namespace Umbraco.Tests.Scheduling [Test] public async void Create_AddTask_IsRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { var waitHandle = new ManualResetEvent(false); runner.TaskCompleted += (sender, args) => @@ -191,7 +382,7 @@ namespace Umbraco.Tests.Scheduling runner.Add(new MyTask()); Assert.IsTrue(runner.IsRunning); waitHandle.WaitOne(); - await runner; //since we are not being kept alive, it will quit + await runner.StoppedAwaitable; //since we are not being kept alive, it will quit Assert.IsFalse(runner.IsRunning); } } @@ -199,7 +390,7 @@ namespace Umbraco.Tests.Scheduling [Test] public void Create_KeepAliveAndAddTask_IsRunning() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = true }, _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = true })) { var waitHandle = new ManualResetEvent(false); runner.TaskCompleted += (sender, args) => @@ -217,14 +408,14 @@ namespace Umbraco.Tests.Scheduling [Test] public async void WaitOnRunner_OneTask() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { var task = new MyTask(); Assert.IsTrue(task.Ended == default(DateTime)); runner.Add(task); await runner.CurrentThreadingTask; // wait for the Task operation to complete Assert.IsTrue(task.Ended != default(DateTime)); // task is done - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete } } @@ -236,11 +427,11 @@ namespace Umbraco.Tests.Scheduling for (var i = 0; i < 10; i++) tasks.Add(new MyTask()); - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = false, LongRunning = true, PreserveRunningTask = true }, _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = false, LongRunning = true, PreserveRunningTask = true })) { tasks.ForEach(runner.Add); - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete // check that tasks are done Assert.IsTrue(tasks.All(x => x.Ended != default(DateTime))); @@ -254,7 +445,7 @@ namespace Umbraco.Tests.Scheduling [Test] public async void WaitOnTask() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { var task = new MyTask(); var waitHandle = new ManualResetEvent(false); @@ -263,7 +454,7 @@ namespace Umbraco.Tests.Scheduling runner.Add(task); waitHandle.WaitOne(); // wait 'til task is done Assert.IsTrue(task.Ended != default(DateTime)); // task is done - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete } } @@ -274,7 +465,7 @@ namespace Umbraco.Tests.Scheduling for (var i = 0; i < 10; i++) tasks.Add(new MyTask(), new ManualResetEvent(false)); - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { runner.TaskCompleted += (sender, task) => tasks[task.Task].Set(); foreach (var t in tasks) runner.Add(t.Key); @@ -283,7 +474,7 @@ namespace Umbraco.Tests.Scheduling WaitHandle.WaitAll(tasks.Values.Select(x => (WaitHandle)x).ToArray()); Assert.IsTrue(tasks.All(x => x.Key.Ended != default(DateTime))); - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete } } @@ -303,7 +494,7 @@ namespace Umbraco.Tests.Scheduling IDictionary tasks = getTasks(); BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true, KeepAlive = true }, _logger)) + using (tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true, KeepAlive = true })) { tManager.TaskCompleted += (sender, task) => tasks[task.Task].Set(); @@ -349,7 +540,7 @@ namespace Umbraco.Tests.Scheduling List tasks = getTasks(); - using (var tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true, PreserveRunningTask = true }, _logger)) + using (var tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true, PreserveRunningTask = true })) { tasks.ForEach(tManager.Add); @@ -391,7 +582,7 @@ namespace Umbraco.Tests.Scheduling { var runCount = 0; var waitHandle = new ManualResetEvent(false); - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { runner.TaskCompleted += (sender, args) => runCount++; runner.TaskStarting += async (sender, args) => @@ -423,9 +614,9 @@ namespace Umbraco.Tests.Scheduling [Test] public async void DelayedTaskRuns() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - var task = new MyDelayedTask(200); + var task = new MyDelayedTask(200, false); runner.Add(task); Assert.IsTrue(runner.IsRunning); Thread.Sleep(5000); @@ -434,23 +625,23 @@ namespace Umbraco.Tests.Scheduling task.Release(); await runner.CurrentThreadingTask; //wait for current task to complete Assert.IsTrue(task.HasRun); - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete } } [Test] public async void DelayedTaskStops() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - var task = new MyDelayedTask(200); + var task = new MyDelayedTask(200, true); runner.Add(task); Assert.IsTrue(runner.IsRunning); Thread.Sleep(5000); Assert.IsTrue(runner.IsRunning); // still waiting for the task to release Assert.IsFalse(task.HasRun); runner.Shutdown(false, false); - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete Assert.IsTrue(task.HasRun); } } @@ -461,7 +652,7 @@ namespace Umbraco.Tests.Scheduling { var runCount = 0; var waitHandle = new ManualResetEvent(false); - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { runner.TaskCompleted += (sender, args) => runCount++; runner.TaskStarting += async (sender, args) => @@ -481,6 +672,7 @@ namespace Umbraco.Tests.Scheduling waitHandle.WaitOne(); Assert.GreaterOrEqual(runCount, 4); + Assert.IsTrue(task.HasRun); // stops recurring runner.Shutdown(false, false); @@ -490,15 +682,32 @@ namespace Umbraco.Tests.Scheduling [Test] public async void FailingTaskSync() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { var exceptions = new ConcurrentQueue(); runner.TaskError += (sender, args) => exceptions.Enqueue(args.Exception); - var task = new MyFailingTask(false); // -async + var task = new MyFailingTask(false, true, false); // -async, +running, -disposing runner.Add(task); Assert.IsTrue(runner.IsRunning); - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete + + Assert.AreEqual(1, exceptions.Count); // traced and reported + } + } + + [Test] + public async void FailingTaskDisposing() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var exceptions = new ConcurrentQueue(); + runner.TaskError += (sender, args) => exceptions.Enqueue(args.Exception); + + var task = new MyFailingTask(false, false, true); // -async, -running, +disposing + runner.Add(task); + Assert.IsTrue(runner.IsRunning); + await runner.StoppedAwaitable; // wait for the entire runner operation to complete Assert.AreEqual(1, exceptions.Count); // traced and reported } @@ -507,15 +716,32 @@ namespace Umbraco.Tests.Scheduling [Test] public async void FailingTaskAsync() { - using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { var exceptions = new ConcurrentQueue(); runner.TaskError += (sender, args) => exceptions.Enqueue(args.Exception); - var task = new MyFailingTask(true); // +async + var task = new MyFailingTask(true, true, false); // +async, +running, -disposing runner.Add(task); Assert.IsTrue(runner.IsRunning); - await runner; // wait for the entire runner operation to complete + await runner.StoppedAwaitable; // wait for the entire runner operation to complete + + Assert.AreEqual(1, exceptions.Count); // traced and reported + } + } + + [Test] + public async void FailingTaskDisposingAsync() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var exceptions = new ConcurrentQueue(); + runner.TaskError += (sender, args) => exceptions.Enqueue(args.Exception); + + var task = new MyFailingTask(false, false, true); // -async, -running, +disposing + runner.Add(task); + Assert.IsTrue(runner.IsRunning); + await runner.StoppedAwaitable; // wait for the entire runner operation to complete Assert.AreEqual(1, exceptions.Count); // traced and reported } @@ -524,22 +750,28 @@ namespace Umbraco.Tests.Scheduling private class MyFailingTask : IBackgroundTask { private readonly bool _isAsync; + private readonly bool _running; + private readonly bool _disposing; - public MyFailingTask(bool isAsync) + public MyFailingTask(bool isAsync, bool running, bool disposing) { _isAsync = isAsync; + _running = running; + _disposing = disposing; } public void Run() { Thread.Sleep(1000); - throw new Exception("Task has thrown."); + if (_running) + throw new Exception("Task has thrown."); } public async Task RunAsync(CancellationToken token) { await Task.Delay(1000); - throw new Exception("Task has thrown."); + if (_running) + throw new Exception("Task has thrown."); } public bool IsAsync @@ -547,13 +779,17 @@ namespace Umbraco.Tests.Scheduling get { return _isAsync; } } - // fixme - must also test what happens if we throw on dispose! public void Dispose() - { } + { + if (_disposing) + throw new Exception("Task has thrown."); + } } private class MyDelayedRecurringTask : DelayedRecurringTaskBase { + public bool HasRun { get; private set; } + public MyDelayedRecurringTask(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) : base(runner, delayMilliseconds, periodMilliseconds) { } @@ -569,7 +805,7 @@ namespace Umbraco.Tests.Scheduling public override void PerformRun() { - // nothing to do at the moment + HasRun = true; } public override Task PerformRunAsync() @@ -586,30 +822,28 @@ namespace Umbraco.Tests.Scheduling private class MyDelayedTask : ILatchedBackgroundTask { private readonly int _runMilliseconds; - private readonly ManualResetEvent _gate; + private readonly ManualResetEventSlim _gate; public bool HasRun { get; private set; } - public MyDelayedTask(int runMilliseconds) + public MyDelayedTask(int runMilliseconds, bool runsOnShutdown) { _runMilliseconds = runMilliseconds; - _gate = new ManualResetEvent(false); + _gate = new ManualResetEventSlim(false); + RunsOnShutdown = runsOnShutdown; } public WaitHandle Latch { - get { return _gate; } + get { return _gate.WaitHandle; } } public bool IsLatched { - get { return true; } + get { return _gate.IsSet == false; } } - public bool RunsOnShutdown - { - get { return true; } - } + public bool RunsOnShutdown { get; private set; } public void Run() { diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index 74b3567a64..38cbd36b25 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -26,15 +26,23 @@ namespace Umbraco.Web.Scheduling private readonly ILogger _logger; private readonly BlockingCollection _tasks = new BlockingCollection(); private readonly object _locker = new object(); - private readonly ManualResetEventSlim _completedEvent = new ManualResetEventSlim(false); - private BackgroundTaskRunnerAwaiter _awaiter; + // that event is used to stop the pump when it is alive and waiting + // on a latched task - so it waits on the latch, the cancellation token, + // and the completed event + private readonly ManualResetEventSlim _completedEvent = new ManualResetEventSlim(false); + + // fixme explain volatile here private volatile bool _isRunning; // is running private volatile bool _isCompleted; // does not accept tasks anymore, may still be running private Task _runningTask; private CancellationTokenSource _tokenSource; + private bool _terminating; // ensures we raise that event only once + private bool _terminated; // remember we've terminated + private TaskCompletionSource _terminatedSource; // awaitable source + internal event TypedEventHandler, TaskEventArgs> TaskError; internal event TypedEventHandler, TaskEventArgs> TaskStarting; internal event TypedEventHandler, TaskEventArgs> TaskCompleted; @@ -43,8 +51,11 @@ namespace Umbraco.Web.Scheduling // triggers when the runner stops (but could start again if a task is added to it) internal event TypedEventHandler, EventArgs> Stopped; - // triggers when the runner completes (no task can be added to it anymore) - internal event TypedEventHandler, EventArgs> Completed; + // triggers when the hosting environment requests that the runner terminates + internal event TypedEventHandler, EventArgs> Terminating; + + // triggers when the runner terminates (no task can be added, no task is running) + internal event TypedEventHandler, EventArgs> Terminated; /// /// Initializes a new instance of the class. @@ -116,7 +127,7 @@ namespace Umbraco.Web.Scheduling } /// - /// Gets an awaiter used to await the running Threading.Task. + /// Gets the running task as an immutable object. /// /// There is no running task. /// @@ -124,32 +135,51 @@ namespace Umbraco.Web.Scheduling /// a background task is added to the queue. Unless the KeepAlive option is true, there /// will be no running task when the queue is empty. /// - public ThreadingTaskAwaiter CurrentThreadingTask + public ThreadingTaskImmutable CurrentThreadingTask { get { if (_runningTask == null) throw new InvalidOperationException("There is no current Threading.Task."); - return new ThreadingTaskAwaiter(_runningTask); + return new ThreadingTaskImmutable(_runningTask); } } /// - /// Gets an awaiter used to await the BackgroundTaskRunner running operation + /// Gets an awaitable used to await the runner running operation. /// - /// An awaiter for the BackgroundTaskRunner running operation - /// - /// This is used to wait until the background task runner is no longer running (IsRunning == false) - /// - /// So long as we have a method called GetAwaiter() that returns an instance of INotifyCompletion - /// we can await anything. In this case we are awaiting with a custom BackgroundTaskRunnerAwaiter - /// which waits for the Completed event to be raised. - /// ref: http://blogs.msdn.com/b/pfxteam/archive/2011/01/13/10115642.aspx - /// - /// - public BackgroundTaskRunnerAwaiter GetAwaiter() + /// An awaitable instance. + /// Used to wait until the runner is no longer running (IsRunning == false), + /// though the runner could be started again afterwards by adding tasks to it. + public ThreadingTaskImmutable StoppedAwaitable { - return _awaiter ?? (_awaiter = new BackgroundTaskRunnerAwaiter(this, _logger)); + get + { + lock (_locker) + { + var task = _runningTask ?? Task.FromResult(0); + return new ThreadingTaskImmutable(task); + } + } + } + + /// + /// Gets an awaitable used to await the runner. + /// + /// An awaitable instance. + /// Used to wait until the runner is terminated. + public ThreadingTaskImmutable TerminatedAwaitable + { + get + { + lock (_locker) + { + if (_terminatedSource == null && _terminated == false) + _terminatedSource = new TaskCompletionSource(); + var task = _terminatedSource == null ? Task.FromResult(0) : _terminatedSource.Task; + return new ThreadingTaskImmutable(task); + } + } } /// @@ -253,7 +283,7 @@ namespace Umbraco.Web.Scheduling if (force) { // we must bring everything down, now - Thread.Sleep(100); // give time to CompleAdding() + Thread.Sleep(100); // give time to CompleteAdding() lock (_locker) { // was CompleteAdding() enough? @@ -287,23 +317,27 @@ namespace Umbraco.Web.Scheduling // because the pump does not lock, there's a race condition, // the pump may stop and then we still have tasks to process, // and then we must restart the pump - lock to avoid race cond + var onStopped = false; lock (_locker) { if (token.IsCancellationRequested || _tasks.Count == 0) { _logger.Debug(_logPrefix + "Stopping"); - _isRunning = false; // done if (_options.PreserveRunningTask == false) _runningTask = null; - OnStopped(); - - return; + // stopped + _isRunning = false; + onStopped = true; } } - + if (onStopped) + { + OnEvent(Stopped, "Stopped"); + return; + } // if _runningTask is taskSource.Task then we must keep continuing it, // not starting a new taskSource, else _runningTask would complete and @@ -430,12 +464,18 @@ namespace Umbraco.Web.Scheduling } catch (Exception ex) { - _logger.Error(_logPrefix + "Task has failed.", ex); + _logger.Error(_logPrefix + "Task has failed", ex); } } #region Events + 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) { if (handler == null) return; @@ -473,16 +513,6 @@ namespace Umbraco.Web.Scheduling e.Task.Dispose(); } - protected virtual void OnStopped() - { - OnEvent(Stopped, "Stopped", EventArgs.Empty); - } - - protected virtual void OnCompleted() - { - OnEvent(Completed, "Completed", EventArgs.Empty); - } - #endregion #region IDisposable @@ -535,13 +565,30 @@ namespace Umbraco.Web.Scheduling /// public void Stop(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 + // would expect the runner to be available from starting. + var onTerminating = false; + lock (_locker) + { + if (_terminating == false) + { + _terminating = true; + _logger.Info(_logPrefix + "Terminating" + (immediate ? " (immediate)" : "")); + onTerminating = true; + } + } + + if (onTerminating) + OnEvent(Terminating, "Terminating"); + if (immediate == false) { // 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. - _logger.Info(_logPrefix + "Shutting down, waiting for tasks to complete."); + _logger.Info(_logPrefix + "Waiting for tasks to complete"); Shutdown(false, false); // do not accept any more tasks, flush the queue, do not wait // raise the completed event only after the running task has completed @@ -550,18 +597,9 @@ namespace Umbraco.Web.Scheduling lock (_locker) { if (_runningTask != null) - _runningTask.ContinueWith(_ => - { - HostingEnvironment.UnregisterObject(this); - _logger.Info(_logPrefix + "Down, tasks completed."); - Completed.RaiseEvent(EventArgs.Empty, this); - }); + _runningTask.ContinueWith(_ => Terminate(false)); else - { - HostingEnvironment.UnregisterObject(this); - _logger.Info(_logPrefix + "Down, tasks completed."); - Completed.RaiseEvent(EventArgs.Empty, this); - } + Terminate(false); } } else @@ -571,13 +609,31 @@ namespace Umbraco.Web.Scheduling // 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 + "Shutting down immediately."); + _logger.Info(_logPrefix + "Cancelling tasks"); Shutdown(true, true); // cancel all tasks, wait for the current one to end - HostingEnvironment.UnregisterObject(this); - _logger.Info(_logPrefix + "Down."); - // raise the completed event: there's no more task running - Completed.RaiseEvent(EventArgs.Empty, this); + Terminate(true); } } + + private void Terminate(bool immediate) + { + // signal the environment we have terminated + // log + // raise the Terminated event + // complete the awaitable completion source, if any + + HostingEnvironment.UnregisterObject(this); + _logger.Info(_logPrefix + "Tasks " + (immediate ? "cancelled" : "completed") + ", terminated"); + OnEvent(Terminated, "Terminated"); + + TaskCompletionSource terminatedSource; + lock (_locker) + { + _terminated = true; + terminatedSource = _terminatedSource; + } + if (terminatedSource != null) + terminatedSource.SetResult(0); + } } } diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerAwaiter.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerAwaiter.cs deleted file mode 100644 index 156da6e115..0000000000 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerAwaiter.cs +++ /dev/null @@ -1,85 +0,0 @@ -using System; -using System.Runtime.CompilerServices; -using System.Threading.Tasks; -using ClientDependency.Core.Logging; -using Umbraco.Core.Logging; -using ILogger = Umbraco.Core.Logging.ILogger; - -namespace Umbraco.Web.Scheduling -{ - /// - /// Custom awaiter used to await when the BackgroundTaskRunner is completed (IsRunning == false) - /// - /// - /// - /// This custom awaiter simply uses a TaskCompletionSource to set the result when the Completed event of the - /// BackgroundTaskRunner executes. - /// A custom awaiter requires implementing INotifyCompletion as well as IsCompleted, OnCompleted and GetResult - /// see: http://blogs.msdn.com/b/pfxteam/archive/2011/01/13/10115642.aspx - /// - internal class BackgroundTaskRunnerAwaiter : INotifyCompletion - where T : class, IBackgroundTask - { - private readonly BackgroundTaskRunner _runner; - private readonly ILogger _logger; - private readonly TaskCompletionSource _tcs; - private readonly TaskAwaiter _awaiter; - - public BackgroundTaskRunnerAwaiter(BackgroundTaskRunner runner, ILogger logger) - { - if (runner == null) throw new ArgumentNullException("runner"); - if (logger == null) throw new ArgumentNullException("logger"); - _runner = runner; - _logger = logger; - - _tcs = new TaskCompletionSource(); - _awaiter = _tcs.Task.GetAwaiter(); - - if (_runner.IsRunning) - { - _runner.Stopped += (s, e) => - { - _logger.Debug>("Runner has stopped."); - _tcs.SetResult(0); - }; - } - else - { - //not running, just set the result - LogHelper.Debug>("Runner is stopped."); - _tcs.SetResult(0); - } - - } - - public BackgroundTaskRunnerAwaiter GetAwaiter() - { - return this; - } - - /// - /// This is completed when the runner is finished running - /// - public bool IsCompleted - { - get - { - // FIXME I DONT UNDERSTAND - _logger.Debug>("IsCompleted :: " + _tcs.Task.IsCompleted + ", " + (_runner.IsRunning == false)); - - //Need to check if the task is completed because it might already be done on the ctor and the runner never runs - return _tcs.Task.IsCompleted || _runner.IsRunning == false; - } - } - - public void OnCompleted(Action continuation) - { - _awaiter.OnCompleted(continuation); - } - - public void GetResult() - { - _awaiter.GetResult(); - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs b/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs index 0ad4d42bdf..79379cb966 100644 --- a/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs +++ b/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs @@ -21,6 +21,7 @@ namespace Umbraco.Web.Scheduling /// /// Gets a value indicating whether the task is latched. /// + /// Should return false as soon as the condition is met. bool IsLatched { get; } /// diff --git a/src/Umbraco.Web/Scheduling/ThreadingTaskAwaiter.cs b/src/Umbraco.Web/Scheduling/ThreadingTaskAwaiter.cs deleted file mode 100644 index 36c00dfa9e..0000000000 --- a/src/Umbraco.Web/Scheduling/ThreadingTaskAwaiter.cs +++ /dev/null @@ -1,42 +0,0 @@ -using System; -using System.Runtime.CompilerServices; -using System.Threading.Tasks; - -namespace Umbraco.Web.Scheduling -{ - /// - /// This is used to return an awaitable instance from a Task without actually returning the - /// underlying Task instance since it shouldn't be mutable. - /// - internal class ThreadingTaskAwaiter - { - private readonly Task _task; - - public ThreadingTaskAwaiter(Task task) - { - if (task == null) throw new ArgumentNullException("task"); - _task = task; - } - - /// - /// With a GetAwaiter declared it means that this instance can be awaited on with the await keyword - /// - /// - public TaskAwaiter GetAwaiter() - { - return _task.GetAwaiter(); - } - - /// - /// Gets the status of the running task. - /// - /// There is no running task. - /// Unless the AutoStart option is true, there will be no running task until - /// a background task is added to the queue. Unless the KeepAlive option is true, there - /// will be no running task when the queue is empty. - public TaskStatus Status - { - get { return _task.Status; } - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ThreadingTaskImmutable.cs b/src/Umbraco.Web/Scheduling/ThreadingTaskImmutable.cs new file mode 100644 index 0000000000..e8ccbeac0e --- /dev/null +++ b/src/Umbraco.Web/Scheduling/ThreadingTaskImmutable.cs @@ -0,0 +1,44 @@ +using System; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; + +namespace Umbraco.Web.Scheduling +{ + /// + /// Wraps a Task within an object that gives access to its GetAwaiter method and Status + /// property while ensuring that it cannot be modified in any way. + /// + internal class ThreadingTaskImmutable + { + private readonly Task _task; + + /// + /// Initializes a new instance of the class with a Task. + /// + /// The task. + public ThreadingTaskImmutable(Task task) + { + if (task == null) + throw new ArgumentNullException("task"); + _task = task; + } + + /// + /// Gets an awaiter used to await the task. + /// + /// An awaiter instance. + public TaskAwaiter GetAwaiter() + { + return _task.GetAwaiter(); + } + + /// + /// Gets the TaskStatus of the task. + /// + /// The current TaskStatus of the task. + public TaskStatus Status + { + get { return _task.Status; } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 193660000c..be49d3c7a9 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -321,7 +321,7 @@ - + @@ -551,7 +551,6 @@ - diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index 5d0e156333..7b9b726223 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -60,10 +60,19 @@ namespace umbraco KeepAlive = true }, logger); - // when the runner has stopped we know we will not be writing - // to the file anymore, so we can release the lock now - and - // not wait for the AppDomain unload - runner.Completed += (sender, args) => + // when the runner is terminating we need to ensure that no modifications + // to content are possible anymore, as they would not be written out to + // the xml file - unfortunately that is not possible in 7.x because we + // cannot lock the content service... and so we do nothing... + //runner.Terminating += (sender, args) => + //{ + //}; + + // when the runner has terminated we know we will not be writing to the file + // anymore, so we can release the lock now - no need to wait for the AppDomain + // unload - which means any "last minute" saves will be lost - but waiting for + // the AppDomain to unload has issues... + runner.Terminated += (sender, args) => { if (_fileLock == null) return; // not locking (testing?) if (_fileLocked == null) return; // not locked