From ffaf1f96b9484dd74b2181c60fc71e934ae480f7 Mon Sep 17 00:00:00 2001 From: Per Osbeck Date: Tue, 24 Mar 2015 09:28:53 +0100 Subject: [PATCH 01/13] Fix for U4-6444 --- .../src/common/services/mediahelper.service.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js index 67dbcecc54..68ca32efbc 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js @@ -327,6 +327,11 @@ function mediaHelper(umbRequestHelper) { * @param {string} imagePath Image path, ex: /media/1234/my-image.jpg */ detectIfImageByExtension: function (imagePath) { + + if (!imagePath) { + return false; + } + var lowered = imagePath.toLowerCase(); var ext = lowered.substr(lowered.lastIndexOf(".") + 1); return ("," + Umbraco.Sys.ServerVariables.umbracoSettings.imageFileTypes + ",").indexOf("," + ext + ",") !== -1; From 9dfccd86fd3961712be529042941c2a62f7c6bb8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Tue, 24 Mar 2015 20:09:48 +0100 Subject: [PATCH 02/13] U4-2499 Remove the possibility for accessing pages with nodeid's or give a redirect option #U4-2499 Fixed Due in version 7.2.5 --- .../UmbracoSettings/IWebRoutingSection.cs | 2 ++ .../Configuration/UmbracoSettings/WebRoutingElement.cs | 5 +++++ .../config/umbracoSettings.Release.config | 6 +++++- src/Umbraco.Web.UI/config/umbracoSettings.config | 10 +++++++--- src/Umbraco.Web/Routing/ContentFinderByIdPath.cs | 4 ++++ 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/IWebRoutingSection.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/IWebRoutingSection.cs index 393387ecfa..f3d42b6904 100644 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/IWebRoutingSection.cs +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/IWebRoutingSection.cs @@ -8,6 +8,8 @@ bool DisableAlternativeTemplates { get; } + bool DisableFindContentByIdPath { get; } + string UrlProviderMode { get; } } diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/WebRoutingElement.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/WebRoutingElement.cs index 82c5a37575..f5b71eb2c7 100644 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/WebRoutingElement.cs +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/WebRoutingElement.cs @@ -21,6 +21,11 @@ namespace Umbraco.Core.Configuration.UmbracoSettings { get { return (bool) base["disableAlternativeTemplates"]; } } + [ConfigurationProperty("disableFindContentByIdPath", DefaultValue = "false")] + public bool DisableFindContentByIdPath + { + get { return (bool) base["disableFindContentByIdPath"]; } + } [ConfigurationProperty("urlProviderMode", DefaultValue = "AutoLegacy")] public string UrlProviderMode diff --git a/src/Umbraco.Web.UI/config/umbracoSettings.Release.config b/src/Umbraco.Web.UI/config/umbracoSettings.Release.config index 2959b0388b..b371f536e6 100644 --- a/src/Umbraco.Web.UI/config/umbracoSettings.Release.config +++ b/src/Umbraco.Web.UI/config/umbracoSettings.Release.config @@ -137,10 +137,14 @@ will make Umbraco render the content on the current page with the template you requested, for example: http://mysite.com/about-us/?altTemplate=Home and http://mysite.com/about-us/Home would render the "About Us" page with a template with the alias Home. Setting this setting to true stops that behavior + @disableFindContentByIdPath + By default you can call any content Id in the url and show the content with that id, for example: + http://mysite.com/1092 or http://mysite.com/1092.aspx would render the content with id 1092. Setting + this setting to true stops that behavior --> + internalRedirectPreservesTemplate="false" disableAlternativeTemplates="false" disableFindContentByIdPath="false"> diff --git a/src/Umbraco.Web.UI/config/umbracoSettings.config b/src/Umbraco.Web.UI/config/umbracoSettings.config index e13441b802..f03cc08d9f 100644 --- a/src/Umbraco.Web.UI/config/umbracoSettings.config +++ b/src/Umbraco.Web.UI/config/umbracoSettings.config @@ -295,10 +295,14 @@ will make Umbraco render the content on the current page with the template you requested, for example: http://mysite.com/about-us/?altTemplate=Home and http://mysite.com/about-us/Home would render the "About Us" page with a template with the alias Home. Setting this setting to true stops that behavior + @disableFindContentByIdPath + By default you can call any content Id in the url and show the content with that id, for example: + http://mysite.com/1092 or http://mysite.com/1092.aspx would render the content with id 1092. Setting + this setting to true stops that behavior --> - + \ No newline at end of file diff --git a/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs b/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs index 9bae24c5dd..90da48039c 100644 --- a/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs +++ b/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs @@ -2,6 +2,7 @@ using System; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core; +using Umbraco.Core.Configuration; namespace Umbraco.Web.Routing { @@ -20,6 +21,9 @@ namespace Umbraco.Web.Routing /// A value indicating whether an Umbraco document was found and assigned. public bool TryFindContent(PublishedContentRequest docRequest) { + if (UmbracoConfig.For.UmbracoSettings().WebRouting.DisableFindContentByIdPath) + return false; + IPublishedContent node = null; var path = docRequest.Uri.GetAbsolutePathDecoded(); From e7754710136515643fc881b9c816e64ae5dcd32f Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Wed, 25 Mar 2015 17:08:53 +0100 Subject: [PATCH 03/13] Set disableFindContentByIdPath=false for local debugging of Umb source (the release config was already correct) --- src/Umbraco.Web.UI/config/umbracoSettings.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI/config/umbracoSettings.config b/src/Umbraco.Web.UI/config/umbracoSettings.config index f03cc08d9f..616b5ba424 100644 --- a/src/Umbraco.Web.UI/config/umbracoSettings.config +++ b/src/Umbraco.Web.UI/config/umbracoSettings.config @@ -302,7 +302,7 @@ --> + internalRedirectPreservesTemplate="false" disableAlternativeTemplates="false" disableFindContentByIdPath="false"> \ No newline at end of file From a82035061ccbbc1435d00289becafafe7811c19d Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 29 Jan 2015 12:45:44 +1100 Subject: [PATCH 04/13] Updates BackgroundTaskRunner to support async operations. Updates the content xml cache file persisting to use a IBackgroundTask instead of the strange queuing for persistence - which was super strange because in many cases another request thread will actually be the thread that is persisting the xml file than the request thread that requested it created. This implementation is far better, the xml file will be persisted on a non request thread and will handle multiple documents being published at the same time guaranteeing that the latest published version is the one persisted. The file persistence is also web aware (due to how BackgroundTaskRunner works) so during app shutdown the file will still be written if it's currently being processed. --- src/Umbraco.Core/XmlExtensions.cs | 28 ++++ .../Scheduling/BackgroundTaskRunnerTests.cs | 10 ++ .../XmlCacheFilePersister.cs | 104 ++++++++++++ .../Scheduling/BackgroundTaskRunner.cs | 32 ++-- src/Umbraco.Web/Scheduling/IBackgroundTask.cs | 3 + src/Umbraco.Web/Scheduling/LogScrubber.cs | 11 ++ .../Scheduling/ScheduledPublishing.cs | 11 ++ src/Umbraco.Web/Scheduling/ScheduledTasks.cs | 11 ++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + src/Umbraco.Web/UmbracoModule.cs | 28 +--- .../umbraco.presentation/content.cs | 149 +++--------------- .../umbraco.presentation/requestModule.cs | 8 +- 12 files changed, 227 insertions(+), 169 deletions(-) create mode 100644 src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs diff --git a/src/Umbraco.Core/XmlExtensions.cs b/src/Umbraco.Core/XmlExtensions.cs index 02ebc07490..8784cbfb30 100644 --- a/src/Umbraco.Core/XmlExtensions.cs +++ b/src/Umbraco.Core/XmlExtensions.cs @@ -1,6 +1,9 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text; +using System.Threading.Tasks; using System.Xml; using System.Xml.Linq; using System.Xml.XPath; @@ -13,6 +16,31 @@ namespace Umbraco.Core /// internal static class XmlExtensions { + /// + /// Saves the xml document async + /// + /// + /// + /// + public static async Task SaveAsync(this XmlDocument xdoc, string filename) + { + if (xdoc.DocumentElement == null) + throw new XmlException("Cannot save xml document, there is no root element"); + + using (var fs = new FileStream(filename, FileMode.Create, FileAccess.Write, FileShare.Read, bufferSize: 4096, useAsync: true)) + using (var xmlWriter = XmlWriter.Create(fs, new XmlWriterSettings + { + Async = true, + Encoding = Encoding.UTF8, + Indent = true + })) + { + //NOTE: There are no nice methods to write it async, only flushing it async. We + // could implement this ourselves but it'd be a very manual process. + xdoc.WriteTo(xmlWriter); + await xmlWriter.FlushAsync(); + } + } public static bool HasAttribute(this XmlAttributeCollection attributes, string attributeName) { diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 8b1981bc9e..7cc763e534 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -265,6 +265,16 @@ namespace Umbraco.Tests.Scheduling public Guid UniqueId { get; protected set; } public abstract void Run(); + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } + public abstract void Cancel(); public DateTime Queued { get; set; } diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs new file mode 100644 index 0000000000..482a666538 --- /dev/null +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs @@ -0,0 +1,104 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using System.Xml; +using Umbraco.Core; +using Umbraco.Core.Logging; +using Umbraco.Web.Scheduling; + +namespace Umbraco.Web.PublishedCache.XmlPublishedCache +{ + /// + /// This is the background task runner that persists the xml file to the file system + /// + /// + /// This is used so that all file saving is done on a web aware worker background thread and all logic is performed async so this + /// process will not interfere with any web requests threads. This is also done as to not require any global locks and to ensure that + /// if multiple threads are performing publishing tasks that the file will be persisted in accordance with the final resulting + /// xml structure since the file writes are queued. + /// + internal class XmlCacheFilePersister : DisposableObject, IBackgroundTask + { + private readonly XmlDocument _xDoc; + private readonly string _xmlFileName; + private readonly ProfilingLogger _logger; + + public XmlCacheFilePersister(XmlDocument xDoc, string xmlFileName, ProfilingLogger logger) + { + _xDoc = xDoc; + _xmlFileName = xmlFileName; + _logger = logger; + } + + public async Task RunAsync() + { + await PersistXmlToFileAsync(_xDoc); + } + + public bool IsAsync + { + get { return true; } + } + + /// + /// Persist a XmlDocument to the Disk Cache + /// + /// + internal async Task PersistXmlToFileAsync(XmlDocument xmlDoc) + { + if (xmlDoc != null) + { + using (_logger.DebugDuration( + string.Format("Saving content to disk on thread '{0}' (Threadpool? {1})", Thread.CurrentThread.Name, Thread.CurrentThread.IsThreadPoolThread), + string.Format("Saved content to disk on thread '{0}' (Threadpool? {1})", Thread.CurrentThread.Name, Thread.CurrentThread.IsThreadPoolThread))) + { + try + { + // Try to create directory for cache path if it doesn't yet exist + var directoryName = Path.GetDirectoryName(_xmlFileName); + // create dir if it is not there, if it's there, this will proceed as normal + Directory.CreateDirectory(directoryName); + + await xmlDoc.SaveAsync(_xmlFileName); + } + catch (Exception ee) + { + // If for whatever reason something goes wrong here, invalidate disk cache + DeleteXmlCache(); + + LogHelper.Error("Error saving content to disk", ee); + } + } + + + } + } + + private void DeleteXmlCache() + { + if (File.Exists(_xmlFileName) == false) return; + + // Reset file attributes, to make sure we can delete file + try + { + File.SetAttributes(_xmlFileName, FileAttributes.Normal); + } + finally + { + File.Delete(_xmlFileName); + } + } + + protected override void DisposeResources() + { + } + + public void Run() + { + throw new NotImplementedException(); + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index a0c6720a88..60eef43b17 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -146,7 +146,8 @@ namespace Umbraco.Web.Scheduling T remainingTask; while (_tasks.TryTake(out remainingTask)) { - ConsumeTaskInternal(remainingTask); + ConsumeTaskInternalAsync(remainingTask) + .Wait(); //block until it completes } } @@ -178,7 +179,7 @@ namespace Umbraco.Web.Scheduling var token = _tokenSource.Token; _consumer = Task.Factory.StartNew(() => - StartThread(token), + StartThreadAsync(token), token, _dedicatedThread ? TaskCreationOptions.LongRunning : TaskCreationOptions.None, TaskScheduler.Default); @@ -197,7 +198,7 @@ namespace Umbraco.Web.Scheduling /// Invokes a new worker thread to consume tasks /// /// - private void StartThread(CancellationToken token) + private async Task StartThreadAsync(CancellationToken token) { // Was cancellation already requested? if (token.IsCancellationRequested) @@ -206,14 +207,14 @@ namespace Umbraco.Web.Scheduling token.ThrowIfCancellationRequested(); } - TakeAndConsumeTask(token); + await TakeAndConsumeTaskAsync(token); } /// /// Trys to get a task from the queue, if there isn't one it will wait a second and try again /// /// - private void TakeAndConsumeTask(CancellationToken token) + private async Task TakeAndConsumeTaskAsync(CancellationToken token) { if (token.IsCancellationRequested) { @@ -235,25 +236,25 @@ namespace Umbraco.Web.Scheduling // cancel when we shutdown foreach (var t in _tasks.GetConsumingEnumerable(token)) { - ConsumeTaskCancellable(t, token); + await ConsumeTaskCancellableAsync(t, token); } //recurse and keep going - TakeAndConsumeTask(token); + await TakeAndConsumeTaskAsync(token); } else { T repositoryTask; while (_tasks.TryTake(out repositoryTask)) { - ConsumeTaskCancellable(repositoryTask, token); + await ConsumeTaskCancellableAsync(repositoryTask, token); } //the task will end here } } - internal void ConsumeTaskCancellable(T task, CancellationToken token) + internal async Task ConsumeTaskCancellableAsync(T task, CancellationToken token) { if (token.IsCancellationRequested) { @@ -266,10 +267,10 @@ namespace Umbraco.Web.Scheduling token.ThrowIfCancellationRequested(); } - ConsumeTaskInternal(task); + await ConsumeTaskInternalAsync(task); } - private void ConsumeTaskInternal(T task) + private async Task ConsumeTaskInternalAsync(T task) { try { @@ -279,7 +280,14 @@ namespace Umbraco.Web.Scheduling { using (task) { - task.Run(); + if (task.IsAsync) + { + await task.RunAsync(); + } + else + { + task.Run(); + } } } catch (Exception e) diff --git a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs index 343f076b2a..48522aeb5f 100644 --- a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs +++ b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs @@ -1,9 +1,12 @@ using System; +using System.Threading.Tasks; namespace Umbraco.Web.Scheduling { internal interface IBackgroundTask : IDisposable { void Run(); + Task RunAsync(); + bool IsAsync { get; } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index 1c7a8f3537..2edbe80726 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using System.Web; using System.Web.Caching; using umbraco.BusinessLogic; @@ -49,5 +50,15 @@ namespace Umbraco.Web.Scheduling Log.CleanLogs(GetLogScrubbingMaximumAge(_settings)); } } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index 8a64098764..cacb4e133d 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -2,6 +2,7 @@ using System; using System.Diagnostics; using System.Net; using System.Text; +using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; @@ -75,5 +76,15 @@ namespace Umbraco.Web.Scheduling } } } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs index 94c035631f..ddcb9ea533 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs @@ -2,6 +2,7 @@ using System; using System.Collections; using System.Linq; using System.Net; +using System.Threading.Tasks; using System.Xml; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; @@ -106,5 +107,15 @@ namespace Umbraco.Web.Scheduling } } } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 82433abf49..8cd6f55046 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -543,6 +543,7 @@ ASPXCodeBehind + True True diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index 633df5c6d5..f2d216f2fd 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using System.Security.Principal; using System.Threading; +using System.Threading.Tasks; using System.Web; using System.Web.Mvc; using System.Web.Routing; @@ -529,24 +530,8 @@ namespace Umbraco.Web urlRouting.PostResolveRequestCache(context); } - /// - /// Checks if the xml cache file needs to be updated/persisted - /// - /// - /// - /// TODO: This needs an overhaul, see the error report created here: - /// https://docs.google.com/document/d/1neGE3q3grB4lVJfgID1keWY2v9JYqf-pw75sxUUJiyo/edit - /// - static void PersistXmlCache(HttpContextBase httpContext) - { - if (content.Instance.IsXmlQueuedForPersistenceToFile) - { - content.Instance.RemoveXmlFilePersistenceQueue(); - content.Instance.PersistXmlToFile(); - } - } - - /// + + /// /// Any object that is in the HttpContext.Items collection that is IDisposable will get disposed on the end of the request /// /// @@ -613,13 +598,6 @@ namespace Umbraco.Web ProcessRequest(new HttpContextWrapper(httpContext)); }; - // used to check if the xml cache file needs to be updated/persisted - app.PostRequestHandlerExecute += (sender, e) => - { - var httpContext = ((HttpApplication)sender).Context; - PersistXmlCache(new HttpContextWrapper(httpContext)); - }; - app.EndRequest += (sender, args) => { var httpContext = ((HttpApplication)sender).Context; diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index 30bfd1c901..2905abaf99 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -1,33 +1,28 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.IO; -using System.Linq; using System.Threading; +using System.Threading.Tasks; using System.Web; using System.Xml; -using System.Xml.XPath; -using umbraco.cms.presentation; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; using Umbraco.Core.IO; using Umbraco.Core.Logging; using umbraco.BusinessLogic; -using umbraco.BusinessLogic.Actions; -using umbraco.BusinessLogic.Utils; using umbraco.cms.businesslogic; -using umbraco.cms.businesslogic.cache; using umbraco.cms.businesslogic.web; using Umbraco.Core.Models; +using Umbraco.Core.Profiling; using umbraco.DataLayer; using umbraco.presentation.nodeFactory; using Umbraco.Web; -using Action = umbraco.BusinessLogic.Actions.Action; +using Umbraco.Web.PublishedCache.XmlPublishedCache; +using Umbraco.Web.Scheduling; using Node = umbraco.NodeFactory.Node; -using Umbraco.Core; using File = System.IO.File; namespace umbraco @@ -37,6 +32,8 @@ namespace umbraco /// public class content { + private static readonly BackgroundTaskRunner FilePersister = new BackgroundTaskRunner(dedicatedThread: true); + #region Declarations // Sync access to disk file @@ -75,7 +72,6 @@ namespace umbraco #endregion - #region Singleton private static readonly Lazy LazyInstance = new Lazy(() => new content()); @@ -320,8 +316,8 @@ namespace umbraco // and clear the queue in case is this a web request, we don't want it reprocessing. if (UmbracoConfig.For.UmbracoSettings().Content.XmlCacheEnabled && UmbracoConfig.For.UmbracoSettings().Content.ContinouslyUpdateXmlDiskCache) { - RemoveXmlFilePersistenceQueue(); - PersistXmlToFile(xmlDoc); + FilePersister.Add(new XmlCacheFilePersister(xmlDoc, UmbracoXmlDiskCacheFileName , + new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler))); } } } @@ -929,54 +925,6 @@ namespace umbraco #region Protected & Private methods - internal const string PersistenceFlagContextKey = "vnc38ykjnkjdnk2jt98ygkxjng"; - - /// - /// Removes the flag that queues the file for persistence - /// - internal void RemoveXmlFilePersistenceQueue() - { - if (UmbracoContext.Current != null && UmbracoContext.Current.HttpContext != null) - { - UmbracoContext.Current.HttpContext.Application.Lock(); - UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] = null; - UmbracoContext.Current.HttpContext.Application.UnLock(); - } - } - - internal bool IsXmlQueuedForPersistenceToFile - { - get - { - if (UmbracoContext.Current != null && UmbracoContext.Current.HttpContext != null) - { - bool val = UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] != null; - if (val) - { - DateTime persistenceTime = DateTime.MinValue; - try - { - persistenceTime = (DateTime)UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey]; - if (persistenceTime > GetCacheFileUpdateTime()) - { - return true; - } - else - { - RemoveXmlFilePersistenceQueue(); - } - } - catch (Exception ex) - { - // Nothing to catch here - we'll just persist - LogHelper.Error("An error occurred checking if xml file is queued for persistence", ex); - } - } - } - return false; - } - } - /// /// Invalidates the disk content cache file. Effectively just deletes it. /// @@ -1248,51 +1196,26 @@ order by umbracoNode.level, umbracoNode.sortOrder"; } } + [Obsolete("This method should not be used, xml file persistence is done in a queue using a BackgroundTaskRunner")] public void PersistXmlToFile() - { - PersistXmlToFile(_xmlContent); - } - - /// - /// Persist a XmlDocument to the Disk Cache - /// - /// - internal void PersistXmlToFile(XmlDocument xmlDoc) { lock (ReaderWriterSyncLock) { - if (xmlDoc != null) - { - LogHelper.Debug("Saving content to disk on thread '{0}' (Threadpool? {1})", - () => Thread.CurrentThread.Name, - () => Thread.CurrentThread.IsThreadPoolThread); - + if (_xmlContent != null) + { try { - Stopwatch stopWatch = Stopwatch.StartNew(); + // create directory for cache path if it doesn't yet exist + var directoryName = Path.GetDirectoryName(UmbracoXmlDiskCacheFileName); + Directory.CreateDirectory(directoryName); - DeleteXmlCache(); - - // Try to create directory for cache path if it doesn't yet exist - string directoryName = Path.GetDirectoryName(UmbracoXmlDiskCacheFileName); - if (!File.Exists(UmbracoXmlDiskCacheFileName) && !Directory.Exists(directoryName)) - { - // We're already in a try-catch and saving will fail if this does, so don't need another - Directory.CreateDirectory(directoryName); - } - - xmlDoc.Save(UmbracoXmlDiskCacheFileName); - - LogHelper.Debug("Saved content on thread '{0}' in {1} (Threadpool? {2})", - () => Thread.CurrentThread.Name, - () => stopWatch.Elapsed, - () => Thread.CurrentThread.IsThreadPoolThread); + _xmlContent.Save(UmbracoXmlDiskCacheFileName); } catch (Exception ee) { // If for whatever reason something goes wrong here, invalidate disk cache DeleteXmlCache(); - + LogHelper.Error(string.Format( "Error saving content on thread '{0}' due to '{1}' (Threadpool? {2})", Thread.CurrentThread.Name, ee.Message, Thread.CurrentThread.IsThreadPoolThread), ee); @@ -1302,48 +1225,18 @@ order by umbracoNode.level, umbracoNode.sortOrder"; } /// - /// Marks a flag in the HttpContext so that, upon page execution completion, the Xml cache will - /// get persisted to disk. Ensure this method is only called from a thread executing a page request - /// since UmbracoModule is the only monitor of this flag and is responsible - /// for enacting the persistence at the PostRequestHandlerExecute stage of the page lifecycle. + /// Adds a task to the xml cache file persister /// private void QueueXmlForPersistence() { - //if this is called outside a web request we cannot queue it it will run in the current thread. - - if (UmbracoContext.Current != null && UmbracoContext.Current.HttpContext != null) - { - UmbracoContext.Current.HttpContext.Application.Lock(); - try - { - if (UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] != null) - { - UmbracoContext.Current.HttpContext.Application.Add(PersistenceFlagContextKey, null); - } - UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] = DateTime.UtcNow; - } - finally - { - UmbracoContext.Current.HttpContext.Application.UnLock(); - } - } - else - { - // Save copy of content - if (UmbracoSettings.CloneXmlCacheOnPublish) - { - XmlDocument xmlContentCopy = CloneXmlDoc(_xmlContent); - PersistXmlToFile(xmlContentCopy); - } - else - { - PersistXmlToFile(); - } - } + FilePersister.Add(new XmlCacheFilePersister(_xmlContent, UmbracoXmlDiskCacheFileName, + new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler))); } internal DateTime GetCacheFileUpdateTime() { + //TODO: Should there be a try/catch here in case the file is being written to while this is trying to be executed? + if (File.Exists(UmbracoXmlDiskCacheFileName)) { return new FileInfo(UmbracoXmlDiskCacheFileName).LastWriteTimeUtc; diff --git a/src/Umbraco.Web/umbraco.presentation/requestModule.cs b/src/Umbraco.Web/umbraco.presentation/requestModule.cs index 6be1737545..9e2eeff4e6 100644 --- a/src/Umbraco.Web/umbraco.presentation/requestModule.cs +++ b/src/Umbraco.Web/umbraco.presentation/requestModule.cs @@ -317,10 +317,10 @@ namespace umbraco.presentation void context_PostRequestHandlerExecute(object sender, EventArgs e) { - if (content.Instance.IsXmlQueuedForPersistenceToFile) - { - content.Instance.PersistXmlToFile(); - } + //if (content.Instance.IsXmlQueuedForPersistenceToFile) + //{ + // content.Instance.PersistXmlToFile(); + //} } From 912b01c9aa5321674d626e9bb96b773ec39b1e5f Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 4 Feb 2015 14:59:33 +1100 Subject: [PATCH 05/13] Updates BackgroundTaskRunner to support more complex options including the ability to only execute the last/final task in the queue. Added tests to support, updated the 'content' object to use this option so that only the last task in the queue will execute so that file persisting doesn't get queued but the correctly queued data will be written. --- .../Scheduling/BackgroundTaskRunnerTests.cs | 24 +++++++ .../Scheduling/BackgroundTaskRunner.cs | 66 ++++++++++++++++--- .../umbraco.presentation/content.cs | 3 +- 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 7cc763e534..3e851a68f0 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -83,6 +83,30 @@ namespace Umbraco.Tests.Scheduling } } + [Test] + public async void Many_Tasks_Added_Only_Last_Task_Executes_With_Option() + { + var tasks = new Dictionary(); + for (var i = 0; i < 10; i++) + { + tasks.Add(new MyTask(), new ManualResetEvent(false)); + } + + BackgroundTaskRunner tManager; + using (tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions{OnlyProcessLastItem = true})) + { + + tasks.ForEach(t => tManager.Add(t.Key)); + + //wait till the thread is done + await tManager; + + var countExecuted = tasks.Count(x => x.Key.Ended != default(DateTime)); + + Assert.AreEqual(1, countExecuted); + } + } + [Test] public void Tasks_Can_Keep_Being_Added_And_Will_Execute() { diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index 60eef43b17..d11ff03d66 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -8,6 +8,26 @@ using Umbraco.Core.Logging; namespace Umbraco.Web.Scheduling { + + internal class BackgroundTaskRunnerOptions + { + public BackgroundTaskRunnerOptions() + { + DedicatedThread = false; + PersistentThread = false; + OnlyProcessLastItem = false; + } + + public bool DedicatedThread { get; set; } + public bool PersistentThread { get; set; } + + /// + /// If this is true, the task runner will skip over all items and only process the last/final + /// item registered + /// + public bool OnlyProcessLastItem { get; set; } + } + /// /// This is used to create a background task runner which will stay alive in the background of and complete /// any tasks that are queued. It is web aware and will ensure that it is shutdown correctly when the app domain @@ -17,8 +37,7 @@ namespace Umbraco.Web.Scheduling internal class BackgroundTaskRunner : IDisposable, IRegisteredObject where T : IBackgroundTask { - private readonly bool _dedicatedThread; - private readonly bool _persistentThread; + private readonly BackgroundTaskRunnerOptions _options; private readonly BlockingCollection _tasks = new BlockingCollection(); private Task _consumer; @@ -31,9 +50,15 @@ namespace Umbraco.Web.Scheduling internal event EventHandler> TaskCancelled; public BackgroundTaskRunner(bool dedicatedThread = false, bool persistentThread = false) + : this(new BackgroundTaskRunnerOptions{DedicatedThread = dedicatedThread, PersistentThread = persistentThread}) + { + } + + public BackgroundTaskRunner(BackgroundTaskRunnerOptions options) { - _dedicatedThread = dedicatedThread; - _persistentThread = persistentThread; + if (options == null) throw new ArgumentNullException("options"); + _options = options; + HostingEnvironment.RegisterObject(this); } @@ -146,6 +171,13 @@ namespace Umbraco.Web.Scheduling T remainingTask; while (_tasks.TryTake(out remainingTask)) { + //skip if this is not the last + if (_options.OnlyProcessLastItem && _tasks.Count > 0) + { + //NOTE: don't raise canceled event, we're shutting down + continue; + } + ConsumeTaskInternalAsync(remainingTask) .Wait(); //block until it completes } @@ -181,13 +213,13 @@ namespace Umbraco.Web.Scheduling _consumer = Task.Factory.StartNew(() => StartThreadAsync(token), token, - _dedicatedThread ? TaskCreationOptions.LongRunning : TaskCreationOptions.None, + _options.DedicatedThread ? TaskCreationOptions.LongRunning : TaskCreationOptions.None, TaskScheduler.Default); //if this is not a persistent thread, wait till it's done and shut ourselves down // thus ending the thread or giving back to the thread pool. If another task is added // another thread will spawn or be taken from the pool to process. - if (!_persistentThread) + if (!_options.PersistentThread) { _consumer.ContinueWith(task => ShutDown()); } @@ -228,7 +260,7 @@ namespace Umbraco.Web.Scheduling //When this is false, the thread will process what is currently in the queue and once that is // done, the thread will end and we will shutdown the process - if (_persistentThread) + if (_options.PersistentThread) { //This will iterate over the collection, if there is nothing to take // the thread will block until there is something available. @@ -236,6 +268,13 @@ namespace Umbraco.Web.Scheduling // cancel when we shutdown foreach (var t in _tasks.GetConsumingEnumerable(token)) { + //skip if this is not the last + if (_options.OnlyProcessLastItem && _tasks.Count > 0) + { + OnTaskCancelled(new TaskEventArgs(t)); + continue; + } + await ConsumeTaskCancellableAsync(t, token); } @@ -244,10 +283,17 @@ namespace Umbraco.Web.Scheduling } else { - T repositoryTask; - while (_tasks.TryTake(out repositoryTask)) + T t; + while (_tasks.TryTake(out t)) { - await ConsumeTaskCancellableAsync(repositoryTask, token); + //skip if this is not the last + if (_options.OnlyProcessLastItem && _tasks.Count > 0) + { + OnTaskCancelled(new TaskEventArgs(t)); + continue; + } + + await ConsumeTaskCancellableAsync(t, token); } //the task will end here diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index 2905abaf99..cd736b98ff 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -32,7 +32,8 @@ namespace umbraco /// public class content { - private static readonly BackgroundTaskRunner FilePersister = new BackgroundTaskRunner(dedicatedThread: true); + private static readonly BackgroundTaskRunner FilePersister + = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions {DedicatedThread = true, OnlyProcessLastItem = true}); #region Declarations From bc068b201d4b937e4ead37df2c10d212f1dc635a Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 4 Feb 2015 15:12:37 +1100 Subject: [PATCH 06/13] Updates BackgroundTaskRunner to ensure canceled or skipped tasks are disposed, updated tests to reflect --- .../Scheduling/BackgroundTaskRunnerTests.cs | 22 ++++++++++------ .../Scheduling/BackgroundTaskRunner.cs | 26 ++++--------------- .../Scheduling/BackgroundTaskRunnerOptions.cs | 23 ++++++++++++++++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + 4 files changed, 43 insertions(+), 29 deletions(-) create mode 100644 src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 3e851a68f0..11ba81b585 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -273,22 +273,25 @@ namespace Umbraco.Tests.Scheduling { } - public override void Run() + public override void PerformRun() { Thread.Sleep(500); } - public override void Cancel() - { - - } } public abstract class BaseTask : IBackgroundTask { + public bool WasCancelled { get; set; } + public Guid UniqueId { get; protected set; } - public abstract void Run(); + public abstract void PerformRun(); + public void Run() + { + PerformRun(); + Ended = DateTime.Now; + } public Task RunAsync() { throw new NotImplementedException(); @@ -299,7 +302,10 @@ namespace Umbraco.Tests.Scheduling get { return false; } } - public abstract void Cancel(); + public virtual void Cancel() + { + WasCancelled = true; + } public DateTime Queued { get; set; } public DateTime Started { get; set; } @@ -307,7 +313,7 @@ namespace Umbraco.Tests.Scheduling public virtual void Dispose() { - Ended = DateTime.Now; + } } diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index d11ff03d66..ffba7a1e0a 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -8,26 +8,6 @@ using Umbraco.Core.Logging; namespace Umbraco.Web.Scheduling { - - internal class BackgroundTaskRunnerOptions - { - public BackgroundTaskRunnerOptions() - { - DedicatedThread = false; - PersistentThread = false; - OnlyProcessLastItem = false; - } - - public bool DedicatedThread { get; set; } - public bool PersistentThread { get; set; } - - /// - /// If this is true, the task runner will skip over all items and only process the last/final - /// item registered - /// - public bool OnlyProcessLastItem { get; set; } - } - /// /// This is used to create a background task runner which will stay alive in the background of and complete /// any tasks that are queued. It is web aware and will ensure that it is shutdown correctly when the app domain @@ -174,7 +154,8 @@ namespace Umbraco.Web.Scheduling //skip if this is not the last if (_options.OnlyProcessLastItem && _tasks.Count > 0) { - //NOTE: don't raise canceled event, we're shutting down + //NOTE: don't raise canceled event, we're shutting down, just dispose + remainingTask.Dispose(); continue; } @@ -372,6 +353,9 @@ namespace Umbraco.Web.Scheduling { var handler = TaskCancelled; if (handler != null) handler(this, e); + + //dispose it + e.Task.Dispose(); } diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs new file mode 100644 index 0000000000..c42fcd681a --- /dev/null +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs @@ -0,0 +1,23 @@ +namespace Umbraco.Web.Scheduling +{ + internal class BackgroundTaskRunnerOptions + { + //TODO: Could add options for using a stack vs queue if required + + public BackgroundTaskRunnerOptions() + { + DedicatedThread = false; + PersistentThread = false; + OnlyProcessLastItem = false; + } + + public bool DedicatedThread { get; set; } + public bool PersistentThread { get; set; } + + /// + /// If this is true, the task runner will skip over all items and only process the last/final + /// item registered + /// + public bool OnlyProcessLastItem { get; set; } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 8cd6f55046..23ba69cf31 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -498,6 +498,7 @@ + From b7436dc55f82b25461f2b46c769b325d9519795e Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 6 Feb 2015 16:10:34 +0100 Subject: [PATCH 07/13] refactor BackgroundTaskRunner --- .../Scheduling/BackgroundTaskRunnerTests.cs | 671 ++++++++++++++---- .../Scheduling/BackgroundTaskRunner.cs | 552 ++++++++------ .../Scheduling/BackgroundTaskRunnerOptions.cs | 37 +- .../Scheduling/DelayedRecurringTaskBase.cs | 52 ++ src/Umbraco.Web/Scheduling/IBackgroundTask.cs | 16 + .../Scheduling/IBackgroundTaskRunner.cs | 20 + .../Scheduling/IDelayedBackgroundTask.cs | 23 + .../Scheduling/RecurringTaskBase.cs | 108 +++ .../Scheduling/TaskAndFactoryExtensions.cs | 62 ++ src/Umbraco.Web/Umbraco.Web.csproj | 5 + .../umbraco.presentation/content.cs | 2 +- 11 files changed, 1168 insertions(+), 380 deletions(-) create mode 100644 src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs create mode 100644 src/Umbraco.Web/Scheduling/IBackgroundTaskRunner.cs create mode 100644 src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs create mode 100644 src/Umbraco.Web/Scheduling/RecurringTaskBase.cs create mode 100644 src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 11ba81b585..e83ce400d9 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -1,7 +1,7 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using NUnit.Framework; @@ -13,97 +13,272 @@ namespace Umbraco.Tests.Scheduling [TestFixture] public class BackgroundTaskRunnerTests { - - - [Test] - public void Startup_And_Shutdown() + private static void AssertRunnerStopsRunning(BackgroundTaskRunner runner, int timeoutMilliseconds = 2000) + where T : class, IBackgroundTask { - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(true, true)) - { - tManager.StartUp(); - } + const int period = 200; - NUnit.Framework.Assert.IsFalse(tManager.IsRunning); + var i = 0; + var m = timeoutMilliseconds/period; + while (runner.IsRunning && i++ < m) + Thread.Sleep(period); + Assert.IsFalse(runner.IsRunning, "Runner is still running."); } [Test] - public void Startup_Starts_Automatically() + public void ShutdownWaitWhenRunning() { - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(true, true)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true })) { - tManager.Add(new MyTask()); - NUnit.Framework.Assert.IsTrue(tManager.IsRunning); + Assert.IsTrue(runner.IsRunning); + Thread.Sleep(800); // for long + Assert.IsTrue(runner.IsRunning); + runner.Shutdown(false, true); // -force +wait + AssertRunnerStopsRunning(runner); + Assert.IsTrue(runner.IsCompleted); } } [Test] - public void Task_Runs() + public void ShutdownWhenRunning() { - var myTask = new MyTask(); - var waitHandle = new ManualResetEvent(false); - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(true, true)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - tManager.TaskCompleted += (sender, task) => waitHandle.Set(); + // do NOT try to do this because the code must run on the UI thread which + // is not availably, and so the thread never actually starts - wondering + // what it means for ASP.NET? + //runner.TaskStarting += (sender, args) => Console.WriteLine("starting {0:c}", DateTime.Now); + //runner.TaskCompleted += (sender, args) => Console.WriteLine("completed {0:c}", DateTime.Now); - tManager.Add(myTask); - - //wait for ITasks to complete - waitHandle.WaitOne(); - - NUnit.Framework.Assert.IsTrue(myTask.Ended != default(DateTime)); + Assert.IsFalse(runner.IsRunning); + runner.Add(new MyTask(5000)); + Assert.IsTrue(runner.IsRunning); // is running the task + runner.Shutdown(false, false); // -force -wait + Assert.IsTrue(runner.IsCompleted); + Assert.IsTrue(runner.IsRunning); // still running that task + Thread.Sleep(3000); + Assert.IsTrue(runner.IsRunning); // still running that task + AssertRunnerStopsRunning(runner, 10000); } } [Test] - public void Many_Tasks_Run() + public void ShutdownFlushesTheQueue() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + Assert.IsFalse(runner.IsRunning); + 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.Shutdown(false, false); // -force -wait + AssertRunnerStopsRunning(runner, 10000); + Assert.AreNotEqual(DateTime.MinValue, t.Ended); // t has run + } + } + + [Test] + public void ShutdownForceTruncatesTheQueue() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + Assert.IsFalse(runner.IsRunning); + 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.Shutdown(true, false); // +force -wait + AssertRunnerStopsRunning(runner, 10000); + Assert.AreEqual(DateTime.MinValue, t.Ended); // t has not run + } + } + + [Test] + public void ShutdownThenForce() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + Assert.IsFalse(runner.IsRunning); + runner.Add(new MyTask(5000)); + runner.Add(new MyTask()); + runner.Add(new MyTask()); + Assert.IsTrue(runner.IsRunning); // is running the task + runner.Shutdown(false, false); // -force -wait + Assert.IsTrue(runner.IsCompleted); + Assert.IsTrue(runner.IsRunning); // still running that task + Thread.Sleep(3000); + Assert.IsTrue(runner.IsRunning); // still running that task + runner.Shutdown(true, false); // +force -wait + AssertRunnerStopsRunning(runner, 20000); + } + } + + [Test] + public void Create_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + Assert.IsFalse(runner.IsRunning); + } + } + + [Test] + public void Create_AutoStart_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true })) + { + Assert.IsTrue(runner.IsRunning); + AssertRunnerStopsRunning(runner); // though not for long + } + } + + [Test] + public void Create_AutoStartAndKeepAlive_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true })) + { + Assert.IsTrue(runner.IsRunning); + Thread.Sleep(800); // for long + Assert.IsTrue(runner.IsRunning); + // dispose will stop it + } + } + + [Test] + public void Dispose_IsRunning() + { + BackgroundTaskRunner runner; + using (runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { AutoStart = true, KeepAlive = true })) + { + Assert.IsTrue(runner.IsRunning); + // dispose will stop it + } + + AssertRunnerStopsRunning(runner); + Assert.Throws(() => runner.Add(new MyTask())); + } + + [Test] + public void Startup_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + Assert.IsFalse(runner.IsRunning); + runner.StartUp(); + Assert.IsTrue(runner.IsRunning); + AssertRunnerStopsRunning(runner); // though not for long + } + } + + [Test] + public void Startup_KeepAlive_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = true })) + { + Assert.IsFalse(runner.IsRunning); + runner.StartUp(); + Assert.IsTrue(runner.IsRunning); + // dispose will stop it + } + } + + [Test] + public void Create_AddTask_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + runner.Add(new MyTask()); + Assert.IsTrue(runner.IsRunning); + Thread.Sleep(800); // task takes 500ms + Assert.IsFalse(runner.IsRunning); + } + } + + [Test] + public void Create_KeepAliveAndAddTask_IsRunning() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = true })) + { + runner.Add(new MyTask()); + Assert.IsTrue(runner.IsRunning); + Thread.Sleep(800); // task takes 500ms + Assert.IsTrue(runner.IsRunning); + // dispose will stop it + } + } + + [Test] + public async void WaitOnRunner_OneTask() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var task = new MyTask(); + Assert.IsTrue(task.Ended == default(DateTime)); + runner.Add(task); + await runner; // wait 'til it's not running anymore + Assert.IsTrue(task.Ended != default(DateTime)); // task is done + AssertRunnerStopsRunning(runner); // though not for long + } + } + + [Test] + public async void WaitOnRunner_Tasks() + { + var tasks = new List(); + for (var i = 0; i < 10; i++) + tasks.Add(new MyTask()); + + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { KeepAlive = false, LongRunning = true, PreserveRunningTask = true })) + { + tasks.ForEach(runner.Add); + + await runner; // wait 'til it's not running anymore + + // check that tasks are done + Assert.IsTrue(tasks.All(x => x.Ended != default(DateTime))); + + Assert.AreEqual(TaskStatus.RanToCompletion, runner.TaskStatus); + Assert.IsFalse(runner.IsRunning); + Assert.IsFalse(runner.IsDisposed); + } + } + + [Test] + public void WaitOnTask() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var task = new MyTask(); + var waitHandle = new ManualResetEvent(false); + runner.TaskCompleted += (sender, t) => waitHandle.Set(); + Assert.IsTrue(task.Ended == default(DateTime)); + runner.Add(task); + waitHandle.WaitOne(); // wait 'til task is done + Assert.IsTrue(task.Ended != default(DateTime)); // task is done + AssertRunnerStopsRunning(runner); // though not for long + } + } + + [Test] + public void WaitOnTasks() { var tasks = new Dictionary(); for (var i = 0; i < 10; i++) - { tasks.Add(new MyTask(), new ManualResetEvent(false)); - } - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(true, true)) + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { - tManager.TaskCompleted += (sender, task) => tasks[task.Task].Set(); + runner.TaskCompleted += (sender, task) => tasks[task.Task].Set(); + foreach (var t in tasks) runner.Add(t.Key); - tasks.ForEach(t => tManager.Add(t.Key)); - - //wait for all ITasks to complete + // wait 'til tasks are done, check that tasks are done WaitHandle.WaitAll(tasks.Values.Select(x => (WaitHandle)x).ToArray()); + Assert.IsTrue(tasks.All(x => x.Key.Ended != default(DateTime))); - foreach (var task in tasks) - { - NUnit.Framework.Assert.IsTrue(task.Key.Ended != default(DateTime)); - } - } - } - - [Test] - public async void Many_Tasks_Added_Only_Last_Task_Executes_With_Option() - { - var tasks = new Dictionary(); - for (var i = 0; i < 10; i++) - { - tasks.Add(new MyTask(), new ManualResetEvent(false)); - } - - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions{OnlyProcessLastItem = true})) - { - - tasks.ForEach(t => tManager.Add(t.Key)); - - //wait till the thread is done - await tManager; - - var countExecuted = tasks.Count(x => x.Key.Ended != default(DateTime)); - - Assert.AreEqual(1, countExecuted); + AssertRunnerStopsRunning(runner); // though not for long } } @@ -123,7 +298,7 @@ namespace Umbraco.Tests.Scheduling IDictionary tasks = getTasks(); BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(true, true)) + using (tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true, KeepAlive = true })) { tManager.TaskCompleted += (sender, task) => tasks[task.Task].Set(); @@ -135,7 +310,7 @@ namespace Umbraco.Tests.Scheduling foreach (var task in tasks) { - NUnit.Framework.Assert.IsTrue(task.Key.Ended != default(DateTime)); + Assert.IsTrue(task.Key.Ended != default(DateTime)); } //execute another batch after a bit @@ -149,71 +324,11 @@ namespace Umbraco.Tests.Scheduling foreach (var task in tasks) { - NUnit.Framework.Assert.IsTrue(task.Key.Ended != default(DateTime)); + Assert.IsTrue(task.Key.Ended != default(DateTime)); } } } - [Test] - public void Task_Queue_Will_Be_Completed_Before_Shutdown() - { - var tasks = new Dictionary(); - for (var i = 0; i < 10; i++) - { - tasks.Add(new MyTask(), new ManualResetEvent(false)); - } - - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(true, true)) - { - tManager.TaskCompleted += (sender, task) => tasks[task.Task].Set(); - - tasks.ForEach(t => tManager.Add(t.Key)); - - ////wait for all ITasks to complete - //WaitHandle.WaitAll(tasks.Values.Select(x => (WaitHandle)x).ToArray()); - - tManager.Stop(false); - //immediate stop will block until complete - but since we are running on - // a single thread this doesn't really matter as the above will just process - // until complete. - tManager.Stop(true); - - NUnit.Framework.Assert.AreEqual(0, tManager.TaskCount); - } - } - - //NOTE: These tests work in .Net 4.5 but in this current version we don't have the correct - // async/await signatures with GetAwaiter, so am just commenting these out in this version - - [Test] - public async void Non_Persistent_Runner_Will_End_After_Queue_Empty() - { - var tasks = new List(); - for (var i = 0; i < 10; i++) - { - tasks.Add(new MyTask()); - } - - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(persistentThread: false, dedicatedThread:true)) - { - tasks.ForEach(t => tManager.Add(t)); - - //wait till the thread is done - await tManager; - - foreach (var task in tasks) - { - Assert.IsTrue(task.Ended != default(DateTime)); - } - - Assert.AreEqual(TaskStatus.RanToCompletion, tManager.TaskStatus); - Assert.IsFalse(tManager.IsRunning); - Assert.IsFalse(tManager.IsDisposed); - } - } - [Test] public async void Non_Persistent_Runner_Will_Start_New_Threads_When_Required() { @@ -229,10 +344,9 @@ namespace Umbraco.Tests.Scheduling List tasks = getTasks(); - BackgroundTaskRunner tManager; - using (tManager = new BackgroundTaskRunner(persistentThread: false, dedicatedThread: true)) + using (var tManager = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true, PreserveRunningTask = true })) { - tasks.ForEach(t => tManager.Add(t)); + tasks.ForEach(tManager.Add); //wait till the thread is done await tManager; @@ -250,7 +364,7 @@ namespace Umbraco.Tests.Scheduling tasks = getTasks(); //add more tasks - tasks.ForEach(t => tManager.Add(t)); + tasks.ForEach(tManager.Add); //wait till the thread is done await tManager; @@ -265,19 +379,296 @@ namespace Umbraco.Tests.Scheduling Assert.IsFalse(tManager.IsDisposed); } } - - private class MyTask : BaseTask + [Test] + public void RecurringTaskTest() { - public MyTask() + // note: can have BackgroundTaskRunner and use it in MyRecurringTask ctor + // because that ctor wants IBackgroundTaskRunner and the generic type + // parameter is contravariant ie defined as IBackgroundTaskRunner so doing the + // following is legal: + // var IBackgroundTaskRunner b = ...; + // var IBackgroundTaskRunner d = b; // legal + + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) { + var task = new MyRecurringTask(runner, 200, 500); + MyRecurringTask.RunCount = 0; + runner.Add(task); + Thread.Sleep(5000); + Assert.GreaterOrEqual(MyRecurringTask.RunCount, 2); // keeps running, count >= 2 + + // stops recurring + runner.Shutdown(false, false); + AssertRunnerStopsRunning(runner); + + // timer may try to add a task but it won't work because runner is completed + } + } + + [Test] + public void DelayedTaskRuns() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var task = new MyDelayedTask(200); + 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); + task.Release(); + Thread.Sleep(500); + Assert.IsTrue(task.HasRun); + AssertRunnerStopsRunning(runner); // runs task & exit + } + } + + [Test] + public void DelayedTaskStops() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var task = new MyDelayedTask(200); + 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); + AssertRunnerStopsRunning(runner); // runs task & exit + Assert.IsTrue(task.HasRun); + } + } + + [Test] + public void DelayedRecurring() + { + using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions())) + { + var task = new MyDelayedRecurringTask(runner, 2000, 1000); + MyDelayedRecurringTask.RunCount = 0; + runner.Add(task); + Thread.Sleep(1000); + Assert.IsTrue(runner.IsRunning); // waiting on delay + Assert.AreEqual(0, MyDelayedRecurringTask.RunCount); + Thread.Sleep(1000); + Assert.AreEqual(1, MyDelayedRecurringTask.RunCount); + Thread.Sleep(5000); + Assert.GreaterOrEqual(MyDelayedRecurringTask.RunCount, 2); // keeps running, count >= 2 + + // stops recurring + runner.Shutdown(false, false); + AssertRunnerStopsRunning(runner); + + // timer may try to add a task but it won't work because runner is completed + } + } + + [Test] + public void FailingTaskSync() + { + 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 + runner.Add(task); + Assert.IsTrue(runner.IsRunning); + AssertRunnerStopsRunning(runner); // runs task & exit + + Assert.AreEqual(1, exceptions.Count); // traced and reported + } + } + + [Test] + public void FailingTaskAsync() + { + 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 + runner.Add(task); + Assert.IsTrue(runner.IsRunning); + AssertRunnerStopsRunning(runner); // runs task & exit + + Assert.AreEqual(1, exceptions.Count); // traced and reported + } + } + + private class MyFailingTask : IBackgroundTask + { + private readonly bool _isAsync; + + public MyFailingTask(bool isAsync) + { + _isAsync = isAsync; + } + + public void Run() + { + Thread.Sleep(1000); + throw new Exception("Task has thrown."); + } + + public async Task RunAsync() + { + await Task.Delay(1000); + throw new Exception("Task has thrown."); + } + + public bool IsAsync + { + get { return _isAsync; } + } + + // fixme - must also test what happens if we throw on dispose! + public void Dispose() + { } + } + + private class MyDelayedRecurringTask : DelayedRecurringTaskBase + { + public MyDelayedRecurringTask(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) + : base(runner, delayMilliseconds, periodMilliseconds) + { } + + private MyDelayedRecurringTask(MyDelayedRecurringTask source) + : base(source) + { } + + public static int RunCount { get; set; } + + public override bool IsAsync + { + get { return false; } } public override void PerformRun() { - Thread.Sleep(500); + // nothing to do at the moment + RunCount += 1; } + public override Task PerformRunAsync() + { + throw new NotImplementedException(); + } + + protected override MyDelayedRecurringTask GetRecurring() + { + return new MyDelayedRecurringTask(this); + } + } + + private class MyDelayedTask : IDelayedBackgroundTask + { + private readonly int _runMilliseconds; + private readonly ManualResetEvent _gate; + + public bool HasRun { get; private set; } + + public MyDelayedTask(int runMilliseconds) + { + _runMilliseconds = runMilliseconds; + _gate = new ManualResetEvent(false); + } + + public WaitHandle DelayWaitHandle + { + get { return _gate; } + } + + public bool IsDelayed + { + get { return true; } + } + + public void Run() + { + Thread.Sleep(_runMilliseconds); + HasRun = true; + } + + public void Release() + { + _gate.Set(); + } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } + + public void Dispose() + { } + } + + private class MyRecurringTask : RecurringTaskBase + { + private readonly int _runMilliseconds; + + public static int RunCount { get; set; } + + public MyRecurringTask(IBackgroundTaskRunner runner, int runMilliseconds, int periodMilliseconds) + : base(runner, periodMilliseconds) + { + _runMilliseconds = runMilliseconds; + } + + private MyRecurringTask(MyRecurringTask source, int runMilliseconds) + : base(source) + { + _runMilliseconds = runMilliseconds; + } + + public override void PerformRun() + { + RunCount += 1; + Thread.Sleep(_runMilliseconds); + } + + public override Task PerformRunAsync() + { + throw new NotImplementedException(); + } + + public override bool IsAsync + { + get { return false; } + } + + protected override MyRecurringTask GetRecurring() + { + return new MyRecurringTask(this, _runMilliseconds); + } + } + + private class MyTask : BaseTask + { + private readonly int _milliseconds; + + public MyTask() + : this(500) + { } + + public MyTask(int milliseconds) + { + _milliseconds = milliseconds; + } + + public override void PerformRun() + { + Thread.Sleep(_milliseconds); + } } public abstract class BaseTask : IBackgroundTask @@ -287,14 +678,17 @@ namespace Umbraco.Tests.Scheduling public Guid UniqueId { get; protected set; } public abstract void PerformRun(); + public void Run() { PerformRun(); Ended = DateTime.Now; } + public Task RunAsync() { throw new NotImplementedException(); + //return Task.Delay(500); // fixme } public bool IsAsync @@ -312,10 +706,7 @@ namespace Umbraco.Tests.Scheduling public DateTime Ended { get; set; } public virtual void Dispose() - { - - } + { } } - } } diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index ffba7a1e0a..c511a0af53 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -9,60 +10,97 @@ using Umbraco.Core.Logging; namespace Umbraco.Web.Scheduling { /// - /// This is used to create a background task runner which will stay alive in the background of and complete - /// any tasks that are queued. It is web aware and will ensure that it is shutdown correctly when the app domain - /// is shutdown. + /// Manages a queue of tasks of type and runs them in the background. /// - /// - internal class BackgroundTaskRunner : IDisposable, IRegisteredObject - where T : IBackgroundTask + /// The type of the managed tasks. + /// The task runner is web-aware and will ensure that it shuts down correctly when the AppDomain + /// shuts down (ie is unloaded). FIXME WHAT DOES THAT MEAN? + internal class BackgroundTaskRunner : IBackgroundTaskRunner + where T : class, IBackgroundTask { private readonly BackgroundTaskRunnerOptions _options; private readonly BlockingCollection _tasks = new BlockingCollection(); - private Task _consumer; + private readonly object _locker = new object(); + private readonly ManualResetEvent _completedEvent = new ManualResetEvent(false); + + private volatile bool _isRunning; // is running + private volatile bool _isCompleted; // does not accept tasks anymore, may still be running + private Task _runningTask; - private volatile bool _isRunning = false; - private static readonly object Locker = new object(); private CancellationTokenSource _tokenSource; + internal event EventHandler> TaskError; internal event EventHandler> TaskStarting; internal event EventHandler> TaskCompleted; internal event EventHandler> TaskCancelled; - public BackgroundTaskRunner(bool dedicatedThread = false, bool persistentThread = false) - : this(new BackgroundTaskRunnerOptions{DedicatedThread = dedicatedThread, PersistentThread = persistentThread}) - { - } + /// + /// Initializes a new instance of the class. + /// + public BackgroundTaskRunner() + : this(new BackgroundTaskRunnerOptions()) + { } + /// + /// Initializes a new instance of the class with a set of options. + /// + /// The set of options. public BackgroundTaskRunner(BackgroundTaskRunnerOptions options) { if (options == null) throw new ArgumentNullException("options"); _options = options; HostingEnvironment.RegisterObject(this); + + if (options.AutoStart) + StartUp(); } + /// + /// Gets the number of tasks in the queue. + /// public int TaskCount { get { return _tasks.Count; } } + /// + /// Gets a value indicating whether a task is currently running. + /// public bool IsRunning { get { return _isRunning; } } - public TaskStatus TaskStatus + /// + /// Gets a value indicating whether the runner has completed and cannot accept tasks anymore. + /// + public bool IsCompleted { - get { return _consumer.Status; } + get { return _isCompleted; } } + /// + /// 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 TaskStatus + { + get + { + if (_runningTask == null) + throw new InvalidOperationException("There is no current task."); + return _runningTask.Status; + } + } /// - /// Returns the task awaiter so that consumers of the BackgroundTaskManager can await - /// the threading operation. + /// Gets an awaiter used to await the running task. /// - /// + /// An awaiter for the running task. /// /// This is just the coolest thing ever, check this article out: /// http://blogs.msdn.com/b/pfxteam/archive/2011/01/13/10115642.aspx @@ -70,267 +108,287 @@ namespace Umbraco.Web.Scheduling /// So long as we have a method called GetAwaiter() that returns an instance of INotifyCompletion /// we can await anything! :) /// + /// 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 TaskAwaiter GetAwaiter() { - return _consumer.GetAwaiter(); + if (_runningTask == null) + throw new InvalidOperationException("There is no current task."); + return _runningTask.GetAwaiter(); } + /// + /// Adds a task to the queue. + /// + /// The task to add. + /// The task runner has completed. public void Add(T task) { - //add any tasks first - LogHelper.Debug>(" Task added {0}", () => task.GetType()); - _tasks.Add(task); + lock (_locker) + { + if (_isCompleted) + throw new InvalidOperationException("The task runner has completed."); - //ensure's everything is started - StartUp(); + // add task + LogHelper.Debug>("Task added {0}", task.GetType); + _tasks.Add(task); + + // start + StartUpLocked(); + } } + /// + /// Tries to add a task to the queue. + /// + /// The task to add. + /// true if the task could be added to the queue; otherwise false. + /// Returns false if the runner is completed. + public bool TryAdd(T task) + { + lock (_locker) + { + if (_isCompleted) return false; + + // add task + LogHelper.Debug>("Task added {0}", task.GetType); + _tasks.Add(task); + + // start + StartUpLocked(); + + return true; + } + } + + /// + /// Starts the tasks runner, if not already running. + /// + /// Is invoked each time a task is added, to ensure it is going to be processed. + /// The task runner has completed. public void StartUp() { - if (!_isRunning) + if (_isRunning) return; + + lock (_locker) { - lock (Locker) - { - //double check - if (!_isRunning) - { - _isRunning = true; - //Create a new token source since this is a new proces - _tokenSource = new CancellationTokenSource(); - StartConsumer(); - LogHelper.Debug>("Starting"); - } - } - } - } + if (_isCompleted) + throw new InvalidOperationException("The task runner has completed."); - public void ShutDown() - { - lock (Locker) - { - _isRunning = false; - - try - { - if (_consumer != null) - { - //cancel all operations - _tokenSource.Cancel(); - - try - { - _consumer.Wait(); - } - catch (AggregateException e) - { - //NOTE: We are logging Debug because we are expecting these errors - - LogHelper.Debug>("AggregateException thrown with the following inner exceptions:"); - // Display information about each exception. - foreach (var v in e.InnerExceptions) - { - var exception = v as TaskCanceledException; - if (exception != null) - { - LogHelper.Debug>(" .Net TaskCanceledException: .Net Task ID {0}", () => exception.Task.Id); - } - else - { - LogHelper.Debug>(" Exception: {0}", () => v.GetType().Name); - } - } - } - } - - if (_tasks.Count > 0) - { - LogHelper.Debug>("Processing remaining tasks before shutdown: {0}", () => _tasks.Count); - - //now we need to ensure the remaining queue is processed if there's any remaining, - // this will all be processed on the current/main thread. - T remainingTask; - while (_tasks.TryTake(out remainingTask)) - { - //skip if this is not the last - if (_options.OnlyProcessLastItem && _tasks.Count > 0) - { - //NOTE: don't raise canceled event, we're shutting down, just dispose - remainingTask.Dispose(); - continue; - } - - ConsumeTaskInternalAsync(remainingTask) - .Wait(); //block until it completes - } - } - - LogHelper.Debug>("Shutdown"); - - //disposing these is really optional since they'll be disposed immediately since they are no longer running - //but we'll put this here anyways. - if (_consumer != null && (_consumer.IsCompleted || _consumer.IsCanceled)) - { - _consumer.Dispose(); - } - } - catch (Exception ex) - { - LogHelper.Error>("Error occurred shutting down task runner", ex); - } - finally - { - HostingEnvironment.UnregisterObject(this); - } + StartUpLocked(); } } /// - /// Starts the consumer task + /// Starts the tasks runner, if not already running. /// - private void StartConsumer() + /// Must be invoked within lock(_locker) and with _isCompleted being false. + private void StartUpLocked() { - var token = _tokenSource.Token; - - _consumer = Task.Factory.StartNew(() => - StartThreadAsync(token), - token, - _options.DedicatedThread ? TaskCreationOptions.LongRunning : TaskCreationOptions.None, - TaskScheduler.Default); - - //if this is not a persistent thread, wait till it's done and shut ourselves down - // thus ending the thread or giving back to the thread pool. If another task is added - // another thread will spawn or be taken from the pool to process. - if (!_options.PersistentThread) - { - _consumer.ContinueWith(task => ShutDown()); - } + // double check + if (_isRunning) return; + _isRunning = true; + // create a new token source since this is a new process + _tokenSource = new CancellationTokenSource(); + _runningTask = PumpIBackgroundTasks(Task.Factory, _tokenSource.Token); + LogHelper.Debug>("Starting"); } /// - /// Invokes a new worker thread to consume tasks + /// Shuts the taks runner down. /// - /// - private async Task StartThreadAsync(CancellationToken token) + /// True for force the runner to stop. + /// True to wait until the runner has stopped. + /// If is false, no more tasks can be queued but all queued tasks + /// will run. If it is true, then only the current one (if any) will end and no other task will run. + public void Shutdown(bool force, bool wait) { - // Was cancellation already requested? - if (token.IsCancellationRequested) + lock (_locker) { - LogHelper.Info>("Thread {0} was cancelled before it got started.", () => Thread.CurrentThread.ManagedThreadId); - token.ThrowIfCancellationRequested(); + _isCompleted = true; // do not accept new tasks + if (_isRunning == false) return; // done already } - await TakeAndConsumeTaskAsync(token); - } + // try to be nice + // assuming multiple threads can do these without problems + _completedEvent.Set(); + _tasks.CompleteAdding(); - /// - /// Trys to get a task from the queue, if there isn't one it will wait a second and try again - /// - /// - private async Task TakeAndConsumeTaskAsync(CancellationToken token) - { - if (token.IsCancellationRequested) + if (force) { - LogHelper.Info>("Thread {0} was cancelled.", () => Thread.CurrentThread.ManagedThreadId); - token.ThrowIfCancellationRequested(); - } - - //If this is true, the thread will stay alive and just wait until there is anything in the queue - // and process it. When there is nothing in the queue, the thread will just block until there is - // something to process. - //When this is false, the thread will process what is currently in the queue and once that is - // done, the thread will end and we will shutdown the process - - if (_options.PersistentThread) - { - //This will iterate over the collection, if there is nothing to take - // the thread will block until there is something available. - //We need to pass our cancellation token so that the thread will - // cancel when we shutdown - foreach (var t in _tasks.GetConsumingEnumerable(token)) + // we must bring everything down, now + Thread.Sleep(100); // give time to CompleAdding() + lock (_locker) { - //skip if this is not the last - if (_options.OnlyProcessLastItem && _tasks.Count > 0) - { - OnTaskCancelled(new TaskEventArgs(t)); - continue; - } - - await ConsumeTaskCancellableAsync(t, token); + // was CompleteAdding() enough? + if (_isRunning == false) return; } - - //recurse and keep going - await TakeAndConsumeTaskAsync(token); + // try to cancel running async tasks (cannot do much about sync tasks) + // break delayed tasks delay + // truncate running queues + _tokenSource.Cancel(false); // false is the default } else { - T t; - while (_tasks.TryTake(out t)) - { - //skip if this is not the last - if (_options.OnlyProcessLastItem && _tasks.Count > 0) - { - OnTaskCancelled(new TaskEventArgs(t)); - continue; - } - - await ConsumeTaskCancellableAsync(t, token); - } - - //the task will end here + // tasks in the queue will be executed... + if (wait == false) return; + _runningTask.Wait(); // wait for whatever is running to end... } } - internal async Task ConsumeTaskCancellableAsync(T task, CancellationToken token) + /// + /// Runs background tasks for as long as there are background tasks in the queue, with an asynchronous operation. + /// + /// The supporting . + /// A cancellation token. + /// The asynchronous operation. + private Task PumpIBackgroundTasks(TaskFactory factory, CancellationToken token) + { + var taskSource = new TaskCompletionSource(factory.CreationOptions); + var enumerator = _options.KeepAlive ? _tasks.GetConsumingEnumerable(token).GetEnumerator() : null; + + // ReSharper disable once MethodSupportsCancellation // always run + var taskSourceContinuing = taskSource.Task.ContinueWith(t => + { + // 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 + lock (_locker) + { + if (token.IsCancellationRequested || _tasks.Count == 0) + { + _isRunning = false; // done + if (_options.PreserveRunningTask == false) + _runningTask = null; + return; + } + } + + // if _runningTask is taskSource.Task then we must keep continuing it, + // not starting a new taskSource, else _runningTask would complete and + // something may be waiting on it + //PumpIBackgroundTasks(factory, token); // restart + // ReSharper disable once MethodSupportsCancellation // always run + t.ContinueWithTask(_ => PumpIBackgroundTasks(factory, token)); // restart + }); + + Action pump = null; + pump = task => + { + // RunIBackgroundTaskAsync does NOT throw exceptions, just raises event + // so if we have an exception here, really, wtf? - must read the exception + // anyways so it does not bubble up and kill everything + if (task != null && task.IsFaulted) + { + var exception = task.Exception; + LogHelper.Error>("Task runner exception.", exception); + } + + // is it ok to run? + if (TaskSourceCanceled(taskSource, token)) return; + + // try to get a task + // the blocking MoveNext will end if token is cancelled or collection is completed + T bgTask; + var hasBgTask = _options.KeepAlive + ? (bgTask = enumerator.MoveNext() ? enumerator.Current : null) != null // blocking + : _tasks.TryTake(out bgTask); // non-blocking + + // no task, signal the runner we're done + if (hasBgTask == false) + { + TaskSourceCompleted(taskSource, token); + return; + } + + // wait for delayed task, supporting cancellation + var dbgTask = bgTask as IDelayedBackgroundTask; + if (dbgTask != null && dbgTask.IsDelayed) + { + WaitHandle.WaitAny(new[] { dbgTask.DelayWaitHandle, token.WaitHandle, _completedEvent }); + if (TaskSourceCanceled(taskSource, token)) return; + // else run now, either because delay is ok or runner is completed + } + + // run the task as first task, or a continuation + task = task == null + ? RunIBackgroundTaskAsync(bgTask, token) + // ReSharper disable once MethodSupportsCancellation // always run + : task.ContinueWithTask(_ => RunIBackgroundTaskAsync(bgTask, token)); + + // and pump + // ReSharper disable once MethodSupportsCancellation // always run + task.ContinueWith(t => pump(t)); + }; + + // start it all + factory.StartNew(() => pump(null), + token, + _options.LongRunning ? TaskCreationOptions.LongRunning : TaskCreationOptions.None, + TaskScheduler.Default); + + return taskSourceContinuing; + } + + private bool TaskSourceCanceled(TaskCompletionSource taskSource, CancellationToken token) { if (token.IsCancellationRequested) { - OnTaskCancelled(new TaskEventArgs(task)); - - //NOTE: Since the task hasn't started this is pretty pointless so leaving it out. - LogHelper.Info>("Task {0}) was cancelled.", - () => task.GetType()); - - token.ThrowIfCancellationRequested(); + taskSource.SetCanceled(); + return true; } - - await ConsumeTaskInternalAsync(task); + return false; } - private async Task ConsumeTaskInternalAsync(T task) + private void TaskSourceCompleted(TaskCompletionSource taskSource, CancellationToken token) + { + if (token.IsCancellationRequested) + taskSource.SetCanceled(); + else + taskSource.SetResult(null); + } + + /// + /// Runs a background task asynchronously. + /// + /// The background task. + /// A cancellation token. + /// The asynchronous operation. + internal async Task RunIBackgroundTaskAsync(T bgTask, CancellationToken token) { try { - OnTaskStarting(new TaskEventArgs(task)); + OnTaskStarting(new TaskEventArgs(bgTask)); try { - using (task) + using (bgTask) // ensure it's disposed { - if (task.IsAsync) - { - await task.RunAsync(); - } + if (bgTask.IsAsync) + await bgTask.RunAsync(); // fixme should pass the token along?! else - { - task.Run(); - } + bgTask.Run(); } } catch (Exception e) { - OnTaskError(new TaskEventArgs(task, e)); + OnTaskError(new TaskEventArgs(bgTask, e)); throw; } - - OnTaskCompleted(new TaskEventArgs(task)); + Console.WriteLine("!1"); + OnTaskCompleted(new TaskEventArgs(bgTask)); } catch (Exception ex) { - LogHelper.Error>("An error occurred consuming task", ex); + LogHelper.Error>("Task has failed.", ex); } } + #region Events + protected virtual void OnTaskError(TaskEventArgs e) { var handler = TaskError; @@ -358,8 +416,10 @@ namespace Umbraco.Web.Scheduling e.Task.Dispose(); } + #endregion + + #region IDisposable - #region Disposal private readonly object _disposalLocker = new object(); public bool IsDisposed { get; private set; } @@ -376,8 +436,9 @@ namespace Umbraco.Web.Scheduling protected virtual void Dispose(bool disposing) { - if (this.IsDisposed || !disposing) + if (this.IsDisposed || disposing == false) return; + lock (_disposalLocker) { if (IsDisposed) @@ -389,31 +450,60 @@ namespace Umbraco.Web.Scheduling protected virtual void DisposeResources() { - ShutDown(); + // just make sure we eventually go down + Shutdown(true, false); } + #endregion + /// + /// Requests a registered object to unregister. + /// + /// true to indicate the registered object should unregister 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 unregister. During + /// processing of the Stop method, the registered object must call the HostingEnvironment.UnregisterObject method. + /// public void Stop(bool immediate) { if (immediate == false) { - LogHelper.Debug>("Application is shutting down, waiting for tasks to complete"); - Dispose(); + // 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. + + LogHelper.Debug>("Shutting down, waiting for tasks to complete."); + Shutdown(false, false); // do not accept any more tasks, flush the queue, do not wait + + lock (_locker) + { + if (_runningTask != null) + _runningTask.ContinueWith(_ => + { + HostingEnvironment.UnregisterObject(this); + LogHelper.Info>("Down, tasks completed."); + }); + else + { + HostingEnvironment.UnregisterObject(this); + LogHelper.Info>("Down, tasks completed."); + } + } } else { - //NOTE: this will thread block the current operation if the manager - // is still shutting down because the Shutdown operation is also locked - // by this same lock instance. This would only matter if Stop is called by ASP.Net - // on two different threads though, otherwise the current thread will just block normally - // until the app is shutdown - lock (Locker) - { - LogHelper.Info>("Application is shutting down immediately"); - } + // 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. + + LogHelper.Info>("Shutting down immediately."); + Shutdown(true, true); // cancel all tasks, wait for the current one to end + HostingEnvironment.UnregisterObject(this); + LogHelper.Info>("Down."); } - } - } } diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs index c42fcd681a..4688ff37d6 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunnerOptions.cs @@ -1,23 +1,44 @@ namespace Umbraco.Web.Scheduling { + /// + /// Provides options to the class. + /// internal class BackgroundTaskRunnerOptions { //TODO: Could add options for using a stack vs queue if required + /// + /// Initializes a new instance of the class. + /// public BackgroundTaskRunnerOptions() { - DedicatedThread = false; - PersistentThread = false; - OnlyProcessLastItem = false; + LongRunning = false; + KeepAlive = false; + AutoStart = false; } - public bool DedicatedThread { get; set; } - public bool PersistentThread { get; set; } + /// + /// Gets or sets a value indicating whether the running task should be a long-running, + /// coarse grained operation. + /// + public bool LongRunning { get; set; } /// - /// If this is true, the task runner will skip over all items and only process the last/final - /// item registered + /// Gets or sets a value indicating whether the running task should block and wait + /// on the queue, or end, when the queue is empty. /// - public bool OnlyProcessLastItem { get; set; } + public bool KeepAlive { get; set; } + + /// + /// Gets or sets a value indicating whether the running task should start immediately + /// or only once a task has been added to the queue. + /// + public bool AutoStart { get; set; } + + /// + /// Gets or setes a value indicating whether the running task should be preserved + /// once completed, or reset to null. For unit tests. + /// + public bool PreserveRunningTask { get; set; } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs new file mode 100644 index 0000000000..573adeda3d --- /dev/null +++ b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs @@ -0,0 +1,52 @@ +using System; +using System.Threading; + +namespace Umbraco.Web.Scheduling +{ + /// + /// Provides a base class for recurring background tasks. + /// + /// The type of the managed tasks. + internal abstract class DelayedRecurringTaskBase : RecurringTaskBase, IDelayedBackgroundTask + where T : class, IBackgroundTask + { + private readonly int _delayMilliseconds; + private ManualResetEvent _gate; + private Timer _timer; + + protected DelayedRecurringTaskBase(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) + : base(runner, periodMilliseconds) + { + _delayMilliseconds = delayMilliseconds; + } + + protected DelayedRecurringTaskBase(DelayedRecurringTaskBase source) + : base(source) + { + _delayMilliseconds = 0; + } + + public WaitHandle DelayWaitHandle + { + get + { + if (_delayMilliseconds == 0) return new ManualResetEvent(true); + + if (_gate != null) return _gate; + _gate = new ManualResetEvent(false); + _timer = new Timer(_ => + { + _timer.Dispose(); + _timer = null; + _gate.Set(); + }, null, _delayMilliseconds, 0); + return _gate; + } + } + + public bool IsDelayed + { + get { return _delayMilliseconds > 0; } + } + } +} diff --git a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs index 48522aeb5f..9be2512d01 100644 --- a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs +++ b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs @@ -3,10 +3,26 @@ using System.Threading.Tasks; namespace Umbraco.Web.Scheduling { + /// + /// Represents a background task. + /// internal interface IBackgroundTask : IDisposable { + /// + /// Runs the background task. + /// void Run(); + + /// + /// Runs the task asynchronously. + /// + /// A instance representing the execution of the background task. + /// The background task cannot run asynchronously. Task RunAsync(); + + /// + /// Indicates whether the background task can run asynchronously. + /// bool IsAsync { get; } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/IBackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/IBackgroundTaskRunner.cs new file mode 100644 index 0000000000..c4e2dab35d --- /dev/null +++ b/src/Umbraco.Web/Scheduling/IBackgroundTaskRunner.cs @@ -0,0 +1,20 @@ +using System; +using System.Web.Hosting; + +namespace Umbraco.Web.Scheduling +{ + /// + /// Defines a service managing a queue of tasks of type and running them in the background. + /// + /// The type of the managed tasks. + /// The interface is not complete and exists only to have the contravariance on T. + internal interface IBackgroundTaskRunner : IDisposable, IRegisteredObject + where T : class, IBackgroundTask + { + bool IsCompleted { get; } + void Add(T task); + bool TryAdd(T task); + + // fixme - complete the interface? + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs b/src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs new file mode 100644 index 0000000000..01f8a5e01a --- /dev/null +++ b/src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs @@ -0,0 +1,23 @@ +using System.Threading; + +namespace Umbraco.Web.Scheduling +{ + /// + /// Represents a delayed background task. + /// + /// Delayed background tasks can suspend their execution until + /// a condition is met. However if the tasks runner has to terminate, + /// delayed background tasks are executed immediately. + internal interface IDelayedBackgroundTask : IBackgroundTask + { + /// + /// Gets a wait handle on the task condition. + /// + WaitHandle DelayWaitHandle { get; } + + /// + /// Gets a value indicating whether the task is delayed. + /// + bool IsDelayed { get; } + } +} diff --git a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs new file mode 100644 index 0000000000..91d86d97b4 --- /dev/null +++ b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs @@ -0,0 +1,108 @@ +using System.Threading; +using System.Threading.Tasks; + +namespace Umbraco.Web.Scheduling +{ + /// + /// Provides a base class for recurring background tasks. + /// + /// The type of the managed tasks. + internal abstract class RecurringTaskBase : IBackgroundTask + where T : class, IBackgroundTask + { + private readonly IBackgroundTaskRunner _runner; + private readonly int _periodMilliseconds; + private Timer _timer; + + /// + /// Initializes a new instance of the class with a tasks runner and a period. + /// + /// The task runner. + /// The period. + /// The task will repeat itself periodically. Use this constructor to create a new task. + protected RecurringTaskBase(IBackgroundTaskRunner runner, int periodMilliseconds) + { + _runner = runner; + _periodMilliseconds = periodMilliseconds; + } + + /// + /// Initializes a new instance of the class with a source task. + /// + /// The source task. + /// Use this constructor to create a new task from a source task in GetRecurring. + protected RecurringTaskBase(RecurringTaskBase source) + { + _runner = source._runner; + _periodMilliseconds = source._periodMilliseconds; + } + + /// + /// Implements IBackgroundTask.Run(). + /// + /// Classes inheriting from RecurringTaskBase must implement PerformRun. + public void Run() + { + PerformRun(); + Repeat(); + } + + /// + /// Implements IBackgroundTask.RunAsync(). + /// + /// Classes inheriting from RecurringTaskBase must implement PerformRun. + public async Task RunAsync() + { + await PerformRunAsync(); + Repeat(); + } + + private void Repeat() + { + // again? + if (_runner.IsCompleted) return; // fail fast + + if (_periodMilliseconds == 0) return; + + var recur = GetRecurring(); + if (recur == null) return; // done + + _timer = new Timer(_ => + { + _timer.Dispose(); + _timer = null; + _runner.TryAdd(recur); + }, null, _periodMilliseconds, 0); + } + + /// + /// Indicates whether the background task can run asynchronously. + /// + public abstract bool IsAsync { get; } + + /// + /// Runs the background task. + /// + public abstract void PerformRun(); + + /// + /// Runs the task asynchronously. + /// + /// A instance representing the execution of the background task. + public abstract Task PerformRunAsync(); + + /// + /// Gets a new occurence of the recurring task. + /// + /// A new task instance to be queued, or null to terminate the recurring task. + /// The new task instance must be created via the RecurringTaskBase(RecurringTaskBase{T} source) constructor, + /// where source is the current task, eg: return new MyTask(this); + protected abstract T GetRecurring(); + + /// + /// Dispose the task. + /// + public virtual void Dispose() + { } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs b/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs new file mode 100644 index 0000000000..a57ea904b0 --- /dev/null +++ b/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs @@ -0,0 +1,62 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Umbraco.Web.Scheduling +{ + internal static class TaskAndFactoryExtensions + { + #region Task Extensions + + static void SetCompletionSource(TaskCompletionSource completionSource, Task task) + { + if (task.IsFaulted) + completionSource.SetException(task.Exception.InnerException); + else + completionSource.SetResult(default(TResult)); + } + + static void SetCompletionSource(TaskCompletionSource completionSource, Task task) + { + if (task.IsFaulted) + completionSource.SetException(task.Exception.InnerException); + else + completionSource.SetResult(task.Result); + } + + public static Task ContinueWithTask(this Task task, Func continuation) + { + var completionSource = new TaskCompletionSource(); + task.ContinueWith(atask => continuation(atask).ContinueWith(atask2 => SetCompletionSource(completionSource, atask2))); + return completionSource.Task; + } + + public static Task ContinueWithTask(this Task task, Func continuation, CancellationToken token) + { + var completionSource = new TaskCompletionSource(); + task.ContinueWith(atask => continuation(atask).ContinueWith(atask2 => SetCompletionSource(completionSource, atask2), token), token); + return completionSource.Task; + } + + #endregion + + #region TaskFactory Extensions + + public static Task Completed(this TaskFactory factory) + { + var taskSource = new TaskCompletionSource(); + taskSource.SetResult(null); + return taskSource.Task; + } + + public static Task Sync(this TaskFactory factory, Action action) + { + var taskSource = new TaskCompletionSource(); + action(); + taskSource.SetResult(null); + return taskSource.Task; + } + + #endregion + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 23ba69cf31..58821bec6b 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -499,6 +499,11 @@ + + + + + diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index cd736b98ff..cbf296115a 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -33,7 +33,7 @@ namespace umbraco public class content { private static readonly BackgroundTaskRunner FilePersister - = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions {DedicatedThread = true, OnlyProcessLastItem = true}); + = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true }); #region Declarations From a73b7a584939785cb08efb28d4f7c2b2d16d0f3c Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 6 Feb 2015 18:10:19 +0100 Subject: [PATCH 08/13] refactor Scheduler to use new BackgroundTaskRunner capabilities Conflicts: src/Umbraco.Web/Scheduling/Scheduler.cs --- .../Scheduling/DelayedRecurringTaskBase.cs | 8 +- src/Umbraco.Web/Scheduling/LogScrubber.cs | 42 +++++-- .../Scheduling/RecurringTaskBase.cs | 7 +- .../Scheduling/ScheduledPublishing.cs | 33 ++++-- src/Umbraco.Web/Scheduling/ScheduledTasks.cs | 35 ++++-- src/Umbraco.Web/Scheduling/Scheduler.cs | 106 ++++-------------- 6 files changed, 113 insertions(+), 118 deletions(-) diff --git a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs index 573adeda3d..cac68241f4 100644 --- a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs @@ -34,12 +34,18 @@ namespace Umbraco.Web.Scheduling if (_gate != null) return _gate; _gate = new ManualResetEvent(false); + + // note + // must use the single-parameter constructor on Timer to avoid it from being GC'd + // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer + _timer = new Timer(_ => { _timer.Dispose(); _timer = null; _gate.Set(); - }, null, _delayMilliseconds, 0); + }); + _timer.Change(_delayMilliseconds, 0); return _gate; } } diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index 2edbe80726..a0c2c6979e 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -9,17 +9,31 @@ using Umbraco.Core.Logging; namespace Umbraco.Web.Scheduling { - internal class LogScrubber : DisposableObject, IBackgroundTask + internal class LogScrubber : DelayedRecurringTaskBase { private readonly ApplicationContext _appContext; private readonly IUmbracoSettingsSection _settings; - public LogScrubber(ApplicationContext appContext, IUmbracoSettingsSection settings) + public LogScrubber(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + ApplicationContext appContext, IUmbracoSettingsSection settings) + : base(runner, delayMilliseconds, periodMilliseconds) { _appContext = appContext; _settings = settings; } + public LogScrubber(LogScrubber source) + : base(source) + { + _appContext = source._appContext; + _settings = source._settings; + } + + protected override LogScrubber GetRecurring() + { + return new LogScrubber(this); + } + private int GetLogScrubbingMaximumAge(IUmbracoSettingsSection settings) { int maximumAge = 24 * 60 * 60; @@ -36,14 +50,22 @@ namespace Umbraco.Web.Scheduling } - /// - /// Handles the disposal of resources. Derived from abstract class which handles common required locking logic. - /// - protected override void DisposeResources() - { + public static int GetLogScrubbingInterval(IUmbracoSettingsSection settings) + { + int interval = 24 * 60 * 60; //24 hours + try + { + if (settings.Logging.CleaningMiliseconds > -1) + interval = settings.Logging.CleaningMiliseconds; + } + catch (Exception e) + { + LogHelper.Error("Unable to locate a log scrubbing interval. Defaulting to 24 horus", e); + } + return interval; } - public void Run() + public override void PerformRun() { using (DisposableTimer.DebugDuration(() => "Log scrubbing executing", () => "Log scrubbing complete")) { @@ -51,12 +73,12 @@ namespace Umbraco.Web.Scheduling } } - public Task RunAsync() + public override Task PerformRunAsync() { throw new NotImplementedException(); } - public bool IsAsync + public override bool IsAsync { get { return false; } } diff --git a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs index 91d86d97b4..553e62d3a0 100644 --- a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs @@ -67,12 +67,17 @@ namespace Umbraco.Web.Scheduling var recur = GetRecurring(); if (recur == null) return; // done + // note + // must use the single-parameter constructor on Timer to avoid it from being GC'd + // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer + _timer = new Timer(_ => { _timer.Dispose(); _timer = null; _runner.TryAdd(recur); - }, null, _periodMilliseconds, 0); + }); + _timer.Change(_periodMilliseconds, 0); } /// diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index cacb4e133d..de92374379 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -12,30 +12,41 @@ using Umbraco.Web.Mvc; namespace Umbraco.Web.Scheduling { - internal class ScheduledPublishing : DisposableObject, IBackgroundTask + internal class ScheduledPublishing : DelayedRecurringTaskBase { private readonly ApplicationContext _appContext; private readonly IUmbracoSettingsSection _settings; - private static bool _isPublishingRunning = false; + private static bool _isPublishingRunning; - public ScheduledPublishing(ApplicationContext appContext, IUmbracoSettingsSection settings) + public ScheduledPublishing(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + ApplicationContext appContext, IUmbracoSettingsSection settings) + : base(runner, delayMilliseconds, periodMilliseconds) { _appContext = appContext; _settings = settings; } - - /// - /// Handles the disposal of resources. Derived from abstract class which handles common required locking logic. - /// - protected override void DisposeResources() + private ScheduledPublishing(ScheduledPublishing source) + : base(source) { + _appContext = source._appContext; + _settings = source._settings; } - public void Run() + protected override ScheduledPublishing GetRecurring() + { + return new ScheduledPublishing(this); + } + + public override void PerformRun() { if (_appContext == null) return; + if (ServerEnvironmentHelper.GetStatus(_settings) == CurrentServerEnvironmentStatus.Slave) + { + LogHelper.Debug("Does not run on slave servers."); + return; + } using (DisposableTimer.DebugDuration(() => "Scheduled publishing executing", () => "Scheduled publishing complete")) { @@ -77,12 +88,12 @@ namespace Umbraco.Web.Scheduling } } - public Task RunAsync() + public override Task PerformRunAsync() { throw new NotImplementedException(); } - public bool IsAsync + public override bool IsAsync { get { return false; } } diff --git a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs index ddcb9ea533..bd3a3524f6 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs @@ -17,19 +17,33 @@ namespace Umbraco.Web.Scheduling // would need to be a publicly available task (URL) which isn't really very good :( // We should really be using the AdminTokenAuthorizeAttribute for this stuff - internal class ScheduledTasks : DisposableObject, IBackgroundTask + internal class ScheduledTasks : DelayedRecurringTaskBase { private readonly ApplicationContext _appContext; private readonly IUmbracoSettingsSection _settings; private static readonly Hashtable ScheduledTaskTimes = new Hashtable(); private static bool _isPublishingRunning = false; - public ScheduledTasks(ApplicationContext appContext, IUmbracoSettingsSection settings) + public ScheduledTasks(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + ApplicationContext appContext, IUmbracoSettingsSection settings) + : base(runner, delayMilliseconds, periodMilliseconds) { _appContext = appContext; _settings = settings; } + public ScheduledTasks(ScheduledTasks source) + : base(source) + { + _appContext = source._appContext; + _settings = source._settings; + } + + protected override ScheduledTasks GetRecurring() + { + return new ScheduledTasks(this); + } + private void ProcessTasks() { var scheduledTasks = _settings.ScheduledTasks.Tasks; @@ -78,15 +92,14 @@ namespace Umbraco.Web.Scheduling return false; } - /// - /// Handles the disposal of resources. Derived from abstract class which handles common required locking logic. - /// - protected override void DisposeResources() + public override void PerformRun() { - } + if (ServerEnvironmentHelper.GetStatus(_settings) == CurrentServerEnvironmentStatus.Slave) + { + LogHelper.Debug("Does not run on slave servers."); + return; + } - public void Run() - { using (DisposableTimer.DebugDuration(() => "Scheduled tasks executing", () => "Scheduled tasks complete")) { if (_isPublishingRunning) return; @@ -108,12 +121,12 @@ namespace Umbraco.Web.Scheduling } } - public Task RunAsync() + public override Task PerformRunAsync() { throw new NotImplementedException(); } - public bool IsAsync + public override bool IsAsync { get { return false; } } diff --git a/src/Umbraco.Web/Scheduling/Scheduler.cs b/src/Umbraco.Web/Scheduling/Scheduler.cs index ee02947e20..6e586efad8 100644 --- a/src/Umbraco.Web/Scheduling/Scheduler.cs +++ b/src/Umbraco.Web/Scheduling/Scheduler.cs @@ -19,12 +19,10 @@ namespace Umbraco.Web.Scheduling internal sealed class Scheduler : ApplicationEventHandler { private static Timer _pingTimer; - private static Timer _schedulingTimer; - private static BackgroundTaskRunner _publishingRunner; - private static BackgroundTaskRunner _tasksRunner; - private static BackgroundTaskRunner _scrubberRunner; - private static Timer _logScrubberTimer; - private static volatile bool _started = false; + private static BackgroundTaskRunner _publishingRunner; + private static BackgroundTaskRunner _tasksRunner; + private static BackgroundTaskRunner _scrubberRunner; + private static volatile bool _started; private static readonly object Locker = new object(); protected override void ApplicationStarted(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) @@ -49,97 +47,37 @@ namespace Umbraco.Web.Scheduling _started = true; LogHelper.Debug(() => "Initializing the scheduler"); - // time to setup the tasks + // backgrounds runners are web aware, if the app domain dies, these tasks will wind down correctly + _publishingRunner = new BackgroundTaskRunner(); + _tasksRunner = new BackgroundTaskRunner(); + _scrubberRunner = new BackgroundTaskRunner(); - //We have 3 background runners that are web aware, if the app domain dies, these tasks will wind down correctly - _publishingRunner = new BackgroundTaskRunner(); - _tasksRunner = new BackgroundTaskRunner(); - _scrubberRunner = new BackgroundTaskRunner(); + var settings = UmbracoConfig.For.UmbracoSettings(); - //NOTE: It is important to note that we need to use the ctor for a timer without the 'state' object specified, this is in order - // to ensure that the timer itself is not GC'd since internally .net will pass itself in as the state object and that will keep it alive. - // There's references to this here: http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer - // we also make these timers static to ensure further GC safety. + // note + // must use the single-parameter constructor on Timer to avoid it from being GC'd + // also make the timer static to ensure further GC safety + // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer - // ping/keepalive - NOTE: we don't use a background runner for this because it does not need to be web aware, if the app domain dies, no problem + // ping/keepalive - no need for a background runner - does not need to be web aware, ok if the app domain dies _pingTimer = new Timer(state => KeepAlive.Start(applicationContext, UmbracoConfig.For.UmbracoSettings())); _pingTimer.Change(60000, 300000); // scheduled publishing/unpublishing - _schedulingTimer = new Timer(state => PerformScheduling(applicationContext, UmbracoConfig.For.UmbracoSettings())); - _schedulingTimer.Change(60000, 60000); + // install on all, will only run on non-slaves servers + // both are delayed recurring tasks + _publishingRunner.Add(new ScheduledPublishing(_publishingRunner, 60000, 60000, applicationContext, settings)); + _tasksRunner.Add(new ScheduledTasks(_tasksRunner, 60000, 60000, applicationContext, settings)); - //log scrubbing - _logScrubberTimer = new Timer(state => PerformLogScrub(applicationContext, UmbracoConfig.For.UmbracoSettings())); - _logScrubberTimer.Change(60000, GetLogScrubbingInterval(UmbracoConfig.For.UmbracoSettings())); + // log scrubbing + // install & run on all servers + // LogScrubber is a delayed recurring task + _scrubberRunner.Add(new LogScrubber(_scrubberRunner, 60000, LogScrubber.GetLogScrubbingInterval(settings), applicationContext, settings)); } } } }; }; } - - - private int GetLogScrubbingInterval(IUmbracoSettingsSection settings) - { - var interval = 4 * 60 * 60 * 1000; // 4 hours, in milliseconds - try - { - if (settings.Logging.CleaningMiliseconds > -1) - interval = settings.Logging.CleaningMiliseconds; - } - catch (Exception e) - { - LogHelper.Error("Unable to locate a log scrubbing interval. Defaulting to 4 hours.", e); - } - return interval; - } - - private static void PerformLogScrub(ApplicationContext appContext, IUmbracoSettingsSection settings) - { - _scrubberRunner.Add(new LogScrubber(appContext, settings)); - } - - /// - /// This performs all of the scheduling on the one timer - /// - /// - /// - /// - /// No processing will be done if this server is a slave - /// - private static void PerformScheduling(ApplicationContext appContext, IUmbracoSettingsSection settings) - { - using (DisposableTimer.DebugDuration(() => "Scheduling interval executing", () => "Scheduling interval complete")) - { - //get the current server status to see if this server should execute the scheduled publishing - var serverStatus = ServerEnvironmentHelper.GetStatus(settings); - - switch (serverStatus) - { - case CurrentServerEnvironmentStatus.Single: - case CurrentServerEnvironmentStatus.Master: - case CurrentServerEnvironmentStatus.Unknown: - //if it's a single server install, a master or it cannot be determined - // then we will process the scheduling - - //do the scheduled publishing - _publishingRunner.Add(new ScheduledPublishing(appContext, settings)); - - //do the scheduled tasks - _tasksRunner.Add(new ScheduledTasks(appContext, settings)); - - break; - case CurrentServerEnvironmentStatus.Slave: - //do not process - - LogHelper.Debug( - () => string.Format("Current server ({0}) detected as a slave, no scheduled processes will execute on this server", NetworkHelper.MachineName)); - - break; - } - } - } - } } From be3702658711df34f88029630a390e80c111478f Mon Sep 17 00:00:00 2001 From: Stephan Date: Sun, 8 Feb 2015 16:25:30 +0100 Subject: [PATCH 09/13] refactor latched background tasks, now use a task for xml Conflicts: src/Umbraco.Web.UI/config/ClientDependency.config --- .../Scheduling/BackgroundTaskRunnerTests.cs | 11 +- .../XmlCacheFilePersister.cs | 115 ++++++++++++++++-- .../Scheduling/BackgroundTaskRunner.cs | 20 +-- .../Scheduling/DelayedRecurringTaskBase.cs | 66 ++++++---- .../Scheduling/IDelayedBackgroundTask.cs | 23 ---- .../Scheduling/ILatchedBackgroundTask.cs | 31 +++++ src/Umbraco.Web/Scheduling/LogScrubber.cs | 5 + .../Scheduling/RecurringTaskBase.cs | 22 ++-- .../Scheduling/ScheduledPublishing.cs | 5 + src/Umbraco.Web/Scheduling/ScheduledTasks.cs | 5 + src/Umbraco.Web/Umbraco.Web.csproj | 2 +- .../umbraco.presentation/content.cs | 17 ++- 12 files changed, 239 insertions(+), 83 deletions(-) delete mode 100644 src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs create mode 100644 src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index e83ce400d9..299c11881d 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -564,7 +564,7 @@ namespace Umbraco.Tests.Scheduling } } - private class MyDelayedTask : IDelayedBackgroundTask + private class MyDelayedTask : ILatchedBackgroundTask { private readonly int _runMilliseconds; private readonly ManualResetEvent _gate; @@ -577,12 +577,17 @@ namespace Umbraco.Tests.Scheduling _gate = new ManualResetEvent(false); } - public WaitHandle DelayWaitHandle + public WaitHandle Latch { get { return _gate; } } - public bool IsDelayed + public bool IsLatched + { + get { return true; } + } + + public bool RunsOnShutdown { get { return true; } } diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs index 482a666538..1bed36160f 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs @@ -4,6 +4,7 @@ using System.IO; using System.Threading; using System.Threading.Tasks; using System.Xml; +using umbraco; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Web.Scheduling; @@ -19,22 +20,120 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache /// if multiple threads are performing publishing tasks that the file will be persisted in accordance with the final resulting /// xml structure since the file writes are queued. /// - internal class XmlCacheFilePersister : DisposableObject, IBackgroundTask + internal class XmlCacheFilePersister : ILatchedBackgroundTask { - private readonly XmlDocument _xDoc; + private readonly IBackgroundTaskRunner _runner; private readonly string _xmlFileName; private readonly ProfilingLogger _logger; + private readonly content _content; + private readonly ManualResetEventSlim _latch = new ManualResetEventSlim(false); + private readonly object _locko = new object(); + private bool _released; + private Timer _timer; + private DateTime _initialTouch; - public XmlCacheFilePersister(XmlDocument xDoc, string xmlFileName, ProfilingLogger logger) + private const int WaitMilliseconds = 4000; // save the cache 4s after the last change (ie every 4s min) + private const int MaxWaitMilliseconds = 10000; // save the cache after some time (ie no more than 10s of changes) + + // save the cache when the app goes down + public bool RunsOnShutdown { get { return true; } } + + public XmlCacheFilePersister(IBackgroundTaskRunner runner, content content, string xmlFileName, ProfilingLogger logger, bool touched = false) { - _xDoc = xDoc; + _runner = runner; + _content = content; _xmlFileName = xmlFileName; _logger = logger; + + if (touched == false) return; + + LogHelper.Debug("Create new touched, start."); + + _initialTouch = DateTime.Now; + _timer = new Timer(_ => Release()); + + LogHelper.Debug("Save in {0}ms.", () => WaitMilliseconds); + _timer.Change(WaitMilliseconds, 0); } + public XmlCacheFilePersister Touch() + { + lock (_locko) + { + if (_released) + { + LogHelper.Debug("Touched, was released, create new."); + + // released, has run or is running, too late, add & return a new task + var persister = new XmlCacheFilePersister(_runner, _content, _xmlFileName, _logger, true); + _runner.Add(persister); + return persister; + } + + if (_timer == null) + { + LogHelper.Debug("Touched, was idle, start."); + + // not started yet, start + _initialTouch = DateTime.Now; + _timer = new Timer(_ => Release()); + LogHelper.Debug("Save in {0}ms.", () => WaitMilliseconds); + _timer.Change(WaitMilliseconds, 0); + return this; + } + + // set the timer to trigger in WaitMilliseconds unless we've been touched first more + // than MaxWaitMilliseconds ago and then release now + + if (DateTime.Now - _initialTouch < TimeSpan.FromMilliseconds(MaxWaitMilliseconds)) + { + LogHelper.Debug("Touched, was waiting, wait.", () => WaitMilliseconds); + LogHelper.Debug("Save in {0}ms.", () => WaitMilliseconds); + _timer.Change(WaitMilliseconds, 0); + } + else + { + LogHelper.Debug("Save now, release."); + ReleaseLocked(); + } + + return this; // still available + } + } + + private void Release() + { + lock (_locko) + { + ReleaseLocked(); + } + } + + private void ReleaseLocked() + { + LogHelper.Debug("Timer: save now, release."); + if (_timer != null) + _timer.Dispose(); + _timer = null; + _released = true; + _latch.Set(); + } + + public WaitHandle Latch + { + get { return _latch.WaitHandle; } + } + + public bool IsLatched + { + get { return true; } + } + public async Task RunAsync() { - await PersistXmlToFileAsync(_xDoc); + LogHelper.Debug("Run now."); + var doc = _content.XmlContentInternal; + await PersistXmlToFileAsync(doc); } public bool IsAsync @@ -91,14 +190,12 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache } } - protected override void DisposeResources() - { - } + public void Dispose() + { } public void Run() { throw new NotImplementedException(); } - } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index c511a0af53..82fb601815 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -21,7 +21,7 @@ namespace Umbraco.Web.Scheduling private readonly BackgroundTaskRunnerOptions _options; private readonly BlockingCollection _tasks = new BlockingCollection(); private readonly object _locker = new object(); - private readonly ManualResetEvent _completedEvent = new ManualResetEvent(false); + private readonly ManualResetEventSlim _completedEvent = new ManualResetEventSlim(false); private volatile bool _isRunning; // is running private volatile bool _isCompleted; // does not accept tasks anymore, may still be running @@ -304,13 +304,19 @@ namespace Umbraco.Web.Scheduling return; } - // wait for delayed task, supporting cancellation - var dbgTask = bgTask as IDelayedBackgroundTask; - if (dbgTask != null && dbgTask.IsDelayed) + // wait for latched task, supporting cancellation + var dbgTask = bgTask as ILatchedBackgroundTask; + if (dbgTask != null && dbgTask.IsLatched) { - WaitHandle.WaitAny(new[] { dbgTask.DelayWaitHandle, token.WaitHandle, _completedEvent }); + WaitHandle.WaitAny(new[] { dbgTask.Latch, token.WaitHandle, _completedEvent.WaitHandle }); if (TaskSourceCanceled(taskSource, token)) return; - // else run now, either because delay is ok or runner is completed + // else run now, either because latch ok or runner is completed + // still latched & not running on shutdown = stop here + if (dbgTask.IsLatched && dbgTask.RunsOnShutdown == false) + { + TaskSourceCompleted(taskSource, token); + return; + } } // run the task as first task, or a continuation @@ -378,7 +384,7 @@ namespace Umbraco.Web.Scheduling OnTaskError(new TaskEventArgs(bgTask, e)); throw; } - Console.WriteLine("!1"); + OnTaskCompleted(new TaskEventArgs(bgTask)); } catch (Exception ex) diff --git a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs index cac68241f4..3ecae089cc 100644 --- a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs @@ -1,5 +1,6 @@ using System; using System.Threading; +using System.Threading.Tasks; namespace Umbraco.Web.Scheduling { @@ -7,52 +8,67 @@ namespace Umbraco.Web.Scheduling /// Provides a base class for recurring background tasks. /// /// The type of the managed tasks. - internal abstract class DelayedRecurringTaskBase : RecurringTaskBase, IDelayedBackgroundTask + internal abstract class DelayedRecurringTaskBase : RecurringTaskBase, ILatchedBackgroundTask where T : class, IBackgroundTask { - private readonly int _delayMilliseconds; - private ManualResetEvent _gate; + private readonly ManualResetEventSlim _latch; private Timer _timer; protected DelayedRecurringTaskBase(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) : base(runner, periodMilliseconds) { - _delayMilliseconds = delayMilliseconds; + if (delayMilliseconds > 0) + { + _latch = new ManualResetEventSlim(false); + _timer = new Timer(_ => + { + _timer.Dispose(); + _timer = null; + _latch.Set(); + }); + _timer.Change(delayMilliseconds, 0); + } } protected DelayedRecurringTaskBase(DelayedRecurringTaskBase source) : base(source) { - _delayMilliseconds = 0; + // no latch on recurring instances + _latch = null; } - public WaitHandle DelayWaitHandle + public override void Run() + { + if (_latch != null) + _latch.Dispose(); + base.Run(); + } + + public override async Task RunAsync() + { + if (_latch != null) + _latch.Dispose(); + await base.RunAsync(); + } + + public WaitHandle Latch { get { - if (_delayMilliseconds == 0) return new ManualResetEvent(true); - - if (_gate != null) return _gate; - _gate = new ManualResetEvent(false); - - // note - // must use the single-parameter constructor on Timer to avoid it from being GC'd - // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer - - _timer = new Timer(_ => - { - _timer.Dispose(); - _timer = null; - _gate.Set(); - }); - _timer.Change(_delayMilliseconds, 0); - return _gate; + if (_latch == null) + throw new InvalidOperationException("The task is not latched."); + return _latch.WaitHandle; } } - public bool IsDelayed + public bool IsLatched { - get { return _delayMilliseconds > 0; } + get { return _latch != null; } + } + + public virtual bool RunsOnShutdown + { + get { return true; } } } } diff --git a/src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs b/src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs deleted file mode 100644 index 01f8a5e01a..0000000000 --- a/src/Umbraco.Web/Scheduling/IDelayedBackgroundTask.cs +++ /dev/null @@ -1,23 +0,0 @@ -using System.Threading; - -namespace Umbraco.Web.Scheduling -{ - /// - /// Represents a delayed background task. - /// - /// Delayed background tasks can suspend their execution until - /// a condition is met. However if the tasks runner has to terminate, - /// delayed background tasks are executed immediately. - internal interface IDelayedBackgroundTask : IBackgroundTask - { - /// - /// Gets a wait handle on the task condition. - /// - WaitHandle DelayWaitHandle { get; } - - /// - /// Gets a value indicating whether the task is delayed. - /// - bool IsDelayed { get; } - } -} diff --git a/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs b/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs new file mode 100644 index 0000000000..0ad4d42bdf --- /dev/null +++ b/src/Umbraco.Web/Scheduling/ILatchedBackgroundTask.cs @@ -0,0 +1,31 @@ +using System; +using System.Threading; + +namespace Umbraco.Web.Scheduling +{ + /// + /// Represents a latched background task. + /// + /// Latched background tasks can suspend their execution until + /// a condition is met. However if the tasks runner has to terminate, + /// latched background tasks can be executed immediately, depending on + /// the value returned by RunsOnShutdown. + internal interface ILatchedBackgroundTask : IBackgroundTask + { + /// + /// Gets a wait handle on the task condition. + /// + /// The task is not latched. + WaitHandle Latch { get; } + + /// + /// Gets a value indicating whether the task is latched. + /// + bool IsLatched { get; } + + /// + /// Gets a value indicating whether the task can be executed immediately if the task runner has to terminate. + /// + bool RunsOnShutdown { get; } + } +} diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index a0c2c6979e..c4a5ce2c00 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -82,5 +82,10 @@ namespace Umbraco.Web.Scheduling { get { return false; } } + + public override bool RunsOnShutdown + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs index 553e62d3a0..6bae7406f9 100644 --- a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs @@ -13,6 +13,7 @@ namespace Umbraco.Web.Scheduling private readonly IBackgroundTaskRunner _runner; private readonly int _periodMilliseconds; private Timer _timer; + private T _recurrent; /// /// Initializes a new instance of the class with a tasks runner and a period. @@ -34,6 +35,7 @@ namespace Umbraco.Web.Scheduling protected RecurringTaskBase(RecurringTaskBase source) { _runner = source._runner; + _timer = source._timer; _periodMilliseconds = source._periodMilliseconds; } @@ -41,7 +43,7 @@ namespace Umbraco.Web.Scheduling /// Implements IBackgroundTask.Run(). /// /// Classes inheriting from RecurringTaskBase must implement PerformRun. - public void Run() + public virtual void Run() { PerformRun(); Repeat(); @@ -51,7 +53,7 @@ namespace Umbraco.Web.Scheduling /// Implements IBackgroundTask.RunAsync(). /// /// Classes inheriting from RecurringTaskBase must implement PerformRun. - public async Task RunAsync() + public virtual async Task RunAsync() { await PerformRunAsync(); Repeat(); @@ -64,19 +66,19 @@ namespace Umbraco.Web.Scheduling if (_periodMilliseconds == 0) return; - var recur = GetRecurring(); - if (recur == null) return; // done + _recurrent = GetRecurring(); + if (_recurrent == null) + { + _timer.Dispose(); + _timer = null; + return; // done + } // note // must use the single-parameter constructor on Timer to avoid it from being GC'd // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer - _timer = new Timer(_ => - { - _timer.Dispose(); - _timer = null; - _runner.TryAdd(recur); - }); + _timer = _timer ?? new Timer(_ => _runner.TryAdd(_recurrent)); _timer.Change(_periodMilliseconds, 0); } diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index de92374379..9db21fba8a 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -97,5 +97,10 @@ namespace Umbraco.Web.Scheduling { get { return false; } } + + public override bool RunsOnShutdown + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs index bd3a3524f6..cba3cb4fc8 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs @@ -130,5 +130,10 @@ namespace Umbraco.Web.Scheduling { get { return false; } } + + public override bool RunsOnShutdown + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 58821bec6b..24d5058e31 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -501,7 +501,7 @@ - + diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index cbf296115a..fa2e93b0b1 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -35,6 +35,15 @@ namespace umbraco private static readonly BackgroundTaskRunner FilePersister = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions { LongRunning = true }); + private XmlCacheFilePersister _persisterTask; + + private content() + { + _persisterTask = new XmlCacheFilePersister(FilePersister, this, UmbracoXmlDiskCacheFileName, + new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler)); + FilePersister.Add(_persisterTask); + } + #region Declarations // Sync access to disk file @@ -131,7 +140,7 @@ namespace umbraco /// /// Before returning we always check to ensure that the xml is loaded /// - protected virtual XmlDocument XmlContentInternal + protected internal virtual XmlDocument XmlContentInternal { get { @@ -317,8 +326,7 @@ namespace umbraco // and clear the queue in case is this a web request, we don't want it reprocessing. if (UmbracoConfig.For.UmbracoSettings().Content.XmlCacheEnabled && UmbracoConfig.For.UmbracoSettings().Content.ContinouslyUpdateXmlDiskCache) { - FilePersister.Add(new XmlCacheFilePersister(xmlDoc, UmbracoXmlDiskCacheFileName , - new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler))); + QueueXmlForPersistence(); } } } @@ -1230,8 +1238,7 @@ order by umbracoNode.level, umbracoNode.sortOrder"; /// private void QueueXmlForPersistence() { - FilePersister.Add(new XmlCacheFilePersister(_xmlContent, UmbracoXmlDiskCacheFileName, - new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler))); + _persisterTask = _persisterTask.Touch(); } internal DateTime GetCacheFileUpdateTime() From 4c3de920c6ec65362a65af2dfa7a5e8a89aedb88 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 17 Feb 2015 15:05:42 +0100 Subject: [PATCH 10/13] Removes the 'else' so that 'wait' is always checked. --- src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index 82fb601815..2dfba25a48 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -231,12 +231,11 @@ namespace Umbraco.Web.Scheduling // truncate running queues _tokenSource.Cancel(false); // false is the default } - else - { - // tasks in the queue will be executed... - if (wait == false) return; - _runningTask.Wait(); // wait for whatever is running to end... - } + + // tasks in the queue will be executed... + if (wait == false) return; + _runningTask.Wait(); // wait for whatever is running to end... + } /// From 58ce04e26b8444214e64fa58feb5abb4849057fa Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 17 Feb 2015 15:09:29 +0100 Subject: [PATCH 11/13] cleanup --- src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs | 8 ++++---- .../XmlPublishedCache/XmlCacheFilePersister.cs | 4 ++-- src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs | 5 ++--- src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs | 4 ++-- src/Umbraco.Web/Scheduling/IBackgroundTask.cs | 4 +++- src/Umbraco.Web/Scheduling/RecurringTaskBase.cs | 2 +- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 299c11881d..ab83294496 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -514,7 +514,7 @@ namespace Umbraco.Tests.Scheduling throw new Exception("Task has thrown."); } - public async Task RunAsync() + public async Task RunAsync(CancellationToken token) { await Task.Delay(1000); throw new Exception("Task has thrown."); @@ -603,7 +603,7 @@ namespace Umbraco.Tests.Scheduling _gate.Set(); } - public Task RunAsync() + public Task RunAsync(CancellationToken token) { throw new NotImplementedException(); } @@ -690,10 +690,10 @@ namespace Umbraco.Tests.Scheduling Ended = DateTime.Now; } - public Task RunAsync() + public Task RunAsync(CancellationToken token) { throw new NotImplementedException(); - //return Task.Delay(500); // fixme + //return Task.Delay(500); } public bool IsAsync diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs index 1bed36160f..1254951d82 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs @@ -128,8 +128,8 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache { get { return true; } } - - public async Task RunAsync() + + public async Task RunAsync(CancellationToken token) { LogHelper.Debug("Run now."); var doc = _content.XmlContentInternal; diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index 2dfba25a48..3a5ace8af8 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -14,7 +13,7 @@ namespace Umbraco.Web.Scheduling /// /// The type of the managed tasks. /// The task runner is web-aware and will ensure that it shuts down correctly when the AppDomain - /// shuts down (ie is unloaded). FIXME WHAT DOES THAT MEAN? + /// shuts down (ie is unloaded). internal class BackgroundTaskRunner : IBackgroundTaskRunner where T : class, IBackgroundTask { @@ -373,7 +372,7 @@ namespace Umbraco.Web.Scheduling using (bgTask) // ensure it's disposed { if (bgTask.IsAsync) - await bgTask.RunAsync(); // fixme should pass the token along?! + await bgTask.RunAsync(token); else bgTask.Run(); } diff --git a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs index 3ecae089cc..f7cec0079b 100644 --- a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs @@ -44,11 +44,11 @@ namespace Umbraco.Web.Scheduling base.Run(); } - public override async Task RunAsync() + public override async Task RunAsync(CancellationToken token) { if (_latch != null) _latch.Dispose(); - await base.RunAsync(); + await base.RunAsync(token); } public WaitHandle Latch diff --git a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs index 9be2512d01..4e646c0623 100644 --- a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs +++ b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using System.Threading.Tasks; namespace Umbraco.Web.Scheduling @@ -16,9 +17,10 @@ namespace Umbraco.Web.Scheduling /// /// Runs the task asynchronously. /// + /// A cancellation token. /// A instance representing the execution of the background task. /// The background task cannot run asynchronously. - Task RunAsync(); + Task RunAsync(CancellationToken token); /// /// Indicates whether the background task can run asynchronously. diff --git a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs index 6bae7406f9..d710a70e03 100644 --- a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs @@ -53,7 +53,7 @@ namespace Umbraco.Web.Scheduling /// Implements IBackgroundTask.RunAsync(). /// /// Classes inheriting from RecurringTaskBase must implement PerformRun. - public virtual async Task RunAsync() + public virtual async Task RunAsync(CancellationToken token) { await PerformRunAsync(); Repeat(); From 4bdac1475ff496062f9e6d81354ea8c22ffdccc7 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 27 Mar 2015 14:52:38 +1100 Subject: [PATCH 12/13] fixes log scrub interval with ms with updated background tasks --- src/Umbraco.Web/Scheduling/LogScrubber.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index c4a5ce2c00..a9f70a612e 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -52,7 +52,7 @@ namespace Umbraco.Web.Scheduling public static int GetLogScrubbingInterval(IUmbracoSettingsSection settings) { - int interval = 24 * 60 * 60; //24 hours + var interval = 4 * 60 * 60 * 1000; // 4 hours, in milliseconds try { if (settings.Logging.CleaningMiliseconds > -1) @@ -60,7 +60,7 @@ namespace Umbraco.Web.Scheduling } catch (Exception e) { - LogHelper.Error("Unable to locate a log scrubbing interval. Defaulting to 24 horus", e); + LogHelper.Error("Unable to locate a log scrubbing interval. Defaulting to 4 hours.", e); } return interval; } From ec742d1f6690d9ae9629abb5d2d8287c64ccd604 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 27 Mar 2015 14:56:12 +1100 Subject: [PATCH 13/13] removes the usages of ILogger and ProfilerLogger since this is not for 7.3.0, will need to re-fix for 7.3 branch. --- .../XmlPublishedCache/XmlCacheFilePersister.cs | 11 ++++++----- src/Umbraco.Web/umbraco.presentation/content.cs | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs index 1254951d82..d304e99f93 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs @@ -24,7 +24,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache { private readonly IBackgroundTaskRunner _runner; private readonly string _xmlFileName; - private readonly ProfilingLogger _logger; + //private readonly ProfilingLogger _logger; private readonly content _content; private readonly ManualResetEventSlim _latch = new ManualResetEventSlim(false); private readonly object _locko = new object(); @@ -38,12 +38,12 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache // save the cache when the app goes down public bool RunsOnShutdown { get { return true; } } - public XmlCacheFilePersister(IBackgroundTaskRunner runner, content content, string xmlFileName, ProfilingLogger logger, bool touched = false) + public XmlCacheFilePersister(IBackgroundTaskRunner runner, content content, string xmlFileName, /*ProfilingLogger logger, */bool touched = false) { _runner = runner; _content = content; _xmlFileName = xmlFileName; - _logger = logger; + //_logger = logger; if (touched == false) return; @@ -65,7 +65,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache LogHelper.Debug("Touched, was released, create new."); // released, has run or is running, too late, add & return a new task - var persister = new XmlCacheFilePersister(_runner, _content, _xmlFileName, _logger, true); + var persister = new XmlCacheFilePersister(_runner, _content, _xmlFileName, /*_logger, */true); _runner.Add(persister); return persister; } @@ -149,7 +149,8 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache { if (xmlDoc != null) { - using (_logger.DebugDuration( + //using (_logger.DebugDuration( + using(DisposableTimer.DebugDuration( string.Format("Saving content to disk on thread '{0}' (Threadpool? {1})", Thread.CurrentThread.Name, Thread.CurrentThread.IsThreadPoolThread), string.Format("Saved content to disk on thread '{0}' (Threadpool? {1})", Thread.CurrentThread.Name, Thread.CurrentThread.IsThreadPoolThread))) { diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index fa2e93b0b1..93e07a680c 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -39,8 +39,8 @@ namespace umbraco private content() { - _persisterTask = new XmlCacheFilePersister(FilePersister, this, UmbracoXmlDiskCacheFileName, - new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler)); + _persisterTask = new XmlCacheFilePersister(FilePersister, this, UmbracoXmlDiskCacheFileName + /*,new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler)*/); FilePersister.Add(_persisterTask); }