From b5f29f2390fa346a8edc098d5e42b0bef4ce89fe Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Fri, 18 Oct 2019 11:08:58 +1100
Subject: [PATCH] Ensure the EventWaitHandle is cleaned up, MainDom and
 BackgroundTaskRunner was not using HostingEnvironment.Stop correctly, ensures
 no exception is thrown when setting task result on a background task, ensure
 multiple terminations don't occur, ensure terminations are done if shutdown
 fails.

---
 src/Umbraco.Core/MainDom.cs                   | 41 ++++++++++++++++---
 src/Umbraco.Core/SystemLock.cs                |  2 +
 src/Umbraco.Core/Umbraco.Core.csproj          |  2 +-
 .../Scheduling/BackgroundTaskRunner.cs        | 41 ++++++++++++++-----
 4 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/src/Umbraco.Core/MainDom.cs b/src/Umbraco.Core/MainDom.cs
index 613abd6946..41ed8eb242 100644
--- a/src/Umbraco.Core/MainDom.cs
+++ b/src/Umbraco.Core/MainDom.cs
@@ -15,7 +15,7 @@ namespace Umbraco.Core
     /// <para>When an AppDomain starts, it tries to acquire the main domain status.</para>
     /// <para>When an AppDomain stops (eg the application is restarting) it should release the main domain status.</para>
     /// </remarks>
-    internal class MainDom : IMainDom, IRegisteredObject
+    internal class MainDom : IMainDom, IRegisteredObject, IDisposable
     {
         #region Vars
 
@@ -204,14 +204,43 @@ namespace Umbraco.Core
         // IRegisteredObject
         void IRegisteredObject.Stop(bool immediate)
         {
-            try
-            {
-                OnSignal("environment"); // will run once
-            }
-            finally
+            OnSignal("environment"); // will run once
+            
+            if (immediate)
             {
+                //only unregister when it's the final call, else we won't be notified of the final call
                 HostingEnvironment.UnregisterObject(this);
+
+                // The web app is stopping immediately, dispose eagerly
+                Dispose(true);
             }
         }
+
+        #region IDisposable Support
+
+        // This code added to correctly implement the disposable pattern.
+
+        private bool disposedValue = false; // To detect redundant calls
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if (!disposedValue)
+            {
+                if (disposing)
+                {
+                    _signal?.Close();
+                    _signal?.Dispose();
+                }
+
+                disposedValue = true;
+            }
+        }
+                
+        public void Dispose()
+        {
+            Dispose(true);
+        }
+
+        #endregion
     }
 }
diff --git a/src/Umbraco.Core/SystemLock.cs b/src/Umbraco.Core/SystemLock.cs
index a04eb2f980..04150d58c1 100644
--- a/src/Umbraco.Core/SystemLock.cs
+++ b/src/Umbraco.Core/SystemLock.cs
@@ -120,6 +120,8 @@ namespace Umbraco.Core
                     {
                         _semaphore.Release();
                         _semaphore.Dispose();
+
+                        
                     }
 
                     // free unmanaged resources (unmanaged objects) and override a finalizer below.
diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj
index c19722433f..2b9631c747 100755
--- a/src/Umbraco.Core/Umbraco.Core.csproj
+++ b/src/Umbraco.Core/Umbraco.Core.csproj
@@ -122,7 +122,7 @@
     </Compile>
     -->
     <Compile Include="AssemblyExtensions.cs" />
-    <Compile Include="AsyncLock.cs" />
+    <Compile Include="SystemLock.cs" />
     <Compile Include="Attempt.cs" />
     <Compile Include="AttemptOfTResult.cs" />
     <Compile Include="AttemptOfTResultTStatus.cs" />
diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs
index 47e97593f0..dab6a8865e 100644
--- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs
+++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs
@@ -700,16 +700,23 @@ namespace Umbraco.Web.Scheduling
                 // processing asynchronously before calling the UnregisterObject method.
 
                 _logger.Info<BackgroundTaskRunner>("{LogPrefix} Waiting for tasks to complete", _logPrefix);
-                Shutdown(false, false); // do not accept any more tasks, flush the queue, do not wait
 
-                // raise the completed event only after the running threading task has completed
-                lock (_locker)
+                try
                 {
-                    if (_runningTask != null)
-                        _runningTask.ContinueWith(_ => Terminate(false));
-                    else
-                        Terminate(false);
+                    Shutdown(false, false); // do not accept any more tasks, flush the queue, do not wait
                 }
+                finally
+                {
+                    // raise the completed event only after the running threading task has completed
+                    lock (_locker)
+                    {
+                        if (_runningTask != null)
+                            _runningTask.ContinueWith(_ => Terminate(false));
+                        else
+                            Terminate(false);
+                    }
+                }
+                
             }
             else
             {
@@ -719,8 +726,14 @@ namespace Umbraco.Web.Scheduling
                 // otherwise, its registration will be removed by the application manager.
 
                 _logger.Info<BackgroundTaskRunner>("{LogPrefix} Canceling tasks", _logPrefix);
-                Shutdown(true, true); // cancel all tasks, wait for the current one to end
-                Terminate(true);
+                try
+                {
+                    Shutdown(true, true); // cancel all tasks, wait for the current one to end
+                }
+                finally
+                {
+                    Terminate(true);
+                }
             }
         }
 
@@ -732,7 +745,13 @@ namespace Umbraco.Web.Scheduling
             // raise the Terminated event
             // complete the awaitable completion source, if any
 
-            HostingEnvironment.UnregisterObject(this);
+            if (immediate)
+            {
+                //only unregister when it's the final call, else we won't be notified of the final call
+                HostingEnvironment.UnregisterObject(this);
+            }
+
+            if (_terminated) return; // already taken care of
 
             TaskCompletionSource<int> terminatedSource;
             lock (_locker)
@@ -747,7 +766,7 @@ namespace Umbraco.Web.Scheduling
 
             OnEvent(Terminated, "Terminated");
 
-            terminatedSource.SetResult(0);
+            terminatedSource.TrySetResult(0);
         }
     }
 }