Merge branch 'v8/8.8' into v8/8.9

nul800sebastiaan · nul800sebastiaan · commit 58c6e655b312 · 2020-12-14T15:07:38.000+01:00
diff --git a/src/Umbraco.Core/Runtime/MainDom.cs b/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)
             {
diff --git a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs b/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;
@@ -49,19 +50,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
@@ -102,7 +107,8 @@ public async Task<bool> AcquireLockAsync(int millisecondsTimeout)
             }
             finally
             {
-                transaction.Complete();
+                db?.CompleteTransaction();
+                db?.Dispose();
             }
             
 
@@ -161,11 +167,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);
 
@@ -189,7 +195,8 @@ private void ListeningLoop()
                     }
                     finally
                     {
-                        transaction.Complete();
+                        db?.CompleteTransaction();
+                        db?.Dispose();
                     }
                 }
 
@@ -208,34 +215,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);
 
@@ -281,14 +301,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.
@@ -298,10 +322,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
@@ -324,7 +350,8 @@ private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)
             }
             finally
             {
-                transaction.Complete();
+                transaction?.Complete();
+                transaction?.Dispose();
             }
         }
 
@@ -375,11 +402,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);
 
@@ -406,7 +434,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.");
+                                }
                             }
                         }
                     }
diff --git a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-tabbed-content.html b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-tabbed-content.html
@@ -1,6 +1,6 @@
 ﻿<div>
     <ng-form name="tabbedContentForm">
-        <div class="umb-group-panel" retrive-dom-element="registerPropertyGroup(element[0], attributes.appAnchor)" data-app-anchor="{{group.id}}" data-element="group-{{group.alias}}" ng-repeat="group in content.tabs track by group.label">
+        <div class="umb-group-panel" retrive-dom-element="registerPropertyGroup(element[0], attributes.appAnchor)" data-app-anchor="{{group.id}}" data-element="group-{{group.alias}}" ng-repeat="group in content.tabs track by group.id">
 
             <div class="umb-group-panel__header">
                 <div id="group-{{group.id}}">{{ group.label }}</div>
diff --git a/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umb-element-editor-content.component.html b/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umb-element-editor-content.component.html
@@ -2,7 +2,7 @@
     <ng-form name="elementTypeContentForm">
         <div class="umb-group-panel"
              data-element="group-{{group.alias}}"
-             ng-repeat="group in vm.model.variants[0].tabs track by group.label">
+             ng-repeat="group in vm.model.variants[0].tabs track by group.id">
 
             <div class="umb-group-panel__header">
                 <div id="group-{{group.id}}">{{ group.label }}</div>
diff --git a/src/Umbraco.Web.UI.Client/src/views/components/member/umb-member-node-info.html b/src/Umbraco.Web.UI.Client/src/views/components/member/umb-member-node-info.html
@@ -1,7 +1,7 @@
 <div class="umb-package-details">
     <div class="umb-package-details__main-content">
 
-        <umb-box data-element="node-info-membership" ng-repeat="group in node.tabs| filter: {properties:{view:'readonlyvalue'}} track by group.label">
+        <umb-box data-element="node-info-membership" ng-repeat="group in node.tabs| filter: {properties:{view:'readonlyvalue'}} track by group.id">
 
             <div class="umb-group-panel__header">
                 <div>{{ group.label }}</div>
diff --git a/src/Umbraco.Web.UI.Client/src/views/media/apps/content/content.html b/src/Umbraco.Web.UI.Client/src/views/media/apps/content/content.html
@@ -1,5 +1,5 @@
 <div class="form-horizontal" ng-controller="Umbraco.Editors.Media.Apps.ContentController as vm">
-    <div class="umb-group-panel" data-element="group-{{group.alias}}" ng-repeat="group in content.tabs | filter: { hide : '!' + true } track by group.label">
+    <div class="umb-group-panel" data-element="group-{{group.alias}}" ng-repeat="group in content.tabs | filter: { hide : '!' + true } track by group.id">
 
         <div class="umb-group-panel__header">
             <div>{{ group.label }}</div>
diff --git a/src/Umbraco.Web.UI.Client/src/views/member/apps/content/content.html b/src/Umbraco.Web.UI.Client/src/views/member/apps/content/content.html
@@ -1,6 +1,6 @@
 <div class="form-horizontal" ng-controller="Umbraco.Editors.Member.Apps.ContentController as vm">
 
