diff --git a/src/Umbraco.Core/Constants-AppSettings.cs b/src/Umbraco.Core/Constants-AppSettings.cs index 509be46b61..85d6b24ae0 100644 --- a/src/Umbraco.Core/Constants-AppSettings.cs +++ b/src/Umbraco.Core/Constants-AppSettings.cs @@ -9,6 +9,8 @@ namespace Umbraco.Core /// public static class AppSettings { + public const string MainDomLock = "Umbraco.Core.MainDom.Lock"; + // TODO: Kill me - still used in Umbraco.Core.IO.SystemFiles:27 [Obsolete("We need to kill this appsetting as we do not use XML content cache umbraco.config anymore due to NuCache")] public const string ContentXML = "Umbraco.Core.ContentXML"; //umbracoContentXML diff --git a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs index 888ff9a632..76a8b3a2f4 100644 --- a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs @@ -151,6 +151,8 @@ namespace Umbraco.Core.Migrations.Install _database.Insert(Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Constants.Locks.Domains, Name = "Domains" }); _database.Insert(Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Constants.Locks.KeyValues, Name = "KeyValues" }); _database.Insert(Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Constants.Locks.Languages, Name = "Languages" }); + + _database.Insert(Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Constants.Locks.MainDom, Name = "MainDom" }); } private void CreateContentTypeData() diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index dfd85b4cfc..302bf21700 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -188,6 +188,7 @@ namespace Umbraco.Core.Migrations.Upgrade To("{0BC866BC-0665-487A-9913-0290BD0169AD}"); To("{3D67D2C8-5E65-47D0-A9E1-DC2EE0779D6B}"); To("{EE288A91-531B-4995-8179-1D62D9AA3E2E}"); + To("{2AB29964-02A1-474D-BD6B-72148D2A53A2}"); //FINAL } diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddMainDomLock.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddMainDomLock.cs new file mode 100644 index 0000000000..6ca493ac7e --- /dev/null +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddMainDomLock.cs @@ -0,0 +1,16 @@ +using Umbraco.Core.Persistence.Dtos; + +namespace Umbraco.Core.Migrations.Upgrade.V_8_6_0 +{ + public class AddMainDomLock : MigrationBase + { + public AddMainDomLock(IMigrationContext context) + : base(context) + { } + + public override void Migrate() + { + Database.Insert(Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Constants.Locks.MainDom, Name = "MainDom" }); + } + } +} diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddPropertyTypeValidationMessageColumns.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddPropertyTypeValidationMessageColumns.cs index 30eb30109e..f44695da69 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddPropertyTypeValidationMessageColumns.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_6_0/AddPropertyTypeValidationMessageColumns.cs @@ -3,6 +3,7 @@ using Umbraco.Core.Persistence.Dtos; namespace Umbraco.Core.Migrations.Upgrade.V_8_6_0 { + public class AddPropertyTypeValidationMessageColumns : MigrationBase { public AddPropertyTypeValidationMessageColumns(IMigrationContext context) diff --git a/src/Umbraco.Core/Persistence/Constants-Locks.cs b/src/Umbraco.Core/Persistence/Constants-Locks.cs index 1dcd2408e7..e64f40ced7 100644 --- a/src/Umbraco.Core/Persistence/Constants-Locks.cs +++ b/src/Umbraco.Core/Persistence/Constants-Locks.cs @@ -8,6 +8,11 @@ namespace Umbraco.Core /// public static class Locks { + /// + /// The lock + /// + public const int MainDom = -1000; + /// /// All servers. /// diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs b/src/Umbraco.Core/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs index 3d0adf175e..bb50fa98a1 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs @@ -250,6 +250,11 @@ where tbl.[name]=@0 and col.[name]=@1;", tableName, columnName) } public override void WriteLock(IDatabase db, params int[] lockIds) + { + WriteLock(db, TimeSpan.FromMilliseconds(1800), lockIds); + } + + public void WriteLock(IDatabase db, TimeSpan timeout, params int[] lockIds) { // soon as we get Database, a transaction is started @@ -260,7 +265,7 @@ where tbl.[name]=@0 and col.[name]=@1;", tableName, columnName) // *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks foreach (var lockId in lockIds) { - db.Execute(@"SET LOCK_TIMEOUT 1800;"); + db.Execute($"SET LOCK_TIMEOUT {timeout.TotalMilliseconds};"); var i = db.Execute(@"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id", new { id = lockId }); if (i == 0) // ensure we are actually locking! throw new ArgumentException($"LockObject with id={lockId} does not exist."); diff --git a/src/Umbraco.Core/Runtime/CoreRuntime.cs b/src/Umbraco.Core/Runtime/CoreRuntime.cs index 5ceb89d7fb..b852aff2ff 100644 --- a/src/Umbraco.Core/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Core/Runtime/CoreRuntime.cs @@ -147,7 +147,7 @@ namespace Umbraco.Core.Runtime // TODO: remove this in netcore, this is purely backwards compat hacks with the empty ctor if (MainDom == null) { - MainDom = new MainDom(Logger); + MainDom = new MainDom(Logger, new MainDomSemaphoreLock(Logger)); } diff --git a/src/Umbraco.Core/IMainDom.cs b/src/Umbraco.Core/Runtime/IMainDom.cs similarity index 96% rename from src/Umbraco.Core/IMainDom.cs rename to src/Umbraco.Core/Runtime/IMainDom.cs index 31b2e2eee0..444fc1c7d0 100644 --- a/src/Umbraco.Core/IMainDom.cs +++ b/src/Umbraco.Core/Runtime/IMainDom.cs @@ -1,5 +1,6 @@ using System; +// TODO: Can't change namespace due to breaking changes, change in netcore namespace Umbraco.Core { /// diff --git a/src/Umbraco.Core/Runtime/IMainDomLock.cs b/src/Umbraco.Core/Runtime/IMainDomLock.cs new file mode 100644 index 0000000000..6a62f48194 --- /dev/null +++ b/src/Umbraco.Core/Runtime/IMainDomLock.cs @@ -0,0 +1,29 @@ +using System; +using System.Threading.Tasks; + +namespace Umbraco.Core.Runtime +{ + /// + /// An application-wide distributed lock + /// + /// + /// Disposing releases the lock + /// + public interface IMainDomLock : IDisposable + { + /// + /// Acquires an application-wide distributed lock + /// + /// + /// + /// An awaitable boolean value which will be false if the elapsed millsecondsTimeout value is exceeded + /// + Task AcquireLockAsync(int millisecondsTimeout); + + /// + /// Wait on a background thread to receive a signal from another AppDomain + /// + /// + Task ListenAsync(); + } +} diff --git a/src/Umbraco.Core/MainDom.cs b/src/Umbraco.Core/Runtime/MainDom.cs similarity index 76% rename from src/Umbraco.Core/MainDom.cs rename to src/Umbraco.Core/Runtime/MainDom.cs index e2049c0190..e6780ec876 100644 --- a/src/Umbraco.Core/MainDom.cs +++ b/src/Umbraco.Core/Runtime/MainDom.cs @@ -4,10 +4,13 @@ using System.Linq; using System.Security.Cryptography; using System.Threading; using System.Web.Hosting; +using Umbraco.Core; using Umbraco.Core.Logging; +using Umbraco.Core.Persistence; -namespace Umbraco.Core +namespace Umbraco.Core.Runtime { + /// /// Provides the full implementation of . /// @@ -20,18 +23,11 @@ namespace Umbraco.Core #region Vars private readonly ILogger _logger; + private readonly IMainDomLock _mainDomLock; // our own lock for local consistency private object _locko = new object(); - // async lock representing the main domain lock - private readonly SystemLock _systemLock; - private IDisposable _systemLocker; - - // event wait handle used to notify current main domain that it should - // release the lock because a new domain wants to be the main domain - private readonly EventWaitHandle _signal; - private bool _isInitialized; // indicates whether... private bool _isMainDom; // we are the main domain @@ -40,39 +36,19 @@ namespace Umbraco.Core // actions to run before releasing the main domain private readonly List> _callbacks = new List>(); - private const int LockTimeoutMilliseconds = 90000; // (1.5 * 60 * 1000) == 1 min 30 seconds + private const int LockTimeoutMilliseconds = 40000; // 40 seconds #endregion #region Ctor // initializes a new instance of MainDom - public MainDom(ILogger logger) + public MainDom(ILogger logger, IMainDomLock systemLock) { HostingEnvironment.RegisterObject(this); _logger = logger; - - // HostingEnvironment.ApplicationID is null in unit tests, making ReplaceNonAlphanumericChars fail - var appId = HostingEnvironment.ApplicationID?.ReplaceNonAlphanumericChars(string.Empty) ?? string.Empty; - - // combining with the physical path because if running on eg IIS Express, - // two sites could have the same appId even though they are different. - // - // now what could still collide is... two sites, running in two different processes - // and having the same appId, and running on the same app physical path - // - // we *cannot* use the process ID here because when an AppPool restarts it is - // a new process for the same application path - - var appPath = HostingEnvironment.ApplicationPhysicalPath?.ToLowerInvariant() ?? string.Empty; - var hash = (appId + ":::" + appPath).GenerateHash(); - - var lockName = "UMBRACO-" + hash + "-MAINDOM-LCK"; - _systemLock = new SystemLock(lockName); - - var eventName = "UMBRACO-" + hash + "-MAINDOM-EVT"; - _signal = new EventWaitHandle(false, EventResetMode.AutoReset, eventName); + _mainDomLock = systemLock; } #endregion @@ -141,13 +117,14 @@ namespace Umbraco.Core continue; } } + _logger.Debug("Stopped ({SignalSource})", source); } finally { // in any case... _isMainDom = false; - _systemLocker?.Dispose(); + _mainDomLock.Dispose(); _logger.Info("Released ({SignalSource})", source); } @@ -167,36 +144,33 @@ namespace Umbraco.Core _logger.Info("Acquiring."); - // signal other instances that we want the lock, then wait one the lock, - // which may timeout, and this is accepted - see comments below + // Get the lock + var acquired = _mainDomLock.AcquireLockAsync(LockTimeoutMilliseconds).GetAwaiter().GetResult(); - // signal, then wait for the lock, then make sure the event is - // reset (maybe there was noone listening..) - _signal.Set(); + if (!acquired) + { + _logger.Info("Cannot acquire (timeout)."); - // if more than 1 instance reach that point, one will get the lock - // and the other one will timeout, which is accepted + // In previous versions we'd let a TimeoutException be thrown + // and the appdomain would not start. We have the opportunity to allow it to + // start without having MainDom? This would mean that it couldn't write + // to nucache/examine and would only be ok if this was a super short lived appdomain. + // maybe safer to just keep throwing in this case. + + throw new TimeoutException("Cannot acquire MainDom"); + // return false; + } - //This can throw a TimeoutException - in which case should this be in a try/finally to ensure the signal is always reset. try { - _systemLocker = _systemLock.Lock(LockTimeoutMilliseconds); + // Listen for the signal from another AppDomain coming online to release the lock + _mainDomLock.ListenAsync().ContinueWith(_ => OnSignal("signal")); } - finally + catch (OperationCanceledException ex) { - // we need to reset the event, because otherwise we would end up - // signaling ourselves and committing suicide immediately. - // only 1 instance can reach that point, but other instances may - // have started and be trying to get the lock - they will timeout, - // which is accepted - - _signal.Reset(); + // the waiting task could be canceled if this appdomain is naturally shutting down, we'll just swallow this exception + _logger.Warn(ex, ex.Message); } - - //WaitOneAsync (ext method) will wait for a signal without blocking the main thread, the waiting is done on a background thread - - _signal.WaitOneAsync() - .ContinueWith(_ => OnSignal("signal")); _logger.Info("Acquired."); return true; @@ -205,6 +179,10 @@ namespace Umbraco.Core /// /// Gets a value indicating whether the current domain is the main domain. /// + /// + /// The lazy initializer call will only call the Acquire callback when it's not been initialized, else it will just return + /// the value from _isMainDom which means when we set _isMainDom to false again after being signaled, this will return false; + /// public bool IsMainDom => LazyInitializer.EnsureInitialized(ref _isMainDom, ref _isInitialized, ref _locko, () => Acquire()); // IRegisteredObject @@ -230,8 +208,7 @@ namespace Umbraco.Core { if (disposing) { - _signal?.Close(); - _signal?.Dispose(); + _mainDomLock.Dispose(); } disposedValue = true; @@ -244,5 +221,25 @@ namespace Umbraco.Core } #endregion + + public static string GetMainDomId() + { + // HostingEnvironment.ApplicationID is null in unit tests, making ReplaceNonAlphanumericChars fail + var appId = HostingEnvironment.ApplicationID?.ReplaceNonAlphanumericChars(string.Empty) ?? string.Empty; + + // combining with the physical path because if running on eg IIS Express, + // two sites could have the same appId even though they are different. + // + // now what could still collide is... two sites, running in two different processes + // and having the same appId, and running on the same app physical path + // + // we *cannot* use the process ID here because when an AppPool restarts it is + // a new process for the same application path + + var appPath = HostingEnvironment.ApplicationPhysicalPath?.ToLowerInvariant() ?? string.Empty; + var hash = (appId + ":::" + appPath).GenerateHash(); + + return hash; + } } } diff --git a/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs b/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs new file mode 100644 index 0000000000..2dcc37e25f --- /dev/null +++ b/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs @@ -0,0 +1,95 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Umbraco.Core.Logging; + +namespace Umbraco.Core.Runtime +{ + /// + /// Uses a system-wide Semaphore and EventWaitHandle to synchronize the current AppDomain + /// + internal class MainDomSemaphoreLock : IMainDomLock + { + private readonly SystemLock _systemLock; + + // event wait handle used to notify current main domain that it should + // release the lock because a new domain wants to be the main domain + private readonly EventWaitHandle _signal; + private readonly ILogger _logger; + private IDisposable _lockRelease; + + public MainDomSemaphoreLock(ILogger logger) + { + var lockName = "UMBRACO-" + MainDom.GetMainDomId() + "-MAINDOM-LCK"; + _systemLock = new SystemLock(lockName); + + var eventName = "UMBRACO-" + MainDom.GetMainDomId() + "-MAINDOM-EVT"; + _signal = new EventWaitHandle(false, EventResetMode.AutoReset, eventName); + _logger = logger; + } + + //WaitOneAsync (ext method) will wait for a signal without blocking the main thread, the waiting is done on a background thread + public Task ListenAsync() => _signal.WaitOneAsync(); + + public Task AcquireLockAsync(int millisecondsTimeout) + { + // signal other instances that we want the lock, then wait on the lock, + // which may timeout, and this is accepted - see comments below + + // signal, then wait for the lock, then make sure the event is + // reset (maybe there was noone listening..) + _signal.Set(); + + // if more than 1 instance reach that point, one will get the lock + // and the other one will timeout, which is accepted + + //This can throw a TimeoutException - in which case should this be in a try/finally to ensure the signal is always reset. + try + { + _lockRelease = _systemLock.Lock(millisecondsTimeout); + return Task.FromResult(true); + } + catch (TimeoutException ex) + { + _logger.Error(ex); + return Task.FromResult(false); + } + finally + { + // we need to reset the event, because otherwise we would end up + // signaling ourselves and committing suicide immediately. + // only 1 instance can reach that point, but other instances may + // have started and be trying to get the lock - they will timeout, + // which is accepted + + _signal.Reset(); + } + } + + #region IDisposable Support + private bool disposedValue = false; // To detect redundant calls + + protected virtual void Dispose(bool disposing) + { + if (!disposedValue) + { + if (disposing) + { + _lockRelease?.Dispose(); + _signal.Close(); + _signal.Dispose(); + } + + disposedValue = true; + } + } + + // This code added to correctly implement the disposable pattern. + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + Dispose(true); + } + #endregion + } +} diff --git a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs new file mode 100644 index 0000000000..4433a8e307 --- /dev/null +++ b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs @@ -0,0 +1,413 @@ +using System; +using System.Data; +using System.Data.SqlClient; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Umbraco.Core.Logging; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Dtos; +using Umbraco.Core.Persistence.Mappers; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Runtime +{ + internal class SqlMainDomLock : IMainDomLock + { + private string _lockId; + private const string MainDomKey = "Umbraco.Core.Runtime.SqlMainDom"; + private const string UpdatedSuffix = "_updated"; + private readonly ILogger _logger; + private IUmbracoDatabase _db; + private CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); + private SqlServerSyntaxProvider _sqlServerSyntax = new SqlServerSyntaxProvider(); + private bool _mainDomChanging = false; + private readonly UmbracoDatabaseFactory _dbFactory; + private bool _hasError; + private object _locker = new object(); + + public SqlMainDomLock(ILogger logger) + { + // unique id for our appdomain, this is more unique than the appdomain id which is just an INT counter to its safer + _lockId = Guid.NewGuid().ToString(); + _logger = logger; + _dbFactory = new UmbracoDatabaseFactory( + Constants.System.UmbracoConnectionName, + _logger, + new Lazy(() => new Persistence.Mappers.MapperCollection(Enumerable.Empty()))); + } + + public async Task AcquireLockAsync(int millisecondsTimeout) + { + if (!(_dbFactory.SqlContext.SqlSyntax is SqlServerSyntaxProvider sqlServerSyntaxProvider)) + throw new NotSupportedException("SqlMainDomLock is only supported for Sql Server"); + + _sqlServerSyntax = sqlServerSyntaxProvider; + + _logger.Debug("Acquiring lock..."); + + var db = GetDatabase(); + + var tempId = Guid.NewGuid().ToString(); + + try + { + db.BeginTransaction(IsolationLevel.ReadCommitted); + + try + { + // wait to get a write lock + _sqlServerSyntax.WriteLock(db, TimeSpan.FromMilliseconds(millisecondsTimeout), Constants.Locks.MainDom); + } + catch (Exception ex) + { + if (IsLockTimeoutException(ex)) + { + _logger.Error(ex, "Sql timeout occurred, could not acquire MainDom."); + _hasError = true; + return false; + } + + // unexpected (will be caught below) + throw; + } + + var result = InsertLockRecord(tempId); //we change the row to a random Id to signal other MainDom to shutdown + if (result == RecordPersistenceType.Insert) + { + // if we've inserted, then there was no MainDom so we can instantly acquire + + // TODO: see the other TODO, could we just delete the row and that would indicate that we + // are MainDom? then we don't leave any orphan rows behind. + + InsertLockRecord(_lockId); // so update with our appdomain id + _logger.Debug("Acquired with ID {LockId}", _lockId); + return true; + } + + // if we've updated, this means there is an active MainDom, now we need to wait to + // for the current MainDom to shutdown which also requires releasing our write lock + } + catch (Exception ex) + { + ResetDatabase(); + // unexpected + _logger.Error(ex, "Unexpected error, cannot acquire MainDom"); + _hasError = true; + return false; + } + finally + { + db?.CompleteTransaction(); + } + + return await WaitForExistingAsync(tempId, millisecondsTimeout); + } + + public Task ListenAsync() + { + if (_hasError) + { + _logger.Warn("Could not acquire MainDom, listening is canceled."); + return Task.CompletedTask; + } + + // 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); + + } + + private void ListeningLoop() + { + while (true) + { + // poll every 1 second + Thread.Sleep(1000); + + lock (_locker) + { + // If cancellation has been requested we will just exit. Depending on timing of the shutdown, + // we will have already flagged _mainDomChanging = true, or we're shutting down faster than + // 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) + return; + + var db = GetDatabase(); + + try + { + db.BeginTransaction(IsolationLevel.ReadCommitted); + + // get a read lock + _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); + + // TODO: We could in theory just check if the main dom row doesn't exist, that could indicate that + // we are still the maindom. An empty value might be better because then we won't have any orphan rows + // if the app is terminated. Could that work? + + if (!IsMainDomValue(_lockId)) + { + // we are no longer main dom, another one has come online, exit + _mainDomChanging = true; + _logger.Debug("Detected new booting application, releasing MainDom lock."); + return; + } + } + catch (Exception ex) + { + ResetDatabase(); + // unexpected + _logger.Error(ex, "Unexpected error, listening is canceled."); + _hasError = true; + return; + } + finally + { + db?.CompleteTransaction(); + } + } + + } + } + + private void ResetDatabase() + { + if (_db.InTransaction) + _db.AbortTransaction(); + _db.Dispose(); + _db = null; + } + + private IUmbracoDatabase GetDatabase() + { + if (_db != null) + return _db; + + _db = _dbFactory.CreateDatabase(); + return _db; + } + + /// + /// Wait for any existing MainDom to release so we can continue booting + /// + /// + /// + /// + private Task WaitForExistingAsync(string tempId, int millisecondsTimeout) + { + var updatedTempId = tempId + UpdatedSuffix; + + return Task.Run(() => + { + var db = GetDatabase(); + var watch = new Stopwatch(); + watch.Start(); + while(true) + { + // poll very often, we need to take over as fast as we can + Thread.Sleep(100); + + try + { + db.BeginTransaction(IsolationLevel.ReadCommitted); + + // get a read lock + _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); + + // the row + var mainDomRows = db.Fetch("SELECT * FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); + + if (mainDomRows.Count == 0 || mainDomRows[0].Value == updatedTempId) + { + // the other main dom has updated our record + // Or the other maindom shutdown super fast and just deleted the record + // which indicates that we + // can acquire it and it has shutdown. + + _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); + + // so now we update the row with our appdomain id + InsertLockRecord(_lockId); + _logger.Debug("Acquired with ID {LockId}", _lockId); + return true; + } + else if (mainDomRows.Count == 1 && !mainDomRows[0].Value.StartsWith(tempId)) + { + // in this case, the prefixed ID is different which means + // another new AppDomain has come online and is wanting to take over. In that case, we will not + // acquire. + + _logger.Debug("Cannot acquire, another booting application detected."); + + return false; + } + } + catch (Exception ex) + { + ResetDatabase(); + + if (IsLockTimeoutException(ex)) + { + _logger.Error(ex, "Sql timeout occurred, waiting for existing MainDom is canceled."); + _hasError = true; + return false; + } + // unexpected + _logger.Error(ex, "Unexpected error, waiting for existing MainDom is canceled."); + _hasError = true; + return false; + } + finally + { + db?.CompleteTransaction(); + } + + if (watch.ElapsedMilliseconds >= millisecondsTimeout) + { + // 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. + // There's really know way of knowing unless we are constantly updating the row for the current maindom + // which isn't ideal. + // So... we're going to 'just' take over, if the writelock works then we'll assume we're ok + + _logger.Debug("Timeout elapsed, assuming orphan row, acquiring MainDom."); + + try + { + db.BeginTransaction(IsolationLevel.ReadCommitted); + + _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); + + // so now we update the row with our appdomain id + InsertLockRecord(_lockId); + _logger.Debug("Acquired with ID {LockId}", _lockId); + return true; + } + catch (Exception ex) + { + ResetDatabase(); + + if (IsLockTimeoutException(ex)) + { + // something is wrong, we cannot acquire, not much we can do + _logger.Error(ex, "Sql timeout occurred, could not forcibly acquire MainDom."); + _hasError = true; + return false; + } + _logger.Error(ex, "Unexpected error, could not forcibly acquire MainDom."); + _hasError = true; + return false; + } + finally + { + db?.CompleteTransaction(); + } + } + } + }, _cancellationTokenSource.Token); + } + + /// + /// Inserts or updates the key/value row + /// + private RecordPersistenceType InsertLockRecord(string id) + { + var db = GetDatabase(); + return db.InsertOrUpdate(new KeyValueDto + { + Key = MainDomKey, + Value = id, + Updated = DateTime.Now + }); + } + + /// + /// Checks if the DB row value is equals the value + /// + /// + private bool IsMainDomValue(string val) + { + var db = GetDatabase(); + return db.ExecuteScalar("SELECT COUNT(*) FROM umbracoKeyValue WHERE [key] = @key AND [value] = @val", + new { key = MainDomKey, val = val }) == 1; + } + + /// + /// Checks if the exception is an SQL timeout + /// + /// + /// + private bool IsLockTimeoutException(Exception exception) => exception is SqlException sqlException && sqlException.Number == 1222; + + #region IDisposable Support + private bool _disposedValue = false; // To detect redundant calls + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + lock (_locker) + { + // immediately cancel all sub-tasks, we don't want them to keep querying + _cancellationTokenSource.Cancel(); + _cancellationTokenSource.Dispose(); + + var db = GetDatabase(); + try + { + db.BeginTransaction(IsolationLevel.ReadCommitted); + + // get a write lock + _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); + + // When we are disposed, it means we have released the MainDom lock + // and called all MainDom release callbacks, in this case + // if another maindom is actually coming online we need + // to signal to the MainDom coming online that we have shutdown. + // To do that, we update the existing main dom DB record with a suffixed "_updated" string. + // Otherwise, if we are just shutting down, we want to just delete the row. + if (_mainDomChanging) + { + _logger.Debug("Releasing MainDom, updating row, new application is booting."); + db.Execute($"UPDATE umbracoKeyValue SET [value] = [value] + '{UpdatedSuffix}' WHERE [key] = @key", new { key = MainDomKey }); + } + else + { + _logger.Debug("Releasing MainDom, deleting row, application is shutting down."); + db.Execute("DELETE FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); + } + } + catch (Exception ex) + { + ResetDatabase(); + _logger.Error(ex, "Unexpected error during dipsose."); + _hasError = true; + } + finally + { + db?.CompleteTransaction(); + ResetDatabase(); + } + } + } + + _disposedValue = true; + } + } + + // This code added to correctly implement the disposable pattern. + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + Dispose(true); + } + #endregion + + } +} diff --git a/src/Umbraco.Core/Scoping/IScopeProvider.cs b/src/Umbraco.Core/Scoping/IScopeProvider.cs index 6c9eb63ba0..96bb939f8e 100644 --- a/src/Umbraco.Core/Scoping/IScopeProvider.cs +++ b/src/Umbraco.Core/Scoping/IScopeProvider.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Scoping /// Provides scopes. /// public interface IScopeProvider - { + { /// /// Creates an ambient scope. /// diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 69c84eeeed..a8e3fc2988 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -128,6 +128,10 @@ --> + + + + @@ -410,7 +414,7 @@ - + @@ -741,7 +745,7 @@ - + diff --git a/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs b/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs index b435af9e77..00ba721bdb 100644 --- a/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs +++ b/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs @@ -223,7 +223,7 @@ namespace Umbraco.Tests.Cache { var d = new SnapDictionary(); d.Test.CollectAuto = false; - + // gen 1 d.Set(1, "one"); Assert.AreEqual(1, d.Test.GetValues(1).Length); @@ -321,7 +321,7 @@ namespace Umbraco.Tests.Cache { var d = new SnapDictionary(); d.Test.CollectAuto = false; - + Assert.AreEqual(0, d.Test.GetValues(1).Length); // gen 1 @@ -416,7 +416,7 @@ namespace Umbraco.Tests.Cache { var d = new SnapDictionary(); d.Test.CollectAuto = false; - + // gen 1 d.Set(1, "one"); Assert.AreEqual(1, d.Test.GetValues(1).Length); @@ -578,7 +578,7 @@ namespace Umbraco.Tests.Cache { var d = new SnapDictionary(); d.Test.CollectAuto = false; - + d.Set(1, "one"); d.Set(2, "two"); @@ -689,7 +689,7 @@ namespace Umbraco.Tests.Cache { // gen 3 Assert.AreEqual(2, d.Test.GetValues(1).Length); - d.Set(1, "ein"); + d.SetLocked(1, "ein"); Assert.AreEqual(3, d.Test.GetValues(1).Length); Assert.AreEqual(3, d.Test.LiveGen); @@ -727,31 +727,25 @@ namespace Umbraco.Tests.Cache using (var w1 = d.GetScopedWriteLock(scopeProvider)) { Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(1, t.WLocked); + Assert.IsTrue(t.IsLocked); Assert.IsTrue(t.NextGen); - using (var w2 = d.GetScopedWriteLock(scopeProvider)) + Assert.Throws(() => { - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(2, t.WLocked); - Assert.IsTrue(t.NextGen); - - Assert.AreNotSame(w1, w2); // get a new writer each time - - d.Set(1, "one"); - - Assert.AreEqual(0, d.CreateSnapshot().Gen); - } + using (var w2 = d.GetScopedWriteLock(scopeProvider)) + { + } + }); Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(1, t.WLocked); + Assert.IsTrue(t.IsLocked); Assert.IsTrue(t.NextGen); Assert.AreEqual(0, d.CreateSnapshot().Gen); } Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(0, t.WLocked); + Assert.IsFalse(t.IsLocked); Assert.IsTrue(t.NextGen); Assert.AreEqual(1, d.CreateSnapshot().Gen); @@ -772,11 +766,14 @@ namespace Umbraco.Tests.Cache using (var w1 = d.GetScopedWriteLock(scopeProvider)) { + // This one is interesting, although we don't allow recursive locks, since this is + // using the same ScopeContext/key, the lock acquisition is only done once + using (var w2 = d.GetScopedWriteLock(scopeProvider)) { Assert.AreSame(w1, w2); - d.Set(1, "one"); + d.SetLocked(1, "one"); } } } @@ -797,19 +794,16 @@ namespace Umbraco.Tests.Cache using (var w1 = d.GetScopedWriteLock(scopeProvider1)) { Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(1, t.WLocked); + Assert.IsTrue(t.IsLocked); Assert.IsTrue(t.NextGen); - using (var w2 = d.GetScopedWriteLock(scopeProvider2)) + Assert.Throws(() => { - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(2, t.WLocked); - Assert.IsTrue(t.NextGen); + using (var w2 = d.GetScopedWriteLock(scopeProvider2)) + { + } + }); - Assert.AreNotSame(w1, w2); - - d.Set(1, "one"); - } } } @@ -848,13 +842,13 @@ namespace Umbraco.Tests.Cache Assert.IsFalse(d.Test.NextGen); Assert.AreEqual("uno", s2.Get(1)); - var scopeProvider = GetScopeProvider(); + var scopeProvider = GetScopeProvider(); using (d.GetScopedWriteLock(scopeProvider)) { // gen 3 Assert.AreEqual(2, d.Test.GetValues(1).Length); - d.Set(1, "ein"); + d.SetLocked(1, "ein"); Assert.AreEqual(3, d.Test.GetValues(1).Length); Assert.AreEqual(3, d.Test.LiveGen); @@ -881,6 +875,7 @@ namespace Umbraco.Tests.Cache { var d = new SnapDictionary(); d.Test.CollectAuto = false; + // gen 1 d.Set(1, "one"); @@ -894,12 +889,11 @@ namespace Umbraco.Tests.Cache Assert.AreEqual("uno", s2.Get(1)); var scopeProvider = GetScopeProvider(); - using (d.GetScopedWriteLock(scopeProvider)) { // creating a snapshot in a write-lock does NOT return the "current" content // it uses the previous snapshot, so new snapshot created only on release - d.Set(1, "ein"); + d.SetLocked(1, "ein"); var s3 = d.CreateSnapshot(); Assert.AreEqual(2, s3.Gen); Assert.AreEqual("uno", s3.Get(1)); @@ -934,12 +928,11 @@ namespace Umbraco.Tests.Cache var scopeContext = new ScopeContext(); var scopeProvider = GetScopeProvider(scopeContext); - using (d.GetScopedWriteLock(scopeProvider)) { // creating a snapshot in a write-lock does NOT return the "current" content // it uses the previous snapshot, so new snapshot created only on release - d.Set(1, "ein"); + d.SetLocked(1, "ein"); var s3 = d.CreateSnapshot(); Assert.AreEqual(2, s3.Gen); Assert.AreEqual("uno", s3.Get(1)); @@ -967,7 +960,7 @@ namespace Umbraco.Tests.Cache var d = new SnapDictionary(); var t = d.Test; t.CollectAuto = false; - + // gen 1 d.Set(1, "one"); var s1 = d.CreateSnapshot(); @@ -984,12 +977,11 @@ namespace Umbraco.Tests.Cache var scopeContext = new ScopeContext(); var scopeProvider = GetScopeProvider(scopeContext); - using (d.GetScopedWriteLock(scopeProvider)) { // creating a snapshot in a write-lock does NOT return the "current" content // it uses the previous snapshot, so new snapshot created only on release - d.Set(1, "ein"); + d.SetLocked(1, "ein"); var s3 = d.CreateSnapshot(); Assert.AreEqual(2, s3.Gen); Assert.AreEqual("uno", s3.Get(1)); @@ -997,7 +989,7 @@ namespace Umbraco.Tests.Cache // we made some changes, so a next gen is required Assert.AreEqual(3, t.LiveGen); Assert.IsTrue(t.NextGen); - Assert.AreEqual(1, t.WLocked); + Assert.IsTrue(t.IsLocked); // but live snapshot contains changes var ls = t.LiveSnapshot; @@ -1008,7 +1000,7 @@ namespace Umbraco.Tests.Cache // nothing is committed until scope exits Assert.AreEqual(3, t.LiveGen); Assert.IsTrue(t.NextGen); - Assert.AreEqual(1, t.WLocked); + Assert.IsTrue(t.IsLocked); // no changes until exit var s4 = d.CreateSnapshot(); @@ -1020,7 +1012,7 @@ namespace Umbraco.Tests.Cache // now things have changed Assert.AreEqual(2, t.LiveGen); Assert.IsFalse(t.NextGen); - Assert.AreEqual(0, t.WLocked); + Assert.IsFalse(t.IsLocked); // no changes since not completed var s5 = d.CreateSnapshot(); @@ -1097,9 +1089,10 @@ namespace Umbraco.Tests.Cache // writer is scope contextual and scoped // when disposed, nothing happens // when the context exists, the writer is released + using (d.GetScopedWriteLock(scopeProvider)) { - d.Set(1, "ein"); + d.SetLocked(1, "ein"); Assert.IsTrue(d.Test.NextGen); Assert.AreEqual(3, d.Test.LiveGen); Assert.IsNotNull(d.Test.GenObj); @@ -1107,7 +1100,7 @@ namespace Umbraco.Tests.Cache } // writer has not released - Assert.AreEqual(1, d.Test.WLocked); + Assert.IsTrue(d.Test.IsLocked); Assert.IsNotNull(d.Test.GenObj); Assert.AreEqual(2, d.Test.GenObj.Gen); @@ -1118,7 +1111,7 @@ namespace Umbraco.Tests.Cache // panic! var s2 = d.CreateSnapshot(); - Assert.AreEqual(1, d.Test.WLocked); + Assert.IsTrue(d.Test.IsLocked); Assert.IsNotNull(d.Test.GenObj); Assert.AreEqual(2, d.Test.GenObj.Gen); Assert.AreEqual(3, d.Test.LiveGen); @@ -1127,7 +1120,7 @@ namespace Umbraco.Tests.Cache // release writer scopeContext.ScopeExit(true); - Assert.AreEqual(0, d.Test.WLocked); + Assert.IsFalse(d.Test.IsLocked); Assert.IsNotNull(d.Test.GenObj); Assert.AreEqual(2, d.Test.GenObj.Gen); Assert.AreEqual(3, d.Test.LiveGen); @@ -1135,7 +1128,7 @@ namespace Umbraco.Tests.Cache var s3 = d.CreateSnapshot(); - Assert.AreEqual(0, d.Test.WLocked); + Assert.IsFalse(d.Test.IsLocked); Assert.IsNotNull(d.Test.GenObj); Assert.AreEqual(3, d.Test.GenObj.Gen); Assert.AreEqual(3, d.Test.LiveGen); @@ -1150,4 +1143,45 @@ namespace Umbraco.Tests.Cache return scopeProvider; } } + + /// + /// Used for tests so that we don't have to wrap every Set/Clear call in locks + /// + public static class SnapDictionaryExtensions + { + internal static void Set(this SnapDictionary d, TKey key, TValue value) + where TValue : class + { + using (d.GetScopedWriteLock(GetScopeProvider())) + { + d.SetLocked(key, value); + } + } + + internal static void Clear(this SnapDictionary d) + where TValue : class + { + using (d.GetScopedWriteLock(GetScopeProvider())) + { + d.ClearLocked(); + } + } + + internal static void Clear(this SnapDictionary d, TKey key) + where TValue : class + { + using (d.GetScopedWriteLock(GetScopeProvider())) + { + d.ClearLocked(key); + } + } + + private static IScopeProvider GetScopeProvider() + { + var scopeProvider = Mock.Of(); + Mock.Get(scopeProvider) + .Setup(x => x.Context).Returns(() => null); + return scopeProvider; + } + } } diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs index ec6b854a46..97fe9057bb 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Persistence.Repositories; +using Umbraco.Core.Runtime; using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Web; diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs index c452c4792a..803b86aec5 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStore.cs @@ -14,6 +14,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.Repositories.Implement; +using Umbraco.Core.Runtime; using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Services.Changes; diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 3857a49a28..f92d8adebb 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -14,12 +14,24 @@ using Umbraco.Web.PublishedCache.NuCache.Snap; namespace Umbraco.Web.PublishedCache.NuCache { - // stores content + /// + /// Stores content in memory and persists it back to disk + /// + /// + /// + /// Methods in this class suffixed with the term "Locked" means that those methods can only be called within a WriteLock. A WriteLock + /// is acquired by the GetScopedWriteLock method. Locks are not allowed to be recursive. + /// + /// + /// This class's logic is based on the class but has been slightly modified to suit these purposes. + /// + /// internal class ContentStore { // this class is an extended version of SnapDictionary // most of the snapshots management code, etc is an exact copy // SnapDictionary has unit tests to ensure it all works correctly + // For locking information, see SnapDictionary private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; private readonly IVariationContextAccessor _variationContextAccessor; @@ -38,7 +50,6 @@ namespace Umbraco.Web.PublishedCache.NuCache private long _liveGen, _floorGen; private bool _nextGen, _collectAuto; private Task _collectTask; - private volatile int _wlocked; private List> _wchanges; // TODO: collection trigger (ok for now) @@ -79,15 +90,9 @@ namespace Umbraco.Web.PublishedCache.NuCache private readonly string _instanceId = Guid.NewGuid().ToString("N"); - private class ReadLockInfo - { - public bool Taken; - } - private class WriteLockInfo { public bool Taken; - public bool Count; } // a scope contextual that represents a locked writer to the dictionary @@ -118,22 +123,26 @@ namespace Umbraco.Web.PublishedCache.NuCache return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped)); } + private void EnsureLocked() + { + if (!Monitor.IsEntered(_wlocko)) + throw new InvalidOperationException("Write lock must be acquried."); + } + private void Lock(WriteLockInfo lockInfo, bool forceGen = false) { + if (Monitor.IsEntered(_wlocko)) + throw new InvalidOperationException("Recursive locks not allowed"); + Monitor.Enter(_wlocko, ref lockInfo.Taken); - var rtaken = false; - try + lock(_rlocko) { - Monitor.Enter(_rlocko, ref rtaken); - // see SnapDictionary try { } finally { - _wlocked++; - lockInfo.Count = true; - if (_nextGen == false || (forceGen && _wlocked == 1)) + if (_nextGen == false || (forceGen)) { // because we are changing things, a new generation // is created, which will trigger a new snapshot @@ -143,63 +152,49 @@ namespace Umbraco.Web.PublishedCache.NuCache _nextGen = true; } } - } - finally - { - if (rtaken) Monitor.Exit(_rlocko); - } - } - - private void Lock(ReadLockInfo lockInfo) - { - Monitor.Enter(_rlocko, ref lockInfo.Taken); + } } private void Release(WriteLockInfo lockInfo, bool commit = true) { - if (commit == false) + try { - var rtaken = false; - try + if (commit == false) { - Monitor.Enter(_rlocko, ref rtaken); - try { } - finally + lock (_rlocko) { - _nextGen = false; - _liveGen -= 1; + // see SnapDictionary + try { } + finally + { + _nextGen = false; + _liveGen -= 1; + } } - } - finally - { - if (rtaken) Monitor.Exit(_rlocko); - } - Rollback(_contentNodes); - RollbackRoot(); - Rollback(_contentTypesById); - Rollback(_contentTypesByAlias); + Rollback(_contentNodes); + RollbackRoot(); + Rollback(_contentTypesById); + Rollback(_contentTypesByAlias); + } + else if (_localDb != null && _wchanges != null) + { + foreach (var change in _wchanges) + { + if (change.Value.IsNull) + _localDb.TryRemove(change.Key, out ContentNodeKit unused); + else + _localDb[change.Key] = change.Value; + } + _wchanges = null; + _localDb.Commit(); + } } - else if (_localDb != null && _wchanges != null) + finally { - foreach (var change in _wchanges) - { - if (change.Value.IsNull) - _localDb.TryRemove(change.Key, out ContentNodeKit unused); - else - _localDb[change.Key] = change.Value; - } - _wchanges = null; - _localDb.Commit(); + if (lockInfo.Taken) + Monitor.Exit(_wlocko); } - - if (lockInfo.Count) _wlocked--; - if (lockInfo.Taken) Monitor.Exit(_wlocko); - } - - private void Release(ReadLockInfo lockInfo) - { - if (lockInfo.Taken) Monitor.Exit(_rlocko); } private void RollbackRoot() @@ -237,7 +232,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { try { - // Trying to lock could throw exceptions so always make sure to clean up. + // Trying to lock could throw exceptions so always make sure to clean up. Lock(lockInfo); } finally @@ -246,7 +241,11 @@ namespace Umbraco.Web.PublishedCache.NuCache { _localDb?.Dispose(); } - catch { /* TBD: May already be throwing so don't throw again */} + catch (Exception ex) + { + /* TBD: May already be throwing so don't throw again */ + _logger.Error(ex, "Error trying to release DB"); + } finally { _localDb = null; @@ -254,6 +253,11 @@ namespace Umbraco.Web.PublishedCache.NuCache } } + catch (Exception ex) + { + _logger.Error(ex, "Error trying to lock"); + throw; + } finally { Release(lockInfo); @@ -270,87 +274,111 @@ namespace Umbraco.Web.PublishedCache.NuCache #region Content types - public void NewContentTypes(IEnumerable types) + /// + /// Sets data for new content types + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public void NewContentTypesLocked(IEnumerable types) { - var lockInfo = new WriteLockInfo(); - try - { - Lock(lockInfo); + EnsureLocked(); - foreach (var type in types) - { - SetValueLocked(_contentTypesById, type.Id, type); - SetValueLocked(_contentTypesByAlias, type.Alias, type); - } - } - finally + foreach (var type in types) { - Release(lockInfo); + SetValueLocked(_contentTypesById, type.Id, type); + SetValueLocked(_contentTypesByAlias, type.Alias, type); } } - public void UpdateContentTypes(IEnumerable types) + /// + /// Sets data for updated content types + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public void UpdateContentTypesLocked(IEnumerable types) { //nothing to do if this is empty, no need to lock/allocate/iterate/etc... if (!types.Any()) return; - var lockInfo = new WriteLockInfo(); - try + EnsureLocked(); + + var index = types.ToDictionary(x => x.Id, x => x); + + foreach (var type in index.Values) { - Lock(lockInfo); - - var index = types.ToDictionary(x => x.Id, x => x); - - foreach (var type in index.Values) - { - SetValueLocked(_contentTypesById, type.Id, type); - SetValueLocked(_contentTypesByAlias, type.Alias, type); - } - - foreach (var link in _contentNodes.Values) - { - var node = link.Value; - if (node == null) continue; - var contentTypeId = node.ContentType.Id; - if (index.TryGetValue(contentTypeId, out var contentType) == false) continue; - SetValueLocked(_contentNodes, node.Id, new ContentNode(node, contentType)); - } + SetValueLocked(_contentTypesById, type.Id, type); + SetValueLocked(_contentTypesByAlias, type.Alias, type); } - finally + + foreach (var link in _contentNodes.Values) { - Release(lockInfo); + var node = link.Value; + if (node == null) continue; + var contentTypeId = node.ContentType.Id; + if (index.TryGetValue(contentTypeId, out var contentType) == false) continue; + SetValueLocked(_contentNodes, node.Id, new ContentNode(node, contentType)); } } - public void SetAllContentTypes(IEnumerable types) + /// + /// Updates/sets data for all content types + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public void SetAllContentTypesLocked(IEnumerable types) { - var lockInfo = new WriteLockInfo(); - try + EnsureLocked(); + + // clear all existing content types + ClearLocked(_contentTypesById); + ClearLocked(_contentTypesByAlias); + + // set all new content types + foreach (var type in types) { - Lock(lockInfo); - - // clear all existing content types - ClearLocked(_contentTypesById); - ClearLocked(_contentTypesByAlias); - - // set all new content types - foreach (var type in types) - { - SetValueLocked(_contentTypesById, type.Id, type); - SetValueLocked(_contentTypesByAlias, type.Alias, type); - } - - // beware! at that point the cache is inconsistent, - // assuming we are going to SetAll content items! - } - finally - { - Release(lockInfo); + SetValueLocked(_contentTypesById, type.Id, type); + SetValueLocked(_contentTypesByAlias, type.Alias, type); } + + // beware! at that point the cache is inconsistent, + // assuming we are going to SetAll content items! } - public void UpdateContentTypes(IReadOnlyCollection removedIds, IReadOnlyCollection refreshedTypes, IReadOnlyCollection kits) + /// + /// Updates/sets/removes data for content types + /// + /// + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public void UpdateContentTypesLocked(IReadOnlyCollection removedIds, IReadOnlyCollection refreshedTypes, IReadOnlyCollection kits) { + EnsureLocked(); + var removedIdsA = removedIds ?? Array.Empty(); var refreshedTypesA = refreshedTypes ?? Array.Empty(); var refreshedIdsA = refreshedTypesA.Select(x => x.Id).ToList(); @@ -359,85 +387,84 @@ namespace Umbraco.Web.PublishedCache.NuCache if (kits.Count == 0 && refreshedIdsA.Count == 0 && removedIdsA.Count == 0) return; //exit - there is nothing to do here - var lockInfo = new WriteLockInfo(); - try + var removedContentTypeNodes = new List(); + var refreshedContentTypeNodes = new List(); + + // find all the nodes that are either refreshed or removed, + // because of their content type being either refreshed or removed + foreach (var link in _contentNodes.Values) { - Lock(lockInfo); - - var removedContentTypeNodes = new List(); - var refreshedContentTypeNodes = new List(); - - // find all the nodes that are either refreshed or removed, - // because of their content type being either refreshed or removed - foreach (var link in _contentNodes.Values) - { - var node = link.Value; - if (node == null) continue; - var contentTypeId = node.ContentType.Id; - if (removedIdsA.Contains(contentTypeId)) removedContentTypeNodes.Add(node.Id); - if (refreshedIdsA.Contains(contentTypeId)) refreshedContentTypeNodes.Add(node.Id); - } - - // perform deletion of content with removed content type - // removing content types should have removed their content already - // but just to be 100% sure, clear again here - foreach (var node in removedContentTypeNodes) - ClearBranchLocked(node); - - // perform deletion of removed content types - foreach (var id in removedIdsA) - { - if (_contentTypesById.TryGetValue(id, out var link) == false || link.Value == null) - continue; - SetValueLocked(_contentTypesById, id, null); - SetValueLocked(_contentTypesByAlias, link.Value.Alias, null); - } - - // perform update of refreshed content types - foreach (var type in refreshedTypesA) - { - SetValueLocked(_contentTypesById, type.Id, type); - SetValueLocked(_contentTypesByAlias, type.Alias, type); - } - - // perform update of content with refreshed content type - from the kits - // skip missing type, skip missing parents & un-buildable kits - what else could we do? - // kits are ordered by level, so ParentExists is ok here - var visited = new List(); - foreach (var kit in kits.Where(x => - refreshedIdsA.Contains(x.ContentTypeId) && - BuildKit(x, out _))) - { - // replacing the node: must preserve the parents - var node = GetHead(_contentNodes, kit.Node.Id)?.Value; - if (node != null) - kit.Node.FirstChildContentId = node.FirstChildContentId; - - SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); - - visited.Add(kit.Node.Id); - if (_localDb != null) RegisterChange(kit.Node.Id, kit); - } - - // all content should have been refreshed - but... - var orphans = refreshedContentTypeNodes.Except(visited); - foreach (var id in orphans) - ClearBranchLocked(id); + var node = link.Value; + if (node == null) continue; + var contentTypeId = node.ContentType.Id; + if (removedIdsA.Contains(contentTypeId)) removedContentTypeNodes.Add(node.Id); + if (refreshedIdsA.Contains(contentTypeId)) refreshedContentTypeNodes.Add(node.Id); } - finally + + // perform deletion of content with removed content type + // removing content types should have removed their content already + // but just to be 100% sure, clear again here + foreach (var node in removedContentTypeNodes) + ClearBranchLocked(node); + + // perform deletion of removed content types + foreach (var id in removedIdsA) { - Release(lockInfo); + if (_contentTypesById.TryGetValue(id, out var link) == false || link.Value == null) + continue; + SetValueLocked(_contentTypesById, id, null); + SetValueLocked(_contentTypesByAlias, link.Value.Alias, null); } + + // perform update of refreshed content types + foreach (var type in refreshedTypesA) + { + SetValueLocked(_contentTypesById, type.Id, type); + SetValueLocked(_contentTypesByAlias, type.Alias, type); + } + + // perform update of content with refreshed content type - from the kits + // skip missing type, skip missing parents & un-buildable kits - what else could we do? + // kits are ordered by level, so ParentExists is ok here + var visited = new List(); + foreach (var kit in kits.Where(x => + refreshedIdsA.Contains(x.ContentTypeId) && + BuildKit(x, out _))) + { + // replacing the node: must preserve the parents + var node = GetHead(_contentNodes, kit.Node.Id)?.Value; + if (node != null) + kit.Node.FirstChildContentId = node.FirstChildContentId; + + SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); + + visited.Add(kit.Node.Id); + if (_localDb != null) RegisterChange(kit.Node.Id, kit); + } + + // all content should have been refreshed - but... + var orphans = refreshedContentTypeNodes.Except(visited); + foreach (var id in orphans) + ClearBranchLocked(id); } - public void UpdateDataTypes(IEnumerable dataTypeIds, Func getContentType) + /// + /// Updates data types + /// + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public void UpdateDataTypesLocked(IEnumerable dataTypeIds, Func getContentType) { - var lockInfo = new WriteLockInfo(); - try - { - Lock(lockInfo); + EnsureLocked(); - var contentTypes = _contentTypesById + var contentTypes = _contentTypesById .Where(kvp => kvp.Value.Value != null && kvp.Value.Value.PropertyTypes.Any(p => dataTypeIds.Contains(p.DataType.Id))) @@ -446,37 +473,32 @@ namespace Umbraco.Web.PublishedCache.NuCache .Where(x => x != null) // poof, gone, very unlikely and probably an anomaly .ToArray(); - var contentTypeIdsA = contentTypes.Select(x => x.Id).ToArray(); - var contentTypeNodes = new Dictionary>(); - foreach (var id in contentTypeIdsA) - contentTypeNodes[id] = new List(); - foreach (var link in _contentNodes.Values) - { - var node = link.Value; - if (node != null && contentTypeIdsA.Contains(node.ContentType.Id)) - contentTypeNodes[node.ContentType.Id].Add(node.Id); - } - - foreach (var contentType in contentTypes) - { - // again, weird situation - if (contentTypeNodes.ContainsKey(contentType.Id) == false) - continue; - - foreach (var id in contentTypeNodes[contentType.Id]) - { - _contentNodes.TryGetValue(id, out var link); - if (link?.Value == null) - continue; - var node = new ContentNode(link.Value, contentType); - SetValueLocked(_contentNodes, id, node); - if (_localDb != null) RegisterChange(id, node.ToKit()); - } - } - } - finally + var contentTypeIdsA = contentTypes.Select(x => x.Id).ToArray(); + var contentTypeNodes = new Dictionary>(); + foreach (var id in contentTypeIdsA) + contentTypeNodes[id] = new List(); + foreach (var link in _contentNodes.Values) { - Release(lockInfo); + var node = link.Value; + if (node != null && contentTypeIdsA.Contains(node.ContentType.Id)) + contentTypeNodes[node.ContentType.Id].Add(node.Id); + } + + foreach (var contentType in contentTypes) + { + // again, weird situation + if (contentTypeNodes.ContainsKey(contentType.Id) == false) + continue; + + foreach (var id in contentTypeNodes[contentType.Id]) + { + _contentNodes.TryGetValue(id, out var link); + if (link?.Value == null) + continue; + var node = new ContentNode(link.Value, contentType); + SetValueLocked(_contentNodes, id, node); + if (_localDb != null) RegisterChange(id, node.ToKit()); + } } } @@ -534,8 +556,22 @@ namespace Umbraco.Web.PublishedCache.NuCache return link; } - public bool Set(ContentNodeKit kit) + /// + /// Sets the data for a + /// + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public bool SetLocked(ContentNodeKit kit) { + EnsureLocked(); + // ReSharper disable LocalizableElement if (kit.IsEmpty) throw new ArgumentException("Kit is empty.", nameof(kit)); @@ -545,57 +581,47 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Set content ID: {KitNodeId}", kit.Node.Id); - var lockInfo = new WriteLockInfo(); - try + // get existing + _contentNodes.TryGetValue(kit.Node.Id, out var link); + var existing = link?.Value; + + if (!BuildKit(kit, out var parent)) + return false; + + // moving? + var moving = existing != null && existing.ParentContentId != kit.Node.ParentContentId; + + // manage children + if (existing != null) { - Lock(lockInfo); - - // get existing - _contentNodes.TryGetValue(kit.Node.Id, out var link); - var existing = link?.Value; - - if (!BuildKit(kit, out var parent)) - return false; - - // moving? - var moving = existing != null && existing.ParentContentId != kit.Node.ParentContentId; - - // manage children - if (existing != null) - { - kit.Node.FirstChildContentId = existing.FirstChildContentId; - kit.Node.LastChildContentId = existing.LastChildContentId; - } - - // set - SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); - if (_localDb != null) RegisterChange(kit.Node.Id, kit); - - // manage the tree - if (existing == null) - { - // new, add to parent - AddTreeNodeLocked(kit.Node, parent); - } - else if (moving || existing.SortOrder != kit.Node.SortOrder) - { - // moved, remove existing from its parent, add content to its parent - RemoveTreeNodeLocked(existing); - AddTreeNodeLocked(kit.Node); - } - else - { - // replacing existing, handle siblings - kit.Node.NextSiblingContentId = existing.NextSiblingContentId; - kit.Node.PreviousSiblingContentId = existing.PreviousSiblingContentId; - } - - _xmap[kit.Node.Uid] = kit.Node.Id; + kit.Node.FirstChildContentId = existing.FirstChildContentId; + kit.Node.LastChildContentId = existing.LastChildContentId; } - finally + + // set + SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); + if (_localDb != null) RegisterChange(kit.Node.Id, kit); + + // manage the tree + if (existing == null) { - Release(lockInfo); + // new, add to parent + AddTreeNodeLocked(kit.Node, parent); } + else if (moving || existing.SortOrder != kit.Node.SortOrder) + { + // moved, remove existing from its parent, add content to its parent + RemoveTreeNodeLocked(existing); + AddTreeNodeLocked(kit.Node); + } + else + { + // replacing existing, handle siblings + kit.Node.NextSiblingContentId = existing.NextSiblingContentId; + kit.Node.PreviousSiblingContentId = existing.PreviousSiblingContentId; + } + + _xmap[kit.Node.Uid] = kit.Node.Id; return true; } @@ -617,195 +643,216 @@ namespace Umbraco.Web.PublishedCache.NuCache /// True if the data is coming from the database (not the local cache db) /// /// + /// /// This requires that the collection is sorted by Level + ParentId + Sort Order. /// This should be used only on a site startup as the first generations. /// This CANNOT be used after startup since it bypasses all checks for Generations. + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// /// - internal bool SetAllFastSorted(IEnumerable kits, bool fromDb) + /// + /// Thrown if this method is not called within a write lock + /// + public bool SetAllFastSortedLocked(IEnumerable kits, bool fromDb) { - var lockInfo = new WriteLockInfo(); + EnsureLocked(); + var ok = true; - try + + ClearLocked(_contentNodes); + ClearRootLocked(); + + // The name of the game here is to populate each kit's + // FirstChildContentId + // LastChildContentId + // NextSiblingContentId + // PreviousSiblingContentId + + ContentNode previousNode = null; + ContentNode parent = null; + + foreach (var kit in kits) { - Lock(lockInfo); - - ClearLocked(_contentNodes); - ClearRootLocked(); - - // The name of the game here is to populate each kit's - // FirstChildContentId - // LastChildContentId - // NextSiblingContentId - // PreviousSiblingContentId - - ContentNode previousNode = null; - ContentNode parent = null; - - foreach (var kit in kits) + if (!BuildKit(kit, out var parentLink)) { - if (!BuildKit(kit, out var parentLink)) - { - ok = false; - continue; // skip that one - } - - var thisNode = kit.Node; - - if (parent == null) - { - // first parent - parent = parentLink.Value; - parent.FirstChildContentId = thisNode.Id; // this node is the first node - } - else if (parent.Id != parentLink.Value.Id) - { - // new parent - parent = parentLink.Value; - parent.FirstChildContentId = thisNode.Id; // this node is the first node - previousNode = null; // there is no previous sibling - } - - _logger.Debug($"Set {thisNode.Id} with parent {thisNode.ParentContentId}"); - SetValueLocked(_contentNodes, thisNode.Id, thisNode); - - // if we are initializing from the database source ensure the local db is updated - if (fromDb && _localDb != null) RegisterChange(thisNode.Id, kit); - - // this node is always the last child - parent.LastChildContentId = thisNode.Id; - - // wire previous node as previous sibling - if (previousNode != null) - { - previousNode.NextSiblingContentId = thisNode.Id; - thisNode.PreviousSiblingContentId = previousNode.Id; - } - - // this node becomes the previous node - previousNode = thisNode; - - _xmap[kit.Node.Uid] = kit.Node.Id; + ok = false; + continue; // skip that one } - } - finally - { - Release(lockInfo); + + var thisNode = kit.Node; + + if (parent == null) + { + // first parent + parent = parentLink.Value; + parent.FirstChildContentId = thisNode.Id; // this node is the first node + } + else if (parent.Id != parentLink.Value.Id) + { + // new parent + parent = parentLink.Value; + parent.FirstChildContentId = thisNode.Id; // this node is the first node + previousNode = null; // there is no previous sibling + } + + _logger.Debug($"Set {thisNode.Id} with parent {thisNode.ParentContentId}"); + SetValueLocked(_contentNodes, thisNode.Id, thisNode); + + // if we are initializing from the database source ensure the local db is updated + if (fromDb && _localDb != null) RegisterChange(thisNode.Id, kit); + + // this node is always the last child + parent.LastChildContentId = thisNode.Id; + + // wire previous node as previous sibling + if (previousNode != null) + { + previousNode.NextSiblingContentId = thisNode.Id; + thisNode.PreviousSiblingContentId = previousNode.Id; + } + + // this node becomes the previous node + previousNode = thisNode; + + _xmap[kit.Node.Uid] = kit.Node.Id; } return ok; } - public bool SetAll(IEnumerable kits) + /// + /// Set all data for a collection of + /// + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public bool SetAllLocked(IEnumerable kits) { - var lockInfo = new WriteLockInfo(); + EnsureLocked(); + var ok = true; - try + + ClearLocked(_contentNodes); + ClearRootLocked(); + + // do NOT clear types else they are gone! + //ClearLocked(_contentTypesById); + //ClearLocked(_contentTypesByAlias); + + foreach (var kit in kits) { - Lock(lockInfo); - - ClearLocked(_contentNodes); - ClearRootLocked(); - - // do NOT clear types else they are gone! - //ClearLocked(_contentTypesById); - //ClearLocked(_contentTypesByAlias); - - foreach (var kit in kits) + if (!BuildKit(kit, out var parent)) { - if (!BuildKit(kit, out var parent)) - { - ok = false; - continue; // skip that one - } - _logger.Debug($"Set {kit.Node.Id} with parent {kit.Node.ParentContentId}"); - SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); - - if (_localDb != null) RegisterChange(kit.Node.Id, kit); - AddTreeNodeLocked(kit.Node, parent); - - _xmap[kit.Node.Uid] = kit.Node.Id; + ok = false; + continue; // skip that one } - } - finally - { - Release(lockInfo); + _logger.Debug($"Set {kit.Node.Id} with parent {kit.Node.ParentContentId}"); + SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); + + if (_localDb != null) RegisterChange(kit.Node.Id, kit); + AddTreeNodeLocked(kit.Node, parent); + + _xmap[kit.Node.Uid] = kit.Node.Id; } return ok; } - // IMPORTANT kits must be sorted out by LEVEL and by SORT ORDER - public bool SetBranch(int rootContentId, IEnumerable kits) + /// + /// Sets data for a branch of + /// + /// + /// + /// + /// + /// + /// IMPORTANT kits must be sorted out by LEVEL and by SORT ORDER + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// + /// Thrown if this method is not called within a write lock + /// + public bool SetBranchLocked(int rootContentId, IEnumerable kits) { - var lockInfo = new WriteLockInfo(); + EnsureLocked(); + var ok = true; - try + + // get existing + _contentNodes.TryGetValue(rootContentId, out var link); + var existing = link?.Value; + + // clear + if (existing != null) { - Lock(lockInfo); - - // get existing - _contentNodes.TryGetValue(rootContentId, out var link); - var existing = link?.Value; - - // clear - if (existing != null) - { - //this zero's out the branch (recursively), if we're in a new gen this will add a NULL placeholder for the gen - ClearBranchLocked(existing); - //TODO: This removes the current GEN from the tree - do we really want to do that? - RemoveTreeNodeLocked(existing); - } - - // now add them all back - foreach (var kit in kits) - { - if (!BuildKit(kit, out var parent)) - { - ok = false; - continue; // skip that one - } - SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); - if (_localDb != null) RegisterChange(kit.Node.Id, kit); - AddTreeNodeLocked(kit.Node, parent); - - _xmap[kit.Node.Uid] = kit.Node.Id; - } + //this zero's out the branch (recursively), if we're in a new gen this will add a NULL placeholder for the gen + ClearBranchLocked(existing); + //TODO: This removes the current GEN from the tree - do we really want to do that? + RemoveTreeNodeLocked(existing); } - finally + + // now add them all back + foreach (var kit in kits) { - Release(lockInfo); + if (!BuildKit(kit, out var parent)) + { + ok = false; + continue; // skip that one + } + SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); + if (_localDb != null) RegisterChange(kit.Node.Id, kit); + AddTreeNodeLocked(kit.Node, parent); + + _xmap[kit.Node.Uid] = kit.Node.Id; } return ok; } - public bool Clear(int id) + /// + /// Clears data for a given node id + /// + /// + /// + /// + /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock + /// otherwise an exception will occur. + /// + /// + /// Thrown if this method is not called within a write lock + /// + public bool ClearLocked(int id) { - var lockInfo = new WriteLockInfo(); - try - { - Lock(lockInfo); + EnsureLocked(); - // try to find the content - // if it is not there, nothing to do - _contentNodes.TryGetValue(id, out var link); // else null - if (link?.Value == null) return false; + // try to find the content + // if it is not there, nothing to do + _contentNodes.TryGetValue(id, out var link); // else null + if (link?.Value == null) return false; - var content = link.Value; - _logger.Debug("Clear content ID: {ContentId}", content.Id); + var content = link.Value; + _logger.Debug("Clear content ID: {ContentId}", content.Id); - // clear the entire branch - ClearBranchLocked(content); + // clear the entire branch + ClearBranchLocked(content); - // manage the tree - RemoveTreeNodeLocked(content); + // manage the tree + RemoveTreeNodeLocked(content); - return true; - } - finally - { - Release(lockInfo); - } + return true; } private void ClearBranchLocked(int id) @@ -1204,11 +1251,8 @@ namespace Umbraco.Web.PublishedCache.NuCache public Snapshot CreateSnapshot() { - var lockInfo = new ReadLockInfo(); - try + lock(_rlocko) { - Lock(lockInfo); - // if no next generation is required, and we already have one, // use it and create a new snapshot if (_nextGen == false && _genObj != null) @@ -1221,7 +1265,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // else we need to try to create a new gen ref // whether we are wlocked or not, noone can rlock while we do, // so _liveGen and _nextGen are safe - if (_wlocked > 0) // volatile, cannot ++ but could -- + if (Monitor.IsEntered(_wlocko)) { // write-locked, cannot use latest gen (at least 1) so use previous var snapGen = _nextGen ? _liveGen - 1 : _liveGen; @@ -1260,10 +1304,6 @@ namespace Umbraco.Web.PublishedCache.NuCache return snapshot; } - finally - { - Release(lockInfo); - } } public Snapshot LiveSnapshot => new Snapshot(this, _liveGen @@ -1381,16 +1421,17 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - public async Task WaitForPendingCollect() - { - Task task; - lock (_rlocko) - { - task = _collectTask; - } - if (task != null) - await task; - } + // TODO: This is never used? Should it be? Maybe move to TestHelper below? + //public async Task WaitForPendingCollect() + //{ + // Task task; + // lock (_rlocko) + // { + // task = _collectTask; + // } + // if (task != null) + // await task; + //} public long GenCount => _genObjs.Count; diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index e0b95e8481..833b0a3665 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; +using System.Threading; using CSharpTest.Net.Collections; using Newtonsoft.Json; using Umbraco.Core; @@ -144,7 +145,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _domainStore = new SnapDictionary(); - LoadCachesOnStartup(); + LoadCachesOnStartup(); } Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? default; @@ -165,14 +166,14 @@ namespace Umbraco.Web.PublishedCache.NuCache /// to not run if MainDom wasn't acquired. /// If MainDom was not acquired, then _localContentDb and _localMediaDb will remain null which means this appdomain /// will load in published content via the DB and in that case this appdomain will probably not exist long enough to - /// serve more than a page of content. + /// serve more than a page of content. /// private void MainDomRegister() { var path = GetLocalFilesPath(); var localContentDbPath = Path.Combine(path, "NuCache.Content.db"); var localMediaDbPath = Path.Combine(path, "NuCache.Media.db"); - + _localContentDbExists = File.Exists(localContentDbPath); _localMediaDbExists = File.Exists(localMediaDbPath); @@ -191,10 +192,15 @@ namespace Umbraco.Web.PublishedCache.NuCache /// private void MainDomRelease() { + _logger.Debug("Releasing from MainDom..."); + lock (_storesLock) { + _logger.Debug("Releasing content store..."); _contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned _localContentDb = null; + + _logger.Debug("Releasing media store..."); _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned _localMediaDb = null; @@ -217,7 +223,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { okContent = LockAndLoadContent(scope => LoadContentFromLocalDbLocked(true)); if (!okContent) - _logger.Warn("Loading content from local db raised warnings, will reload from database."); + _logger.Warn("Loading content from local db raised warnings, will reload from database."); } if (_localMediaDbExists) @@ -378,7 +384,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var contentTypes = _serviceContext.ContentTypeService.GetAll() .Select(x => _publishedContentTypeFactory.CreateContentType(x)); - _contentStore.SetAllContentTypes(contentTypes); + _contentStore.SetAllContentTypesLocked(contentTypes); using (_logger.TraceDuration("Loading content from database")) { @@ -389,7 +395,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // IMPORTANT GetAllContentSources sorts kits by level + parentId + sortOrder var kits = _dataSource.GetAllContentSources(scope); - return onStartup ? _contentStore.SetAllFastSorted(kits, true) : _contentStore.SetAll(kits); + return onStartup ? _contentStore.SetAllFastSortedLocked(kits, true) : _contentStore.SetAllLocked(kits); } } @@ -397,7 +403,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { var contentTypes = _serviceContext.ContentTypeService.GetAll() .Select(x => _publishedContentTypeFactory.CreateContentType(x)); - _contentStore.SetAllContentTypes(contentTypes); + _contentStore.SetAllContentTypesLocked(contentTypes); using (_logger.TraceDuration("Loading content from local cache file")) { @@ -449,7 +455,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var mediaTypes = _serviceContext.MediaTypeService.GetAll() .Select(x => _publishedContentTypeFactory.CreateContentType(x)); - _mediaStore.SetAllContentTypes(mediaTypes); + _mediaStore.SetAllContentTypesLocked(mediaTypes); using (_logger.TraceDuration("Loading media from database")) { @@ -461,7 +467,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Loading media from database..."); // IMPORTANT GetAllMediaSources sorts kits by level + parentId + sortOrder var kits = _dataSource.GetAllMediaSources(scope); - return onStartup ? _mediaStore.SetAllFastSorted(kits, true) : _mediaStore.SetAll(kits); + return onStartup ? _mediaStore.SetAllFastSortedLocked(kits, true) : _mediaStore.SetAllLocked(kits); } } @@ -469,7 +475,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { var mediaTypes = _serviceContext.MediaTypeService.GetAll() .Select(x => _publishedContentTypeFactory.CreateContentType(x)); - _mediaStore.SetAllContentTypes(mediaTypes); + _mediaStore.SetAllContentTypesLocked(mediaTypes); using (_logger.TraceDuration("Loading media from local cache file")) { @@ -510,7 +516,7 @@ namespace Umbraco.Web.PublishedCache.NuCache return false; } - return onStartup ? store.SetAllFastSorted(kits, false) : store.SetAll(kits); + return onStartup ? store.SetAllFastSortedLocked(kits, false) : store.SetAllLocked(kits); } // keep these around - might be useful @@ -616,7 +622,7 @@ namespace Umbraco.Web.PublishedCache.NuCache .Where(x => x.RootContentId.HasValue && x.LanguageIsoCode.IsNullOrWhiteSpace() == false) .Select(x => new Domain(x.Id, x.DomainName, x.RootContentId.Value, CultureInfo.GetCultureInfo(x.LanguageIsoCode), x.IsWildcard))) { - _domainStore.Set(domain.Id, domain); + _domainStore.SetLocked(domain.Id, domain); } } @@ -664,10 +670,12 @@ namespace Umbraco.Web.PublishedCache.NuCache publishedChanged = publishedChanged2; } + if (draftChanged || publishedChanged) ((PublishedSnapshot)CurrentPublishedSnapshot)?.Resync(); } + // Calling this method means we have a lock on the contentStore (i.e. GetScopedWriteLock) private void NotifyLocked(IEnumerable payloads, out bool draftChanged, out bool publishedChanged) { publishedChanged = false; @@ -677,7 +685,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // content (and content types) are read-locked while reading content // contentStore is wlocked (so readable, only no new views) // and it can be wlocked by 1 thread only at a time - // contentStore is write-locked during changes + // contentStore is write-locked during changes - see note above, calls to this method are wrapped in contentStore.GetScopedWriteLock foreach (var payload in payloads) { @@ -697,7 +705,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (payload.ChangeTypes.HasType(TreeChangeTypes.Remove)) { - if (_contentStore.Clear(payload.Id)) + if (_contentStore.ClearLocked(payload.Id)) draftChanged = publishedChanged = true; continue; } @@ -720,7 +728,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // ?? should we do some RV check here? // IMPORTANT GetbranchContentSources sorts kits by level and by sort order var kits = _dataSource.GetBranchContentSources(scope, capture.Id); - _contentStore.SetBranch(capture.Id, kits); + _contentStore.SetBranchLocked(capture.Id, kits); } else { @@ -728,11 +736,11 @@ namespace Umbraco.Web.PublishedCache.NuCache var kit = _dataSource.GetContentSource(scope, capture.Id); if (kit.IsEmpty) { - _contentStore.Clear(capture.Id); + _contentStore.ClearLocked(capture.Id); } else { - _contentStore.Set(kit); + _contentStore.SetLocked(kit); } } @@ -790,7 +798,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (payload.ChangeTypes.HasType(TreeChangeTypes.Remove)) { - if (_mediaStore.Clear(payload.Id)) + if (_mediaStore.ClearLocked(payload.Id)) anythingChanged = true; continue; } @@ -813,7 +821,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // ?? should we do some RV check here? // IMPORTANT GetbranchContentSources sorts kits by level and by sort order var kits = _dataSource.GetBranchMediaSources(scope, capture.Id); - _mediaStore.SetBranch(capture.Id, kits); + _mediaStore.SetBranchLocked(capture.Id, kits); } else { @@ -821,11 +829,11 @@ namespace Umbraco.Web.PublishedCache.NuCache var kit = _dataSource.GetMediaSource(scope, capture.Id); if (kit.IsEmpty) { - _mediaStore.Clear(capture.Id); + _mediaStore.ClearLocked(capture.Id); } else { - _mediaStore.Set(kit); + _mediaStore.SetLocked(kit); } } @@ -927,14 +935,14 @@ namespace Umbraco.Web.PublishedCache.NuCache using (var scope = _scopeProvider.CreateScope()) { scope.ReadLock(Constants.Locks.ContentTree); - _contentStore.UpdateDataTypes(idsA, id => CreateContentType(PublishedItemType.Content, id)); + _contentStore.UpdateDataTypesLocked(idsA, id => CreateContentType(PublishedItemType.Content, id)); scope.Complete(); } using (var scope = _scopeProvider.CreateScope()) { scope.ReadLock(Constants.Locks.MediaTree); - _mediaStore.UpdateDataTypes(idsA, id => CreateContentType(PublishedItemType.Media, id)); + _mediaStore.UpdateDataTypesLocked(idsA, id => CreateContentType(PublishedItemType.Media, id)); scope.Complete(); } } @@ -964,7 +972,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } break; case DomainChangeTypes.Remove: - _domainStore.Clear(payload.Id); + _domainStore.ClearLocked(payload.Id); break; case DomainChangeTypes.Refresh: var domain = _serviceContext.DomainService.GetById(payload.Id); @@ -972,14 +980,14 @@ namespace Umbraco.Web.PublishedCache.NuCache if (domain.RootContentId.HasValue == false) continue; // anomaly if (domain.LanguageIsoCode.IsNullOrWhiteSpace()) continue; // anomaly var culture = CultureInfo.GetCultureInfo(domain.LanguageIsoCode); - _domainStore.Set(domain.Id, new Domain(domain.Id, domain.DomainName, domain.RootContentId.Value, culture, domain.IsWildcard)); + _domainStore.SetLocked(domain.Id, new Domain(domain.Id, domain.DomainName, domain.RootContentId.Value, culture, domain.IsWildcard)); break; } } } } - //Methods used to prevent allocations of lists + //Methods used to prevent allocations of lists private void AddToList(ref List list, int val) => GetOrCreateList(ref list).Add(val); private List GetOrCreateList(ref List list) => list ?? (list = new List()); @@ -1057,11 +1065,11 @@ namespace Umbraco.Web.PublishedCache.NuCache ? Array.Empty() : _dataSource.GetTypeContentSources(scope, refreshedIds).ToArray(); - _contentStore.UpdateContentTypes(removedIds, typesA, kits); + _contentStore.UpdateContentTypesLocked(removedIds, typesA, kits); if (!otherIds.IsCollectionEmpty()) - _contentStore.UpdateContentTypes(CreateContentTypes(PublishedItemType.Content, otherIds.ToArray())); + _contentStore.UpdateContentTypesLocked(CreateContentTypes(PublishedItemType.Content, otherIds.ToArray())); if (!newIds.IsCollectionEmpty()) - _contentStore.NewContentTypes(CreateContentTypes(PublishedItemType.Content, newIds.ToArray())); + _contentStore.NewContentTypesLocked(CreateContentTypes(PublishedItemType.Content, newIds.ToArray())); scope.Complete(); } } @@ -1088,11 +1096,11 @@ namespace Umbraco.Web.PublishedCache.NuCache ? Array.Empty() : _dataSource.GetTypeMediaSources(scope, refreshedIds).ToArray(); - _mediaStore.UpdateContentTypes(removedIds, typesA, kits); + _mediaStore.UpdateContentTypesLocked(removedIds, typesA, kits); if (!otherIds.IsCollectionEmpty()) - _mediaStore.UpdateContentTypes(CreateContentTypes(PublishedItemType.Media, otherIds.ToArray()).ToArray()); + _mediaStore.UpdateContentTypesLocked(CreateContentTypes(PublishedItemType.Media, otherIds.ToArray()).ToArray()); if (!newIds.IsCollectionEmpty()) - _mediaStore.NewContentTypes(CreateContentTypes(PublishedItemType.Media, newIds.ToArray()).ToArray()); + _mediaStore.NewContentTypesLocked(CreateContentTypes(PublishedItemType.Media, newIds.ToArray()).ToArray()); scope.Complete(); } } @@ -1163,12 +1171,14 @@ namespace Umbraco.Web.PublishedCache.NuCache // elements // just need to make sure nothing gets elements in another enlisted action... so using // a MaxValue to make sure this one runs last, and it should be ok + scopeContext.Enlist("Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.Resync", () => this, (completed, svc) => { ((PublishedSnapshot)svc.CurrentPublishedSnapshot)?.Resync(); }, int.MaxValue); } + // create a new snapshot cache if snapshots are different gens if (contentSnap.Gen != _contentGen || mediaSnap.Gen != _mediaGen || domainSnap.Gen != _domainGen || _elementsCache == null) { @@ -1335,7 +1345,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { //culture changed on an existing language var cultureChanged = e.SavedEntities.Any(x => !x.WasPropertyDirty(nameof(ILanguage.Id)) && x.WasPropertyDirty(nameof(ILanguage.IsoCode))); - if(cultureChanged) + if (cultureChanged) { RebuildContentDbCache(); } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs b/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs index 9671949ff0..c38940da25 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs @@ -22,6 +22,11 @@ namespace Umbraco.Web.PublishedCache.NuCache // This class is optimized for many readers, few writers // Readers are lock-free + // NOTE - we used to lock _rlocko the long hand way with Monitor.Enter(_rlocko, ref lockTaken) but this has + // been replaced with a normal c# lock because that's exactly how the normal c# lock works, + // see https://blogs.msdn.microsoft.com/ericlippert/2009/03/06/locks-and-exceptions-do-not-mix/ + // for the readlock, there's no reason here to use the long hand way. + private readonly ConcurrentDictionary> _items; private readonly ConcurrentQueue _genObjs; private GenObj _genObj; @@ -30,7 +35,6 @@ namespace Umbraco.Web.PublishedCache.NuCache private long _liveGen, _floorGen; private bool _nextGen, _collectAuto; private Task _collectTask; - private volatile int _wlocked; // minGenDelta to be adjusted // we may want to throttle collects even if delta is reached @@ -71,20 +75,14 @@ namespace Umbraco.Web.PublishedCache.NuCache // are all ignored - Release is private and meant to be invoked with 'commit' being false only // only on the outermost lock (by SnapDictionaryWriter) - // using (...) {} for locking is prone to nasty leaks in case of weird exceptions - // such as thread-abort or out-of-memory, but let's not worry about it now + // side note - using (...) {} for locking is prone to nasty leaks in case of weird exceptions + // such as thread-abort or out-of-memory, which is why we've moved away from the old using wrapper we had on locking. private readonly string _instanceId = Guid.NewGuid().ToString("N"); - private class ReadLockInfo - { - public bool Taken; - } - private class WriteLockInfo { public bool Taken; - public bool Count; } // a scope contextual that represents a locked writer to the dictionary @@ -92,8 +90,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { private readonly WriteLockInfo _lockinfo = new WriteLockInfo(); private readonly SnapDictionary _dictionary; - private int _released; - + public ScopedWriteLock(SnapDictionary dictionary, bool scoped) { _dictionary = dictionary; @@ -102,8 +99,6 @@ namespace Umbraco.Web.PublishedCache.NuCache public override void Release(bool completed) { - if (Interlocked.CompareExchange(ref _released, 1, 0) != 0) - return; _dictionary.Release(_lockinfo, completed); } } @@ -117,28 +112,31 @@ namespace Umbraco.Web.PublishedCache.NuCache return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped)); } + private void EnsureLocked() + { + if (!Monitor.IsEntered(_wlocko)) + throw new InvalidOperationException("Write lock must be acquried."); + } + private void Lock(WriteLockInfo lockInfo, bool forceGen = false) { + if (Monitor.IsEntered(_wlocko)) + throw new InvalidOperationException("Recursive locks not allowed"); + Monitor.Enter(_wlocko, ref lockInfo.Taken); - var rtaken = false; - try + lock(_rlocko) { - Monitor.Enter(_rlocko, ref rtaken); - // assume everything in finally runs atomically // http://stackoverflow.com/questions/18501678/can-this-unexpected-behavior-of-prepareconstrainedregions-and-thread-abort-be-ex // http://joeduffyblog.com/2005/03/18/atomicity-and-asynchronous-exception-failures/ // http://joeduffyblog.com/2007/02/07/introducing-the-new-readerwriterlockslim-in-orcas/ // http://chabster.blogspot.fr/2013/12/readerwriterlockslim-fails-on-dual.html //RuntimeHelpers.PrepareConstrainedRegions(); - try { } finally + try { } + finally { - // increment the lock count, and register that this lock is counting - _wlocked++; - lockInfo.Count = true; - - if (_nextGen == false || (forceGen && _wlocked == 1)) + if (_nextGen == false || (forceGen)) { // because we are changing things, a new generation // is created, which will trigger a new snapshot @@ -149,15 +147,6 @@ namespace Umbraco.Web.PublishedCache.NuCache } } } - finally - { - if (rtaken) Monitor.Exit(_rlocko); - } - } - - private void Lock(ReadLockInfo lockInfo) - { - Monitor.Enter(_rlocko, ref lockInfo.Taken); } private void Release(WriteLockInfo lockInfo, bool commit = true) @@ -168,22 +157,17 @@ namespace Umbraco.Web.PublishedCache.NuCache if (commit == false) { - var rtaken = false; - try + lock(_rlocko) { - Monitor.Enter(_rlocko, ref rtaken); - try { } finally + try { } + finally { // forget about the temp. liveGen _nextGen = false; _liveGen -= 1; } } - finally - { - if (rtaken) Monitor.Exit(_rlocko); - } - + foreach (var item in _items) { var link = item.Value; @@ -197,16 +181,10 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - // decrement the lock count, if counting, then exit the lock - if (lockInfo.Count) _wlocked--; + // TODO: Shouldn't this be in a finally block? Monitor.Exit(_wlocko); } - private void Release(ReadLockInfo lockInfo) - { - if (lockInfo.Taken) Monitor.Exit(_rlocko); - } - #endregion #region Set, Clear, Get, Has @@ -219,75 +197,59 @@ namespace Umbraco.Web.PublishedCache.NuCache return link; } - public void Set(TKey key, TValue value) + public void SetLocked(TKey key, TValue value) { - var lockInfo = new WriteLockInfo(); - try - { - Lock(lockInfo); + EnsureLocked(); - // this is safe only because we're write-locked - var link = GetHead(key); - if (link != null) + // this is safe only because we're write-locked + var link = GetHead(key); + if (link != null) + { + // already in the dict + if (link.Gen != _liveGen) { - // already in the dict - if (link.Gen != _liveGen) - { - // for an older gen - if value is different then insert a new - // link for the new gen, with the new value - if (link.Value != value) - _items.TryUpdate(key, new LinkedNode(value, _liveGen, link), link); - } - else - { - // for the live gen - we can fix the live gen - and remove it - // if value is null and there's no next gen - if (value == null && link.Next == null) - _items.TryRemove(key, out link); - else - link.Value = value; - } + // for an older gen - if value is different then insert a new + // link for the new gen, with the new value + if (link.Value != value) + _items.TryUpdate(key, new LinkedNode(value, _liveGen, link), link); } else { - _items.TryAdd(key, new LinkedNode(value, _liveGen)); - } - } - finally - { - Release(lockInfo); - } - } - - public void Clear(TKey key) - { - Set(key, null); - } - - public void Clear() - { - var lockInfo = new WriteLockInfo(); - try - { - Lock(lockInfo); - - // this is safe only because we're write-locked - foreach (var kvp in _items.Where(x => x.Value != null)) - { - if (kvp.Value.Gen < _liveGen) - { - var link = new LinkedNode(null, _liveGen, kvp.Value); - _items.TryUpdate(kvp.Key, link, kvp.Value); - } + // for the live gen - we can fix the live gen - and remove it + // if value is null and there's no next gen + if (value == null && link.Next == null) + _items.TryRemove(key, out link); else - { - kvp.Value.Value = null; - } + link.Value = value; } } - finally + else { - Release(lockInfo); + _items.TryAdd(key, new LinkedNode(value, _liveGen)); + } + } + + public void ClearLocked(TKey key) + { + SetLocked(key, null); + } + + public void ClearLocked() + { + EnsureLocked(); + + // this is safe only because we're write-locked + foreach (var kvp in _items.Where(x => x.Value != null)) + { + if (kvp.Value.Gen < _liveGen) + { + var link = new LinkedNode(null, _liveGen, kvp.Value); + _items.TryUpdate(kvp.Key, link, kvp.Value); + } + else + { + kvp.Value.Value = null; + } } } @@ -347,11 +309,8 @@ namespace Umbraco.Web.PublishedCache.NuCache public Snapshot CreateSnapshot() { - var lockInfo = new ReadLockInfo(); - try + lock(_rlocko) { - Lock(lockInfo); - // if no next generation is required, and we already have a gen object, // use it to create a new snapshot if (_nextGen == false && _genObj != null) @@ -360,7 +319,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // else we need to try to create a new gen object // whether we are wlocked or not, noone can rlock while we do, // so _liveGen and _nextGen are safe - if (_wlocked > 0) // volatile, cannot ++ but could -- + if (Monitor.IsEntered(_wlocko)) { // write-locked, cannot use latest gen (at least 1) so use previous var snapGen = _nextGen ? _liveGen - 1 : _liveGen; @@ -398,10 +357,6 @@ namespace Umbraco.Web.PublishedCache.NuCache return snapshot; } - finally - { - Release(lockInfo); - } } public Task CollectAsync() @@ -496,17 +451,18 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - public /*async*/ Task PendingCollect() - { - Task task; - lock (_rlocko) - { - task = _collectTask; - } - return task ?? Task.CompletedTask; - //if (task != null) - // await task; - } + // TODO: This is never used? Should it be? Maybe move to TestHelper below? + //public /*async*/ Task PendingCollect() + //{ + // Task task; + // lock (_rlocko) + // { + // task = _collectTask; + // } + // return task ?? Task.CompletedTask; + // //if (task != null) + // // await task; + //} public long GenCount => _genObjs.Count; @@ -531,7 +487,7 @@ namespace Umbraco.Web.PublishedCache.NuCache public long LiveGen => _dict._liveGen; public long FloorGen => _dict._floorGen; public bool NextGen => _dict._nextGen; - public int WLocked => _dict._wlocked; + public bool IsLocked => Monitor.IsEntered(_dict._wlocko); public bool CollectAuto { diff --git a/src/Umbraco.Web/PublishedCache/NuCache/notes.txt b/src/Umbraco.Web/PublishedCache/NuCache/readme.md similarity index 70% rename from src/Umbraco.Web/PublishedCache/NuCache/notes.txt rename to src/Umbraco.Web/PublishedCache/NuCache/readme.md index ff2d8dd48b..c8e8a363e9 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/notes.txt +++ b/src/Umbraco.Web/PublishedCache/NuCache/readme.md @@ -1,10 +1,6 @@ NuCache Documentation ====================== -NOTE - RENAMING -Facade = PublishedSnapshot -and everything needs to be renamed accordingly - HOW IT WORKS ------------- @@ -22,12 +18,12 @@ When reading the cache, we read views up the chain until we find a value (which null) for the given id, and finally we read the store itself. -The FacadeService manages a ContentStore for content, and another for media. -When a Facade is created, the FacadeService gets ContentView objects from the stores. +The PublishedSnapshotService manages a ContentStore for content, and another for media. +When a PublishedSnapshot is created, the PublishedSnapshotService gets ContentView objects from the stores. Views provide an immutable snapshot of the content and media. -When the FacadeService is notified of changes, it notifies the stores. -Then it resync the current Facade, so that it requires new views, etc. +When the PublishedSnapshotService is notified of changes, it notifies the stores. +Then it resync the current PublishedSnapshot, so that it requires new views, etc. Whenever a content, media or member is modified or removed, the cmsContentNu table is updated with a json dictionary of alias => property value, so that a content, @@ -50,32 +46,32 @@ Each ContentStore has a _freezeLock object used to protect the 'Frozen' state of the store. It's a disposable object that releases the lock when disposed, so usage would be: using (store.Frozen) { ... }. -The FacadeService has a _storesLock object used to guarantee atomic access to the +The PublishedSnapshotService has a _storesLock object used to guarantee atomic access to the set of content, media stores. CACHE ------ -For each set of views, the FacadeService creates a SnapshotCache. So a SnapshotCache +For each set of views, the PublishedSnapshotService creates a SnapshotCache. So a SnapshotCache is valid until anything changes in the content or media trees. In other words, things that go in the SnapshotCache stay until a content or media is modified. -For each Facade, the FacadeService creates a FacadeCache. So a FacadeCache is valid -for the duration of the Facade (usually, the request). In other words, things that go -in the FacadeCache stay (and are visible to) for the duration of the request only. +For each PublishedSnapshot, the PublishedSnapshotService creates a PublishedSnapshotCache. So a PublishedSnapshotCache is valid +for the duration of the PublishedSnapshot (usually, the request). In other words, things that go +in the PublishedSnapshotCache stay (and are visible to) for the duration of the request only. -The FacadeService defines a static constant FullCacheWhenPreviewing, that defines +The PublishedSnapshotService defines a static constant FullCacheWhenPreviewing, that defines how caches operate when previewing: - when true, the caches in preview mode work normally. -- when false, everything that would go to the SnapshotCache goes to the FacadeCache. +- when false, everything that would go to the SnapshotCache goes to the PublishedSnapshotCache. At the moment it is true in the code, which means that eg converted values for previewed content will go in the SnapshotCache. Makes for faster preview, but uses more memory on the long term... would need some benchmarking to figure out what is best. -Members only live for the duration of the Facade. So, for members SnapshotCache is -never used, and everything goes to the FacadeCache. +Members only live for the duration of the PublishedSnapshot. So, for members SnapshotCache is +never used, and everything goes to the PublishedSnapshotCache. All cache keys are computed in the CacheKeys static class. @@ -85,15 +81,15 @@ TESTS For testing purposes the following mechanisms exist: -The Facade type has a static Current property that is used to obtain the 'current' -facade in many places, going through the PublishedCachesServiceResolver to get the -current service, and asking the current service for the current facade, which by +The PublishedSnapshot type has a static Current property that is used to obtain the 'current' +PublishedSnapshot in many places, going through the PublishedCachesServiceResolver to get the +current service, and asking the current service for the current PublishedSnapshot, which by default relies on UmbracoContext. For test purposes, it is possible to override the -entire mechanism by defining Facade.GetCurrentFacadeFunc which should return a facade. +entire mechanism by defining PublishedSnapshot.GetCurrentPublishedSnapshotFunc which should return a PublishedSnapshot. A PublishedContent keeps only id-references to its parent and children, and needs a way to retrieve the actual objects from the cache - which depends on the current -facade. It is possible to override the entire mechanism by defining PublishedContent. +PublishedSnapshot. It is possible to override the entire mechanism by defining PublishedContent. GetContentByIdFunc or .GetMediaByIdFunc. Setting these functions must be done before Resolution is frozen. @@ -110,7 +106,7 @@ possible to support detached contents & properties, even those that do not have int id, but again this should be refactored entirely anyway. Not doing any row-version checks (see XmlStore) when reloading from database, though it -is maintained in the database. Two FIXME in FacadeService. Should we do it? +is maintained in the database. Two FIXME in PublishedSnapshotService. Should we do it? There is no on-disk cache at all so everything is reloaded from the cmsContentNu table when the site restarts. This is pretty fast, but we should experiment with solutions to @@ -121,4 +117,4 @@ PublishedMember exposes properties that IPublishedContent does not, and that are to be lost soon as the member is wrapped (content set, model...) - so we probably need some sort of IPublishedMember. -/ \ No newline at end of file +/ diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 85a3ce9ea7..0111a36993 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -1222,7 +1222,7 @@ - + diff --git a/src/Umbraco.Web/UmbracoApplication.cs b/src/Umbraco.Web/UmbracoApplication.cs index 94403bc1be..f8ee238da7 100644 --- a/src/Umbraco.Web/UmbracoApplication.cs +++ b/src/Umbraco.Web/UmbracoApplication.cs @@ -1,7 +1,9 @@ -using System.Threading; +using System.Configuration; +using System.Threading; using System.Web; using Umbraco.Core; using Umbraco.Core.Logging.Serilog; +using Umbraco.Core.Runtime; using Umbraco.Web.Runtime; namespace Umbraco.Web @@ -14,7 +16,17 @@ namespace Umbraco.Web protected override IRuntime GetRuntime() { var logger = SerilogLogger.CreateWithDefaultConfiguration(); - return new WebRuntime(this, logger, new MainDom(logger)); + + // Determine if we should use the sql main dom or the default + var appSettingMainDomLock = ConfigurationManager.AppSettings[Constants.AppSettings.MainDomLock]; + + var mainDomLock = appSettingMainDomLock == "SqlMainDomLock" + ? (IMainDomLock)new SqlMainDomLock(logger) + : new MainDomSemaphoreLock(logger); + + var runtime = new WebRuntime(this, logger, new MainDom(logger, mainDomLock)); + + return runtime; } ///