From cd1120e062dc95fba2f7d9b2a29c0193e6ba8919 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 23 Nov 2020 07:43:24 +0100 Subject: [PATCH] Merge pull request #9408 from umbraco/v8/bugfix/task-scheduler-maindom-logging Ensure that TaskScheduler.Default is used anywhere that ContinueWith is used, adds more debug logging to MainDom operations (cherry picked from commit 9a1b4569494d6bce3e04687868ba944d69dbb309) # Conflicts: # src/Umbraco.Core/Runtime/SqlMainDomLock.cs --- .../Logging/LoggingTaskExtension.cs | 13 +++++++++-- src/Umbraco.Core/Runtime/MainDom.cs | 12 +++++++++- src/Umbraco.Core/Runtime/SqlMainDomLock.cs | 17 ++++++++++++-- .../PublishedCache/NuCache/ContentStore.cs | 6 ++++- .../PublishedCache/NuCache/SnapDictionary.cs | 6 ++++- .../Scheduling/BackgroundTaskRunner.cs | 10 ++++++++- .../Scheduling/TaskAndFactoryExtensions.cs | 22 +++++++++++++++++-- .../WebApi/HttpActionContextExtensions.cs | 4 +++- 8 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/Logging/LoggingTaskExtension.cs b/src/Umbraco.Core/Logging/LoggingTaskExtension.cs index 1f742133c3ab..2e3aa0a883c1 100644 --- a/src/Umbraco.Core/Logging/LoggingTaskExtension.cs +++ b/src/Umbraco.Core/Logging/LoggingTaskExtension.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using System.Threading.Tasks; namespace Umbraco.Core.Logging @@ -14,7 +15,12 @@ internal static class LoggingTaskExtension /// public static Task LogErrors(this Task task, Action logMethod) { - return task.ContinueWith(t => LogErrorsInner(t, logMethod), TaskContinuationOptions.OnlyOnFaulted); + return task.ContinueWith( + t => LogErrorsInner(t, logMethod), + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); } /// @@ -26,7 +32,10 @@ public static Task LogErrors(this Task task, Action logMethod /// public static Task LogErrorsWaitable(this Task task, Action logMethod) { - return task.ContinueWith(t => LogErrorsInner(t, logMethod)); + return task.ContinueWith( + t => LogErrorsInner(t, logMethod), + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); } private static void LogErrorsInner(Task task, Action logAction) diff --git a/src/Umbraco.Core/Runtime/MainDom.cs b/src/Umbraco.Core/Runtime/MainDom.cs index 02f37f654ea4..71842905dd80 100644 --- a/src/Umbraco.Core/Runtime/MainDom.cs +++ b/src/Umbraco.Core/Runtime/MainDom.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Security.Cryptography; using System.Threading; +using System.Threading.Tasks; using System.Web.Hosting; using Umbraco.Core; using Umbraco.Core.Logging; @@ -38,6 +39,9 @@ internal class MainDom : IMainDom, IRegisteredObject, IDisposable private const int LockTimeoutMilliseconds = 40000; // 40 seconds + private Task _listenTask; + private Task _listenCompleteTask; + #endregion #region Ctor @@ -172,7 +176,13 @@ private bool Acquire() try { // Listen for the signal from another AppDomain coming online to release the lock - _mainDomLock.ListenAsync().ContinueWith(_ => OnSignal("signal")); + _listenTask = _mainDomLock.ListenAsync(); + _listenCompleteTask = _listenTask.ContinueWith(t => + { + _logger.Debug("Listening task completed with {TaskStatus}", _listenTask.Status); + + OnSignal("signal"); + }, TaskScheduler.Default); // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html } catch (OperationCanceledException ex) { diff --git a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs index 84d98775d959..48b5804305cb 100644 --- a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs @@ -124,7 +124,12 @@ public Task ListenAsync() // Create a long running task (dedicated thread) // to poll to check if we are still the MainDom registered in the DB - return Task.Factory.StartNew(ListeningLoop, _cancellationTokenSource.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); + return Task.Factory.StartNew( + ListeningLoop, + _cancellationTokenSource.Token, + TaskCreationOptions.LongRunning, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); } @@ -159,7 +164,11 @@ private void ListeningLoop() // the other MainDom is taking to startup. In this case the db row will just be deleted and the // new MainDom will just take over. if (_cancellationTokenSource.IsCancellationRequested) + { + _logger.Debug("Task canceled, exiting loop"); return; + } + IUmbracoDatabase db = null; try { @@ -183,8 +192,10 @@ private void ListeningLoop() // We need to keep on listening unless we've been notified by our own AppDomain to shutdown since // we don't want to shutdown resources controlled by MainDom inadvertently. We'll just keep listening otherwise. if (_cancellationTokenSource.IsCancellationRequested) + { + _logger.Debug("Task canceled, exiting loop"); return; - + } } finally { @@ -389,6 +400,8 @@ protected virtual void Dispose(bool disposing) { lock (_locker) { + _logger.Debug($"{nameof(SqlMainDomLock)} Disposing..."); + // immediately cancel all sub-tasks, we don't want them to keep querying _cancellationTokenSource.Cancel(); _cancellationTokenSource.Dispose(); diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 34d21497a29d..07b10c9fbc52 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -1335,7 +1335,11 @@ private Task CollectAsyncLocked() { _collectTask = null; } - }, TaskContinuationOptions.ExecuteSynchronously); + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); // ReSharper restore InconsistentlySynchronizedField return task; diff --git a/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs b/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs index c38940da2592..589cd06d8a80 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs @@ -380,7 +380,11 @@ private Task CollectAsyncLocked() { _collectTask = null; } - }, TaskContinuationOptions.ExecuteSynchronously); + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); // ReSharper restore InconsistentlySynchronizedField return task; diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index c0475b1f7903..81bb45e27088 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -756,9 +756,17 @@ private void StopInitial() lock (_locker) { if (_runningTask != null) - _runningTask.ContinueWith(_ => StopImmediate()); + { + _runningTask.ContinueWith( + _ => StopImmediate(), + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); + } else + { StopImmediate(); + } + } } diff --git a/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs b/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs index 7220e77e0c2b..557fe377098e 100644 --- a/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs +++ b/src/Umbraco.Web/Scheduling/TaskAndFactoryExtensions.cs @@ -8,6 +8,7 @@ internal static class TaskAndFactoryExtensions { #region Task Extensions + // TODO: Not used, is this used in Deploy or something? static void SetCompletionSource(TaskCompletionSource completionSource, Task task) { if (task.IsFaulted) @@ -16,6 +17,7 @@ static void SetCompletionSource(TaskCompletionSource completio completionSource.SetResult(default(TResult)); } + // TODO: Not used, is this used in Deploy or something? static void SetCompletionSource(TaskCompletionSource completionSource, Task task) { if (task.IsFaulted) @@ -24,17 +26,33 @@ static void SetCompletionSource(TaskCompletionSource completio completionSource.SetResult(task.Result); } + // TODO: Not used, is this used in Deploy or something? public static Task ContinueWithTask(this Task task, Func continuation) { var completionSource = new TaskCompletionSource(); - task.ContinueWith(atask => continuation(atask).ContinueWith(atask2 => SetCompletionSource(completionSource, atask2))); + task.ContinueWith(atask => continuation(atask).ContinueWith( + atask2 => SetCompletionSource(completionSource, atask2), + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default), + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); return completionSource.Task; } + // TODO: Not used, is this used in Deploy or something? 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); + task.ContinueWith(atask => continuation(atask).ContinueWith( + atask2 => SetCompletionSource(completionSource, atask2), + token, + TaskContinuationOptions.None, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default), + token, + TaskContinuationOptions.None, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); return completionSource.Task; } diff --git a/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs b/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs index c67ec2f6a7b3..08d0ac825404 100644 --- a/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs +++ b/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs @@ -89,7 +89,9 @@ public static MultipartFormDataStreamProvider ReadAsMultipart(this HttpActionCon throw x.Exception; } result = x.ConfigureAwait(false).GetAwaiter().GetResult(); - }); + }, + // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html + TaskScheduler.Default); task.Wait(); if (result == null)