-    <div class="umb-group-panel" ng-repeat="group in content.tabs track by group.label">
+    <div class="umb-group-panel" ng-repeat="group in content.tabs track by group.id">
 
         <div class="umb-group-panel__header">
             <div>{{ group.label }}</div>

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`		`-using System;`
	`1`	`+using NPoco;`
	`2`	`+using System;`
`2`	`3`	`using System.Data;`
`3`	`4`	`using System.Data.SqlClient;`
`4`	`5`	`using System.Diagnostics;`
`@@ -49,19 +50,23 @@ public async Task<bool> AcquireLockAsync(int millisecondsTimeout)`
`49`	`50`	`}`
`50`	`51`
`51`	`52`	`if (!(_dbFactory.SqlContext.SqlSyntax is SqlServerSyntaxProvider sqlServerSyntaxProvider))`
	`53`	`+ {`
`52`	`54`	`throw new NotSupportedException("SqlMainDomLock is only supported for Sql Server");`
	`55`	`+ }`
`53`	`56`
`54`	`57`	`_sqlServerSyntax = sqlServerSyntaxProvider;`
`55`	`58`
`56`	`59`	`_logger.Debug<SqlMainDomLock>("Acquiring lock...");`
`57`	`60`
`58`	`61`	`var tempId = Guid.NewGuid().ToString();`
`59`	`62`
`60`		`- using var db = _dbFactory.CreateDatabase();`
`61`		`- using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
	`63`	`+ IUmbracoDatabase db = null;`
`62`	`64`
`63`	`65`	`try`
`64`	`66`	`{`
	`67`	`+ db = _dbFactory.CreateDatabase();`
	`68`	`+ db.BeginTransaction(IsolationLevel.ReadCommitted);`
	`69`	`+`
`65`	`70`	`try`
`66`	`71`	`{`
`67`	`72`	`// wait to get a write lock`
`@@ -102,7 +107,8 @@ public async Task<bool> AcquireLockAsync(int millisecondsTimeout)`
`102`	`107`	`}`
`103`	`108`	`finally`
`104`	`109`	`{`
`105`		`- transaction.Complete();`
	`110`	`+ db?.CompleteTransaction();`
	`111`	`+ db?.Dispose();`
