Merge pull request #9543 from umbraco/v8/bugfix/sqlmaindom-server-terminated

SqlMainDomLock will stop listening if Sql Server connection terminates

Author: bergmania

src/Umbraco.Core/Runtime/MainDom.cs

@@ -144,8 +144,16 @@ private bool Acquire()
 
             _logger.Info<MainDom>("Acquiring.");
 
-            // Get the lock 
-            var acquired = _mainDomLock.AcquireLockAsync(LockTimeoutMilliseconds).GetAwaiter().GetResult();
+            // Get the lock
+            var acquired = false;
+            try
+            {
+                acquired = _mainDomLock.AcquireLockAsync(LockTimeoutMilliseconds).GetAwaiter().GetResult();
+            }
+            catch (Exception ex)
+            {
+                _logger.Error<MainDom>(ex, "Error while acquiring");
+            }
 
             if (!acquired)
             {

src/Umbraco.Core/Runtime/SqlMainDomLock.cs

@@ -1,4 +1,5 @@
-using System;
+using NPoco;
+using System;
 using System.Data;
 using System.Data.SqlClient;
 using System.Diagnostics;
@@ -48,19 +49,23 @@ public async Task<bool> AcquireLockAsync(int millisecondsTimeout)
             }
 
             if (!(_dbFactory.SqlContext.SqlSyntax is SqlServerSyntaxProvider sqlServerSyntaxProvider))
+            {
                 throw new NotSupportedException("SqlMainDomLock is only supported for Sql Server");
+            }   
 
             _sqlServerSyntax = sqlServerSyntaxProvider;
 
             _logger.Debug<SqlMainDomLock>("Acquiring lock...");
 
             var tempId = Guid.NewGuid().ToString();
 
-            using var db = _dbFactory.CreateDatabase();
-            using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
+            IUmbracoDatabase db = null;
 
             try
             {
+                db = _dbFactory.CreateDatabase();
+                db.BeginTransaction(IsolationLevel.ReadCommitted);
+
                 try
                 {
                     // wait to get a write lock
@@ -101,7 +106,8 @@ public async Task<bool> AcquireLockAsync(int millisecondsTimeout)
             }
             finally
             {
-                transaction.Complete();
+                db?.CompleteTransaction();
+                db?.Dispose();
             }
 
 
@@ -154,11 +160,11 @@ private void ListeningLoop()
                     // new MainDom will just take over.
                     if (_cancellationTokenSource.IsCancellationRequested)
                         return;
-
-                    using var db = _dbFactory.CreateDatabase();
-                    using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
+                    IUmbracoDatabase db = null;
                     try
                     {
+                        db = _dbFactory.CreateDatabase();
+                        db.BeginTransaction(IsolationLevel.ReadCommitted);
                         // get a read lock
                         _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom);
 
@@ -182,7 +188,8 @@ private void ListeningLoop()
                     }
                     finally
                     {
-                        transaction.Complete();
+                        db?.CompleteTransaction();
+                        db?.Dispose();
                     }
                 }
 
@@ -201,34 +208,47 @@ private Task<bool> WaitForExistingAsync(string tempId, int millisecondsTimeout)
 
             return Task.Run(() =>
             {
-                using var db = _dbFactory.CreateDatabase();
-
-                var watch = new Stopwatch();
-                watch.Start();
-                while (true)
+                try
                 {
-                    // poll very often, we need to take over as fast as we can
-                    // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO
-                    Thread.Sleep(1000);
-
-                    var acquired = TryAcquire(db, tempId, updatedTempId);
-                    if (acquired.HasValue)
-                        return acquired.Value;
+                    using var db = _dbFactory.CreateDatabase();
 
-                    if (watch.ElapsedMilliseconds >= millisecondsTimeout)
+                    var watch = new Stopwatch();
+                    watch.Start();
+                    while (true)
                     {
-                        return AcquireWhenMaxWaitTimeElapsed(db);
+                        // poll very often, we need to take over as fast as we can
+                        // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO
+                        Thread.Sleep(1000);
+
+                        var acquired = TryAcquire(db, tempId, updatedTempId);
+                        if (acquired.HasValue)
+                            return acquired.Value;
+
+                        if (watch.ElapsedMilliseconds >= millisecondsTimeout)
+                        {
+                            return AcquireWhenMaxWaitTimeElapsed(db);
+                        }
                     }
                 }
+                catch (Exception ex)
+                {
+                    _logger.Error<SqlMainDomLock>(ex, "An error occurred trying to acquire and waiting for existing SqlMainDomLock to shutdown");
+                    return false;
+                }
+                
             }, _cancellationTokenSource.Token);
         }
 
         private bool? TryAcquire(IUmbracoDatabase db, string tempId, string updatedTempId)
         {
-            using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
+            // Creates a separate transaction to the DB instance so we aren't allocating tons of new DB instances for each transaction
+            // since this is executed in a tight loop
+
+            ITransaction transaction = null;
 
             try
             {
+                transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
                 // get a read lock
                 _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom);
 
@@ -274,14 +294,18 @@ private Task<bool> WaitForExistingAsync(string tempId, int millisecondsTimeout)
             }
             finally
             {
-                transaction.Complete();
+                transaction?.Complete();
+                transaction?.Dispose();
             }
 
             return null; // continue
         }
 
         private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)
         {
+            // Creates a separate transaction to the DB instance so we aren't allocating tons of new DB instances for each transaction
+            // since this is executed in a tight loop
+
             // if the timeout has elapsed, it either means that the other main dom is taking too long to shutdown,
             // or it could mean that the previous appdomain was terminated and didn't clear out the main dom SQL row
             // and it's just been left as an orphan row.
@@ -291,10 +315,12 @@ private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)
 
             _logger.Debug<SqlMainDomLock>("Timeout elapsed, assuming orphan row, acquiring MainDom.");
 
-            using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
+            ITransaction transaction = null;
 
             try
             {
+                transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
+                
                 _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom);
 
                 // so now we update the row with our appdomain id
@@ -317,7 +343,8 @@ private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)
             }
             finally
             {
-                transaction.Complete();
+                transaction?.Complete();
+                transaction?.Dispose();
             }
         }
 
@@ -368,11 +395,12 @@ protected virtual void Dispose(bool disposing)
 
                         if (_dbFactory.Configured)
                         {
-                            using var db = _dbFactory.CreateDatabase();
-                            using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);
-
+                            IUmbracoDatabase db = null;
                             try
                             {
+                                db = _dbFactory.CreateDatabase();
+                                db.BeginTransaction(IsolationLevel.ReadCommitted);
+
                                 // get a write lock
                                 _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom);
 
@@ -399,7 +427,15 @@ protected virtual void Dispose(bool disposing)
                             }
                             finally
                             {
-                                transaction.Complete();
+                                try
+                                {
+                                    db?.CompleteTransaction();
+                                    db?.Dispose();
+                                }
+                                catch (Exception ex)
+                                {
+                                    _logger.Error<SqlMainDomLock>(ex, "Unexpected error during dispose when completing transaction.");
+                                }
                             }
                         }
                     }