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 a3e5122a06..68ac552b0b 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -553,6 +553,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 e2773a7112..63c0652edf 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(); + //} }