`106`	`112`	`}`
`107`	`113`
`108`	`114`
`@@ -161,11 +167,11 @@ private void ListeningLoop()`
`161`	`167`	`// new MainDom will just take over.`
`162`	`168`	`if (_cancellationTokenSource.IsCancellationRequested)`
`163`	`169`	`return;`
`164`		`-`
`165`		`- using var db = _dbFactory.CreateDatabase();`
`166`		`- using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
	`170`	`+ IUmbracoDatabase db = null;`
`167`	`171`	`try`
`168`	`172`	`{`
	`173`	`+ db = _dbFactory.CreateDatabase();`
	`174`	`+ db.BeginTransaction(IsolationLevel.ReadCommitted);`
`169`	`175`	`// get a read lock`
`170`	`176`	`_sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom);`
`171`	`177`
`@@ -189,7 +195,8 @@ private void ListeningLoop()`
`189`	`195`	`}`
`190`	`196`	`finally`
`191`	`197`	`{`
`192`		`- transaction.Complete();`
	`198`	`+ db?.CompleteTransaction();`
	`199`	`+ db?.Dispose();`
`193`	`200`	`}`
`194`	`201`	`}`
`195`	`202`
`@@ -208,34 +215,47 @@ private Task<bool> WaitForExistingAsync(string tempId, int millisecondsTimeout)`
`208`	`215`
`209`	`216`	`return Task.Run(() =>`
`210`	`217`	`{`
`211`		`- using var db = _dbFactory.CreateDatabase();`
`212`		`-`
`213`		`- var watch = new Stopwatch();`
`214`		`- watch.Start();`
`215`		`- while (true)`
	`218`	`+ try`
`216`	`219`	`{`
`217`		`- // poll very often, we need to take over as fast as we can`
`218`		`- // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO`
`219`		`- Thread.Sleep(1000);`
`220`		`-`
`221`		`- var acquired = TryAcquire(db, tempId, updatedTempId);`
`222`		`- if (acquired.HasValue)`
`223`		`- return acquired.Value;`
	`220`	`+ using var db = _dbFactory.CreateDatabase();`
`224`	`221`
`225`		`- if (watch.ElapsedMilliseconds >= millisecondsTimeout)`
	`222`	`+ var watch = new Stopwatch();`
	`223`	`+ watch.Start();`
	`224`	`+ while (true)`
`226`	`225`	`{`
`227`		`- return AcquireWhenMaxWaitTimeElapsed(db);`
	`226`	`+ // poll very often, we need to take over as fast as we can`
	`227`	`+ // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO`
	`228`	`+ Thread.Sleep(1000);`
	`229`	`+`
	`230`	`+ var acquired = TryAcquire(db, tempId, updatedTempId);`
	`231`	`+ if (acquired.HasValue)`
	`232`	`+ return acquired.Value;`
	`233`	`+`
	`234`	`+ if (watch.ElapsedMilliseconds >= millisecondsTimeout)`
	`235`	`+ {`
	`236`	`+ return AcquireWhenMaxWaitTimeElapsed(db);`
	`237`	`+ }`
`228`	`238`	`}`
`229`	`239`	`}`
	`240`	`+ catch (Exception ex)`
	`241`	`+ {`
	`242`	`+ _logger.Error<SqlMainDomLock>(ex, "An error occurred trying to acquire and waiting for existing SqlMainDomLock to shutdown");`
	`243`	`+ return false;`
	`244`	`+ }`
	`245`	`+`
`230`	`246`	`}, _cancellationTokenSource.Token);`
`231`	`247`	`}`
`232`	`248`
`233`	`249`	`private bool? TryAcquire(IUmbracoDatabase db, string tempId, string updatedTempId)`
`234`	`250`	`{`
`235`		`- using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
	`251`	`+ // Creates a separate transaction to the DB instance so we aren't allocating tons of new DB instances for each transaction`
	`252`	`+ // since this is executed in a tight loop`
	`253`	`+`
	`254`	`+ ITransaction transaction = null;`
`236`	`255`
`237`	`256`	`try`
`238`	`257`	`{`
	`258`	`+ transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
`239`	`259`	`// get a read lock`
`240`	`260`	`_sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom);`
`241`	`261`
`@@ -281,14 +301,18 @@ private Task<bool> WaitForExistingAsync(string tempId, int millisecondsTimeout)`
`281`	`301`	`}`
`282`	`302`	`finally`
`283`	`303`	`{`
`284`		`- transaction.Complete();`
	`304`	`+ transaction?.Complete();`
	`305`	`+ transaction?.Dispose();`
`285`	`306`	`}`
`286`	`307`
`287`	`308`	`return null; // continue`
`288`	`309`	`}`
`289`	`310`
`290`	`311`	`private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)`
`291`	`312`	`{`
	`313`	`+ // Creates a separate transaction to the DB instance so we aren't allocating tons of new DB instances for each transaction`
	`314`	`+ // since this is executed in a tight loop`
	`315`	`+`
`292`	`316`	`// if the timeout has elapsed, it either means that the other main dom is taking too long to shutdown,`
`293`	`317`	`// or it could mean that the previous appdomain was terminated and didn't clear out the main dom SQL row`
`294`	`318`	`// and it's just been left as an orphan row.`
`@@ -298,10 +322,12 @@ private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)`
`298`	`322`
`299`	`323`	`_logger.Debug<SqlMainDomLock>("Timeout elapsed, assuming orphan row, acquiring MainDom.");`
`300`	`324`
`301`		`- using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
	`325`	`+ ITransaction transaction = null;`
`302`	`326`
`303`	`327`	`try`
`304`	`328`	`{`
	`329`	`+ transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
	`330`	`+`
`305`	`331`	`_sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom);`
`306`	`332`
`307`	`333`	`// so now we update the row with our appdomain id`
`@@ -324,7 +350,8 @@ private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db)`
`324`	`350`	`}`
`325`	`351`	`finally`
`326`	`352`	`{`
`327`		`- transaction.Complete();`
	`353`	`+ transaction?.Complete();`
	`354`	`+ transaction?.Dispose();`
`328`	`355`	`}`
`329`	`356`	`}`
`330`	`357`
`@@ -375,11 +402,12 @@ protected virtual void Dispose(bool disposing)`
`375`	`402`
`376`	`403`	`if (_dbFactory.Configured)`
`377`	`404`	`{`
`378`		`- using var db = _dbFactory.CreateDatabase();`
`379`		`- using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted);`
`380`		`-`
	`405`	`+ IUmbracoDatabase db = null;`
`381`	`406`	`try`
`382`	`407`	`{`
	`408`	`+ db = _dbFactory.CreateDatabase();`
	`409`	`+ db.BeginTransaction(IsolationLevel.ReadCommitted);`
	`410`	`+`
`383`	`411`	`// get a write lock`
`384`	`412`	`_sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom);`
`385`	`413`
`@@ -406,7 +434,15 @@ protected virtual void Dispose(bool disposing)`
`406`	`434`	`}`
`407`	`435`	`finally`
`408`	`436`	`{`
`409`		`- transaction.Complete();`
	`437`	`+ try`
	`438`	`+ {`
	`439`	`+ db?.CompleteTransaction();`
	`440`	`+ db?.Dispose();`
	`441`	`+ }`
	`442`	`+ catch (Exception ex)`
	`443`	`+ {`
	`444`	`+ _logger.Error<SqlMainDomLock>(ex, "Unexpected error during dispose when completing transaction.");`
	`445`	`+ }`
`410`	`446`	`}`
`411`	`447`	`}`
`412`	`448`	`}`