diff --git a/build/NuSpecs/UmbracoCms.Core.nuspec b/build/NuSpecs/UmbracoCms.Core.nuspec index 11b1870b90..b257f8dcdd 100644 --- a/build/NuSpecs/UmbracoCms.Core.nuspec +++ b/build/NuSpecs/UmbracoCms.Core.nuspec @@ -1,6 +1,6 @@ - + UmbracoCms.Core 8.0.0 Umbraco Cms Core Binaries diff --git a/build/NuSpecs/UmbracoCms.Web.nuspec b/build/NuSpecs/UmbracoCms.Web.nuspec index e81c199288..79113a7c94 100644 --- a/build/NuSpecs/UmbracoCms.Web.nuspec +++ b/build/NuSpecs/UmbracoCms.Web.nuspec @@ -1,6 +1,6 @@ - + UmbracoCms.Web 8.0.0 Umbraco Cms Core Binaries diff --git a/build/NuSpecs/UmbracoCms.nuspec b/build/NuSpecs/UmbracoCms.nuspec index c93a82c691..8ec1484452 100644 --- a/build/NuSpecs/UmbracoCms.nuspec +++ b/build/NuSpecs/UmbracoCms.nuspec @@ -1,6 +1,6 @@ - + UmbracoCms 8.0.0 Umbraco Cms diff --git a/src/Umbraco.Core/Scoping/ScopeContextualBase.cs b/src/Umbraco.Core/Scoping/ScopeContextualBase.cs index 25f176d471..7461142234 100644 --- a/src/Umbraco.Core/Scoping/ScopeContextualBase.cs +++ b/src/Umbraco.Core/Scoping/ScopeContextualBase.cs @@ -2,61 +2,39 @@ namespace Umbraco.Core.Scoping { - /// - /// Provides a base class for scope contextual objects. - /// - /// - /// A scope contextual object is enlisted in the current scope context, - /// if any, and released when the context exists. It must be used in a 'using' - /// block, and will be released when disposed, if not part of a scope. - /// + // base class for an object that will be enlisted in scope context, if any. it + // must be used in a 'using' block, and if not scoped, released when disposed, + // else when scope context runs enlisted actions public abstract class ScopeContextualBase : IDisposable { - private bool _scoped; + private bool _using, _scoped; - /// - /// Gets a contextual object. - /// - /// The type of the object. - /// A scope provider. - /// A context key for the object. - /// A function producing the contextual object. - /// The contextual object. - /// - /// - /// public static T Get(IScopeProvider scopeProvider, string key, Func ctor) where T : ScopeContextualBase { - // no scope context = create a non-scoped object var scopeContext = scopeProvider.Context; if (scopeContext == null) return ctor(false); - // create & enlist the scoped object var w = scopeContext.Enlist("ScopeContextualBase_" + key, () => ctor(true), (completed, item) => { item.Release(completed); }); + if (w._using) throw new InvalidOperationException("panic: used."); + w._using = true; w._scoped = true; return w; } - /// - /// - /// If not scoped, then this releases the contextual object. - /// public void Dispose() { + _using = false; + if (_scoped == false) Release(true); } - /// - /// Releases the contextual object. - /// - /// A value indicating whether the scoped operation completed. public abstract void Release(bool completed); } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index f2bef52922..cc962fbe22 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -2875,14 +2875,7 @@ namespace Umbraco.Core.Services.Implement { foreach (var property in blueprint.Properties) { - if (property.PropertyType.VariesByCulture()) - { - content.SetValue(property.Alias, property.GetValue(culture), culture); - } - else - { - content.SetValue(property.Alias, property.GetValue()); - } + content.SetValue(property.Alias, property.GetValue(culture), culture); } content.Name = blueprint.Name; diff --git a/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs b/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs index b435af9e77..013dbadbb8 100644 --- a/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs +++ b/src/Umbraco.Tests/Cache/SnapDictionaryTests.cs @@ -5,7 +5,6 @@ using Moq; using NUnit.Framework; using Umbraco.Core.Scoping; using Umbraco.Web.PublishedCache.NuCache; -using Umbraco.Web.PublishedCache.NuCache.Snap; namespace Umbraco.Tests.Cache { @@ -389,7 +388,8 @@ namespace Umbraco.Tests.Cache // collect liveGen GC.Collect(); - Assert.IsTrue(d.Test.GenObjs.TryPeek(out var genObj)); + SnapDictionary.GenerationObject genObj; + Assert.IsTrue(d.Test.GenerationObjects.TryPeek(out genObj)); genObj = null; // in Release mode, it works, but in Debug mode, the weak reference is still alive @@ -399,14 +399,14 @@ namespace Umbraco.Tests.Cache GC.Collect(); #endif - Assert.IsTrue(d.Test.GenObjs.TryPeek(out genObj)); - Assert.IsFalse(genObj.WeakGenRef.IsAlive); // snapshot is gone, along with its reference + Assert.IsTrue(d.Test.GenerationObjects.TryPeek(out genObj)); + Assert.IsFalse(genObj.WeakReference.IsAlive); // snapshot is gone, along with its reference await d.CollectAsync(); Assert.AreEqual(0, d.Test.GetValues(1).Length); // null value is gone Assert.AreEqual(0, d.Count); // item is gone - Assert.AreEqual(0, d.Test.GenObjs.Count); + Assert.AreEqual(0, d.Test.GenerationObjects.Count); Assert.AreEqual(0, d.SnapCount); // snapshot is gone Assert.AreEqual(0, d.GenCount); // and generation has been dequeued } @@ -632,7 +632,7 @@ namespace Umbraco.Tests.Cache Assert.AreEqual(1, d.Test.LiveGen); Assert.IsTrue(d.Test.NextGen); - using (d.GetScopedWriteLock(GetScopeProvider())) + using (d.GetWriter(GetScopeProvider())) { var s1 = d.CreateSnapshot(); @@ -685,7 +685,7 @@ namespace Umbraco.Tests.Cache Assert.IsFalse(d.Test.NextGen); Assert.AreEqual("uno", s2.Get(1)); - using (d.GetScopedWriteLock(GetScopeProvider())) + using (d.GetWriter(GetScopeProvider())) { // gen 3 Assert.AreEqual(2, d.Test.GetValues(1).Length); @@ -712,102 +712,16 @@ namespace Umbraco.Tests.Cache } [Test] - public void NestedWriteLocking1() - { - var d = new SnapDictionary(); - var t = d.Test; - t.CollectAuto = false; - - Assert.AreEqual(0, d.CreateSnapshot().Gen); - - // no scope context: writers nest, last one to be disposed commits - - var scopeProvider = GetScopeProvider(); - - using (var w1 = d.GetScopedWriteLock(scopeProvider)) - { - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(1, t.WLocked); - Assert.IsTrue(t.NextGen); - - using (var w2 = d.GetScopedWriteLock(scopeProvider)) - { - 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); - } - - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(1, t.WLocked); - Assert.IsTrue(t.NextGen); - - Assert.AreEqual(0, d.CreateSnapshot().Gen); - } - - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(0, t.WLocked); - Assert.IsTrue(t.NextGen); - - Assert.AreEqual(1, d.CreateSnapshot().Gen); - } - - [Test] - public void NestedWriteLocking2() + public void NestedWriteLocking() { var d = new SnapDictionary(); d.Test.CollectAuto = false; - Assert.AreEqual(0, d.CreateSnapshot().Gen); - - // scope context: writers enlist - - var scopeContext = new ScopeContext(); - var scopeProvider = GetScopeProvider(scopeContext); - - using (var w1 = d.GetScopedWriteLock(scopeProvider)) + var scopeProvider = GetScopeProvider(); + using (d.GetWriter(scopeProvider)) { - using (var w2 = d.GetScopedWriteLock(scopeProvider)) + using (d.GetWriter(scopeProvider)) { - Assert.AreSame(w1, w2); - - d.Set(1, "one"); - } - } - } - - [Test] - public void NestedWriteLocking3() - { - var d = new SnapDictionary(); - var t = d.Test; - t.CollectAuto = false; - - Assert.AreEqual(0, d.CreateSnapshot().Gen); - - var scopeContext = new ScopeContext(); - var scopeProvider1 = GetScopeProvider(); - var scopeProvider2 = GetScopeProvider(scopeContext); - - using (var w1 = d.GetScopedWriteLock(scopeProvider1)) - { - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(1, t.WLocked); - Assert.IsTrue(t.NextGen); - - using (var w2 = d.GetScopedWriteLock(scopeProvider2)) - { - Assert.AreEqual(1, t.LiveGen); - Assert.AreEqual(2, t.WLocked); - Assert.IsTrue(t.NextGen); - - Assert.AreNotSame(w1, w2); - d.Set(1, "one"); } } @@ -850,7 +764,7 @@ namespace Umbraco.Tests.Cache var scopeProvider = GetScopeProvider(); - using (d.GetScopedWriteLock(scopeProvider)) + using (d.GetWriter(scopeProvider)) { // gen 3 Assert.AreEqual(2, d.Test.GetValues(1).Length); @@ -895,7 +809,7 @@ namespace Umbraco.Tests.Cache var scopeProvider = GetScopeProvider(); - using (d.GetScopedWriteLock(scopeProvider)) + using (d.GetWriter(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 @@ -932,10 +846,9 @@ namespace Umbraco.Tests.Cache Assert.AreEqual(2, s2.Gen); Assert.AreEqual("uno", s2.Get(1)); - var scopeContext = new ScopeContext(); - var scopeProvider = GetScopeProvider(scopeContext); + var scopeProvider = GetScopeProvider(true); - using (d.GetScopedWriteLock(scopeProvider)) + using (d.GetWriter(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 @@ -954,7 +867,7 @@ namespace Umbraco.Tests.Cache Assert.AreEqual(2, s4.Gen); Assert.AreEqual("uno", s4.Get(1)); - scopeContext.ScopeExit(true); + ((ScopeContext) scopeProvider.Context).ScopeExit(true); var s5 = d.CreateSnapshot(); Assert.AreEqual(3, s5.Gen); @@ -965,8 +878,7 @@ namespace Umbraco.Tests.Cache public void ScopeLocking2() { var d = new SnapDictionary(); - var t = d.Test; - t.CollectAuto = false; + d.Test.CollectAuto = false; // gen 1 d.Set(1, "one"); @@ -979,13 +891,12 @@ namespace Umbraco.Tests.Cache Assert.AreEqual(2, s2.Gen); Assert.AreEqual("uno", s2.Get(1)); - Assert.AreEqual(2, t.LiveGen); - Assert.IsFalse(t.NextGen); - + var scopeProviderMock = new Mock(); var scopeContext = new ScopeContext(); - var scopeProvider = GetScopeProvider(scopeContext); + scopeProviderMock.Setup(x => x.Context).Returns(scopeContext); + var scopeProvider = scopeProviderMock.Object; - using (d.GetScopedWriteLock(scopeProvider)) + using (d.GetWriter(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 @@ -994,35 +905,18 @@ namespace Umbraco.Tests.Cache Assert.AreEqual(2, s3.Gen); Assert.AreEqual("uno", s3.Get(1)); - // we made some changes, so a next gen is required - Assert.AreEqual(3, t.LiveGen); - Assert.IsTrue(t.NextGen); - Assert.AreEqual(1, t.WLocked); - // but live snapshot contains changes - var ls = t.LiveSnapshot; + var ls = d.Test.LiveSnapshot; Assert.AreEqual("ein", ls.Get(1)); Assert.AreEqual(3, ls.Gen); } - // nothing is committed until scope exits - Assert.AreEqual(3, t.LiveGen); - Assert.IsTrue(t.NextGen); - Assert.AreEqual(1, t.WLocked); - - // no changes until exit var s4 = d.CreateSnapshot(); Assert.AreEqual(2, s4.Gen); Assert.AreEqual("uno", s4.Get(1)); scopeContext.ScopeExit(false); - // now things have changed - Assert.AreEqual(2, t.LiveGen); - Assert.IsFalse(t.NextGen); - Assert.AreEqual(0, t.WLocked); - - // no changes since not completed var s5 = d.CreateSnapshot(); Assert.AreEqual(2, s5.Gen); Assert.AreEqual("uno", s5.Get(1)); @@ -1061,92 +955,12 @@ namespace Umbraco.Tests.Cache Assert.AreEqual("four", all[3]); } - [Test] - public void DontPanic() + private IScopeProvider GetScopeProvider(bool withContext = false) { - var d = new SnapDictionary(); - d.Test.CollectAuto = false; - - Assert.IsNull(d.Test.GenObj); - - // gen 1 - d.Set(1, "one"); - Assert.IsTrue(d.Test.NextGen); - Assert.AreEqual(1, d.Test.LiveGen); - Assert.IsNull(d.Test.GenObj); - - var s1 = d.CreateSnapshot(); - Assert.IsFalse(d.Test.NextGen); - Assert.AreEqual(1, d.Test.LiveGen); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(1, d.Test.GenObj.Gen); - - Assert.AreEqual(1, s1.Gen); - Assert.AreEqual("one", s1.Get(1)); - - d.Set(1, "uno"); - Assert.IsTrue(d.Test.NextGen); - Assert.AreEqual(2, d.Test.LiveGen); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(1, d.Test.GenObj.Gen); - - var scopeContext = new ScopeContext(); - var scopeProvider = GetScopeProvider(scopeContext); - - // scopeProvider.Context == scopeContext -> writer is scoped - // 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"); - Assert.IsTrue(d.Test.NextGen); - Assert.AreEqual(3, d.Test.LiveGen); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(2, d.Test.GenObj.Gen); - } - - // writer has not released - Assert.AreEqual(1, d.Test.WLocked); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(2, d.Test.GenObj.Gen); - - // nothing changed - Assert.IsTrue(d.Test.NextGen); - Assert.AreEqual(3, d.Test.LiveGen); - - // panic! - var s2 = d.CreateSnapshot(); - - Assert.AreEqual(1, d.Test.WLocked); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(2, d.Test.GenObj.Gen); - Assert.AreEqual(3, d.Test.LiveGen); - Assert.IsTrue(d.Test.NextGen); - - // release writer - scopeContext.ScopeExit(true); - - Assert.AreEqual(0, d.Test.WLocked); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(2, d.Test.GenObj.Gen); - Assert.AreEqual(3, d.Test.LiveGen); - Assert.IsTrue(d.Test.NextGen); - - var s3 = d.CreateSnapshot(); - - Assert.AreEqual(0, d.Test.WLocked); - Assert.IsNotNull(d.Test.GenObj); - Assert.AreEqual(3, d.Test.GenObj.Gen); - Assert.AreEqual(3, d.Test.LiveGen); - Assert.IsFalse(d.Test.NextGen); - } - - private IScopeProvider GetScopeProvider(ScopeContext scopeContext = null) - { - var scopeProvider = Mock.Of(); - Mock.Get(scopeProvider) - .Setup(x => x.Context).Returns(scopeContext); + var scopeProviderMock = new Mock(); + var scopeContext = withContext ? new ScopeContext() : null; + scopeProviderMock.Setup(x => x.Context).Returns(scopeContext); + var scopeProvider = scopeProviderMock.Object; return scopeProvider; } } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js index c027e0778e..9ee83dc2ba 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js @@ -62,8 +62,8 @@ function valPropertyMsg(serverValidationManager) { if (!watcher) { watcher = scope.$watch("currentProperty.value", function (newValue, oldValue) { - - if (angular.equals(newValue, oldValue)) { + + if (!newValue || angular.equals(newValue, oldValue)) { return; } @@ -78,12 +78,10 @@ function valPropertyMsg(serverValidationManager) { // based on other errors. We'll also check if there's no other validation errors apart from valPropertyMsg, if valPropertyMsg // is the only one, then we'll clear. - if (errCount === 0 || (errCount === 1 && angular.isArray(formCtrl.$error.valPropertyMsg)) || (formCtrl.$invalid && angular.isArray(formCtrl.$error.valServer))) { + if ((errCount === 1 && angular.isArray(formCtrl.$error.valPropertyMsg)) || (formCtrl.$invalid && angular.isArray(formCtrl.$error.valServer))) { scope.errorMsg = ""; formCtrl.$setValidity('valPropertyMsg', true); - } else if (showValidation && scope.errorMsg === "") { - formCtrl.$setValidity('valPropertyMsg', false); - scope.errorMsg = getErrorMsg(); + stopWatch(); } }, true); } @@ -154,7 +152,6 @@ function valPropertyMsg(serverValidationManager) { showValidation = true; if (hasError && scope.errorMsg === "") { scope.errorMsg = getErrorMsg(); - startWatch(); } else if (!hasError) { scope.errorMsg = ""; diff --git a/src/Umbraco.Web.UI.Client/src/preview/preview.controller.js b/src/Umbraco.Web.UI.Client/src/preview/preview.controller.js index 4733c58556..7d6584d2f1 100644 --- a/src/Umbraco.Web.UI.Client/src/preview/preview.controller.js +++ b/src/Umbraco.Web.UI.Client/src/preview/preview.controller.js @@ -113,12 +113,7 @@ var app = angular.module("umbraco.preview", ['umbraco.resources', 'umbraco.servi $scope.exitPreview = function () { var culture = $location.search().culture || getParameterByName("culture"); - var relativeUrl = "/" + $scope.pageId; - - if(culture){ - relativeUrl +='?culture='+ culture; - } - + var relativeUrl = "/" + $scope.pageId +'?culture='+ culture; window.top.location.href = "../preview/end?redir=" + encodeURIComponent(relativeUrl); }; diff --git a/src/Umbraco.Web.UI.Client/src/views/content/content.assigndomain.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/content.assigndomain.controller.js index 5fa1eebd0b..0f27f3046c 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/content.assigndomain.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/content.assigndomain.controller.js @@ -46,7 +46,8 @@ if (data.language !== "undefined") { var lang = vm.languages.filter(function (l) { - return matchLanguageById(l, data.language); + return matchLanguageById(l, data.language.Id); + }); if (lang.length > 0) { vm.language = lang[0]; diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/MediaPickerValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/MediaPickerValueConverter.cs index fb3134c9a3..802f42ed9e 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/MediaPickerValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/MediaPickerValueConverter.cs @@ -1,5 +1,4 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Linq; using Umbraco.Core; @@ -15,18 +14,11 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters [DefaultPropertyValueConverter] public class MediaPickerValueConverter : PropertyValueConverterBase { - // hard-coding "image" here but that's how it works at UI level too - private const string ImageTypeAlias = "image"; - - private readonly IPublishedModelFactory _publishedModelFactory; private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; - public MediaPickerValueConverter(IPublishedSnapshotAccessor publishedSnapshotAccessor, - IPublishedModelFactory publishedModelFactory) + public MediaPickerValueConverter(IPublishedSnapshotAccessor publishedSnapshotAccessor) { - _publishedSnapshotAccessor = publishedSnapshotAccessor ?? - throw new ArgumentNullException(nameof(publishedSnapshotAccessor)); - _publishedModelFactory = publishedModelFactory; + _publishedSnapshotAccessor = publishedSnapshotAccessor ?? throw new ArgumentNullException(nameof(publishedSnapshotAccessor)); } public override bool IsConverter(PublishedPropertyType propertyType) @@ -39,19 +31,15 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters var isMultiple = IsMultipleDataType(propertyType.DataType); var isOnlyImages = IsOnlyImagesDataType(propertyType.DataType); + // hard-coding "image" here but that's how it works at UI level too + return isMultiple - ? isOnlyImages - ? typeof(IEnumerable<>).MakeGenericType(ModelType.For(ImageTypeAlias)) - : typeof(IEnumerable) - : isOnlyImages - ? ModelType.For(ImageTypeAlias) - : typeof(IPublishedContent); + ? (isOnlyImages ? typeof(IEnumerable<>).MakeGenericType(ModelType.For("image")) : typeof(IEnumerable)) + : (isOnlyImages ? ModelType.For("image") : typeof(IPublishedContent)); } public override PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) - { - return PropertyCacheLevel.Snapshot; - } + => PropertyCacheLevel.Snapshot; private bool IsMultipleDataType(PublishedDataType dataType) { @@ -65,31 +53,26 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters return config.OnlyImages; } - public override object ConvertSourceToIntermediate(IPublishedElement owner, PublishedPropertyType propertyType, - object source, bool preview) + public override object ConvertSourceToIntermediate(IPublishedElement owner, PublishedPropertyType propertyType, object source, bool preview) { if (source == null) return null; var nodeIds = source.ToString() - .Split(new[] {","}, StringSplitOptions.RemoveEmptyEntries) + .Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries) .Select(Udi.Parse) .ToArray(); return nodeIds; } - public override object ConvertIntermediateToObject(IPublishedElement owner, PublishedPropertyType propertyType, - PropertyCacheLevel cacheLevel, object source, bool preview) + public override object ConvertIntermediateToObject(IPublishedElement owner, PublishedPropertyType propertyType, PropertyCacheLevel cacheLevel, object source, bool preview) { - var isMultiple = IsMultipleDataType(propertyType.DataType); - var isOnlyImages = IsOnlyImagesDataType(propertyType.DataType); - - var udis = (Udi[]) source; - var mediaItems = isOnlyImages - ? _publishedModelFactory.CreateModelList(ImageTypeAlias) - : new List(); - - if (source == null) return isMultiple ? mediaItems : null; + if (source == null) + { + return null; + } + var udis = (Udi[])source; + var mediaItems = new List(); if (udis.Any()) { foreach (var udi in udis) @@ -101,15 +84,12 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters mediaItems.Add(item); } - return isMultiple ? mediaItems : FirstOrDefault(mediaItems); + if (IsMultipleDataType(propertyType.DataType)) + return mediaItems; + return mediaItems.FirstOrDefault(); } return source; } - - private object FirstOrDefault(IList mediaItems) - { - return mediaItems.Count == 0 ? null : mediaItems[0]; - } } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 7ab4a64f31..b3996050a6 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -8,7 +8,6 @@ using CSharpTest.Net.Collections; using Umbraco.Core.Logging; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Scoping; -using Umbraco.Web.PublishedCache.NuCache.Snap; namespace Umbraco.Web.PublishedCache.NuCache { @@ -30,8 +29,8 @@ namespace Umbraco.Web.PublishedCache.NuCache private readonly ILogger _logger; private BPlusTree _localDb; - private readonly ConcurrentQueue _genObjs; - private GenObj _genObj; + private readonly ConcurrentQueue _genRefRefs; + private GenRefRef _genRefRef; private readonly object _wlocko = new object(); private readonly object _rlocko = new object(); private long _liveGen, _floorGen; @@ -65,8 +64,8 @@ namespace Umbraco.Web.PublishedCache.NuCache _contentTypesByAlias = new ConcurrentDictionary>(StringComparer.InvariantCultureIgnoreCase); _xmap = new ConcurrentDictionary(); - _genObjs = new ConcurrentQueue(); - _genObj = null; // no initial gen exists + _genRefRefs = new ConcurrentQueue(); + _genRefRef = null; // no initial gen exists _liveGen = _floorGen = 0; _nextGen = false; // first time, must create a snapshot _collectAuto = true; // collect automatically by default @@ -92,13 +91,12 @@ namespace Umbraco.Web.PublishedCache.NuCache } // a scope contextual that represents a locked writer to the dictionary - private class ScopedWriteLock : ScopeContextualBase + private class ContentStoreWriter : ScopeContextualBase { private readonly WriteLockInfo _lockinfo = new WriteLockInfo(); - private readonly ContentStore _store; - private int _released; + private ContentStore _store; - public ScopedWriteLock(ContentStore store, bool scoped) + public ContentStoreWriter(ContentStore store, bool scoped) { _store = store; store.Lock(_lockinfo, scoped); @@ -106,17 +104,17 @@ namespace Umbraco.Web.PublishedCache.NuCache public override void Release(bool completed) { - if (Interlocked.CompareExchange(ref _released, 1, 0) != 0) - return; + if (_store== null) return; _store.Release(_lockinfo, completed); + _store = null; } } // gets a scope contextual representing a locked writer to the dictionary // TODO: GetScopedWriter? should the dict have a ref onto the scope provider? - public IDisposable GetScopedWriteLock(IScopeProvider scopeProvider) + public IDisposable GetWriter(IScopeProvider scopeProvider) { - return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped)); + return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ContentStoreWriter(this, scoped)); } private void Lock(WriteLockInfo lockInfo, bool forceGen = false) @@ -133,14 +131,12 @@ namespace Umbraco.Web.PublishedCache.NuCache { _wlocked++; lockInfo.Count = true; - if (_nextGen == false || (forceGen && _wlocked == 1)) + if (_nextGen == false || (forceGen && _wlocked == 1)) // if true already... ok to have "holes" in generation objects { // because we are changing things, a new generation // is created, which will trigger a new snapshot - if (_nextGen) - _genObjs.Enqueue(_genObj = new GenObj(_liveGen)); - _liveGen += 1; _nextGen = true; + _liveGen += 1; } } } @@ -216,6 +212,7 @@ namespace Umbraco.Web.PublishedCache.NuCache else dictionary.TryUpdate(key, link.Next, link); } + } #endregion @@ -839,8 +836,8 @@ namespace Umbraco.Web.PublishedCache.NuCache // if no next generation is required, and we already have one, // use it and create a new snapshot - if (_nextGen == false && _genObj != null) - return new Snapshot(this, _genObj.GetGenRef() + if (_nextGen == false && _genRefRef != null) + return new Snapshot(this, _genRefRef.GetGenRef() #if DEBUG , _logger #endif @@ -855,15 +852,15 @@ namespace Umbraco.Web.PublishedCache.NuCache var snapGen = _nextGen ? _liveGen - 1 : _liveGen; // create a new gen ref unless we already have it - if (_genObj == null) - _genObjs.Enqueue(_genObj = new GenObj(snapGen)); - else if (_genObj.Gen != snapGen) + if (_genRefRef == null) + _genRefRefs.Enqueue(_genRefRef = new GenRefRef(snapGen)); + else if (_genRefRef.Gen != snapGen) throw new Exception("panic"); } else { // not write-locked, can use latest gen, create a new gen ref - _genObjs.Enqueue(_genObj = new GenObj(_liveGen)); + _genRefRefs.Enqueue(_genRefRef = new GenRefRef(_liveGen)); _nextGen = false; // this is the ONLY thing that triggers a _liveGen++ } @@ -876,7 +873,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // - the genRefRef weak ref is dead because all snapshots have been collected // in both cases, we will dequeue and collect - var snapshot = new Snapshot(this, _genObj.GetGenRef() + var snapshot = new Snapshot(this, _genRefRef.GetGenRef() #if DEBUG , _logger #endif @@ -933,10 +930,10 @@ namespace Umbraco.Web.PublishedCache.NuCache #if DEBUG _logger.Debug("Collect."); #endif - while (_genObjs.TryPeek(out var genObj) && (genObj.Count == 0 || genObj.WeakGenRef.IsAlive == false)) + while (_genRefRefs.TryPeek(out GenRefRef genRefRef) && (genRefRef.Count == 0 || genRefRef.WGenRef.IsAlive == false)) { - _genObjs.TryDequeue(out genObj); // cannot fail since TryPeek has succeeded - _floorGen = genObj.Gen; + _genRefRefs.TryDequeue(out genRefRef); // cannot fail since TryPeek has succeeded + _floorGen = genRefRef.Gen; #if DEBUG //_logger.Debug("_floorGen=" + _floorGen + ", _liveGen=" + _liveGen); #endif @@ -1012,9 +1009,9 @@ namespace Umbraco.Web.PublishedCache.NuCache await task; } - public long GenCount => _genObjs.Count; + public long GenCount => _genRefRefs.Count; - public long SnapCount => _genObjs.Sum(x => x.Count); + public long SnapCount => _genRefRefs.Sum(x => x.Count); #endregion @@ -1064,6 +1061,24 @@ namespace Umbraco.Web.PublishedCache.NuCache #region Classes + private class LinkedNode + where TValue: class + { + public LinkedNode(TValue value, long gen, LinkedNode next = null) + { + Value = value; + Gen = gen; + Next = next; + } + + internal readonly long Gen; + + // reading & writing references is thread-safe on all .NET platforms + // mark as volatile to ensure we always read the correct value + internal volatile TValue Value; + internal volatile LinkedNode Next; + } + public class Snapshot : IDisposable { private readonly ContentStore _store; @@ -1085,7 +1100,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _store = store; _genRef = genRef; _gen = genRef.Gen; - Interlocked.Increment(ref genRef.GenObj.Count); + Interlocked.Increment(ref genRef.GenRefRef.Count); //_thisCount = _count++; #if DEBUG @@ -1186,15 +1201,49 @@ namespace Umbraco.Web.PublishedCache.NuCache { if (_gen < 0) return; #if DEBUG - _logger.Debug("Dispose snapshot ({Snapshot})", _genRef?.GenObj.Count.ToString() ?? "live"); + _logger.Debug("Dispose snapshot ({Snapshot})", _genRef?.GenRefRef.Count.ToString() ?? "live"); #endif _gen = -1; if (_genRef != null) - Interlocked.Decrement(ref _genRef.GenObj.Count); + Interlocked.Decrement(ref _genRef.GenRefRef.Count); GC.SuppressFinalize(this); } } + internal class GenRefRef + { + public GenRefRef(long gen) + { + Gen = gen; + WGenRef = new WeakReference(null); + } + + public GenRef GetGenRef() + { + // not thread-safe but always invoked from within a lock + var genRef = (GenRef) WGenRef.Target; + if (genRef == null) + WGenRef.Target = genRef = new GenRef(this, Gen); + return genRef; + } + + public readonly long Gen; + public readonly WeakReference WGenRef; + public int Count; + } + + internal class GenRef + { + public GenRef(GenRefRef genRefRef, long gen) + { + GenRefRef = genRefRef; + Gen = gen; + } + + public readonly GenRefRef GenRefRef; + public readonly long Gen; + } + #endregion } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 9c5587fbd5..e19531a25b 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -330,15 +330,14 @@ namespace Umbraco.Web.PublishedCache.NuCache private void LockAndLoadContent(Action action) { - // first get a writer, then a scope - // if there already is a scope, the writer will attach to it - // otherwise, it will only exist here - cheap - using (_contentStore.GetScopedWriteLock(_scopeProvider)) - using (var scope = _scopeProvider.CreateScope()) + using (_contentStore.GetWriter(_scopeProvider)) { - scope.ReadLock(Constants.Locks.ContentTree); - action(scope); - scope.Complete(); + using (var scope = _scopeProvider.CreateScope()) + { + scope.ReadLock(Constants.Locks.ContentTree); + action(scope); + scope.Complete(); + } } } @@ -400,13 +399,14 @@ namespace Umbraco.Web.PublishedCache.NuCache private void LockAndLoadMedia(Action action) { - // see note in LockAndLoadContent - using (_mediaStore.GetScopedWriteLock(_scopeProvider)) - using (var scope = _scopeProvider.CreateScope()) + using (_mediaStore.GetWriter(_scopeProvider)) { - scope.ReadLock(Constants.Locks.MediaTree); - action(scope); - scope.Complete(); + using (var scope = _scopeProvider.CreateScope()) + { + scope.ReadLock(Constants.Locks.MediaTree); + action(scope); + scope.Complete(); + } } } @@ -528,13 +528,14 @@ namespace Umbraco.Web.PublishedCache.NuCache private void LockAndLoadDomains() { - // see note in LockAndLoadContent - using (_domainStore.GetScopedWriteLock(_scopeProvider)) - using (var scope = _scopeProvider.CreateScope()) + using (_domainStore.GetWriter(_scopeProvider)) { - scope.ReadLock(Constants.Locks.Domains); - LoadDomainsLocked(); - scope.Complete(); + using (var scope = _scopeProvider.CreateScope()) + { + scope.ReadLock(Constants.Locks.Domains); + LoadDomainsLocked(); + scope.Complete(); + } } } @@ -586,7 +587,7 @@ namespace Umbraco.Web.PublishedCache.NuCache return; } - using (_contentStore.GetScopedWriteLock(_scopeProvider)) + using (_contentStore.GetWriter(_scopeProvider)) { NotifyLocked(payloads, out bool draftChanged2, out bool publishedChanged2); draftChanged = draftChanged2; @@ -682,7 +683,7 @@ namespace Umbraco.Web.PublishedCache.NuCache return; } - using (_mediaStore.GetScopedWriteLock(_scopeProvider)) + using (_mediaStore.GetWriter(_scopeProvider)) { NotifyLocked(payloads, out bool anythingChanged2); anythingChanged = anythingChanged2; @@ -803,7 +804,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (removedIds.Count == 0 && refreshedIds.Count == 0 && otherIds.Count == 0 && newIds.Count == 0) return; - using (store.GetScopedWriteLock(_scopeProvider)) + using (store.GetWriter(_scopeProvider)) { // ReSharper disable AccessToModifiedClosure action(removedIds, refreshedIds, otherIds, newIds); @@ -824,8 +825,8 @@ namespace Umbraco.Web.PublishedCache.NuCache payload.Removed ? "Removed" : "Refreshed", payload.Id); - using (_contentStore.GetScopedWriteLock(_scopeProvider)) - using (_mediaStore.GetScopedWriteLock(_scopeProvider)) + using (_contentStore.GetWriter(_scopeProvider)) + using (_mediaStore.GetWriter(_scopeProvider)) { // TODO: need to add a datatype lock // this is triggering datatypes reload in the factory, and right after we create some @@ -857,8 +858,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (_isReady == false) return; - // see note in LockAndLoadContent - using (_domainStore.GetScopedWriteLock(_scopeProvider)) + using (_domainStore.GetWriter(_scopeProvider)) { foreach (var payload in payloads) { diff --git a/src/Umbraco.Web/PublishedCache/NuCache/Snap/GenObj.cs b/src/Umbraco.Web/PublishedCache/NuCache/Snap/GenObj.cs deleted file mode 100644 index b69dab7dac..0000000000 --- a/src/Umbraco.Web/PublishedCache/NuCache/Snap/GenObj.cs +++ /dev/null @@ -1,37 +0,0 @@ -using System; -using System.Threading; - -namespace Umbraco.Web.PublishedCache.NuCache.Snap -{ - internal class GenObj - { - public GenObj(long gen) - { - Gen = gen; - WeakGenRef = new WeakReference(null); - } - - public GenRef GetGenRef() - { - // not thread-safe but always invoked from within a lock - var genRef = (GenRef)WeakGenRef.Target; - if (genRef == null) - WeakGenRef.Target = genRef = new GenRef(this); - return genRef; - } - - public readonly long Gen; - public readonly WeakReference WeakGenRef; - public int Count; - - public void Reference() - { - Interlocked.Increment(ref Count); - } - - public void Release() - { - Interlocked.Decrement(ref Count); - } - } -} diff --git a/src/Umbraco.Web/PublishedCache/NuCache/Snap/GenRef.cs b/src/Umbraco.Web/PublishedCache/NuCache/Snap/GenRef.cs deleted file mode 100644 index ade0251b8d..0000000000 --- a/src/Umbraco.Web/PublishedCache/NuCache/Snap/GenRef.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace Umbraco.Web.PublishedCache.NuCache.Snap -{ - internal class GenRef - { - public GenRef(GenObj genObj) - { - GenObj = genObj; - } - - public readonly GenObj GenObj; - public long Gen => GenObj.Gen; - } -} diff --git a/src/Umbraco.Web/PublishedCache/NuCache/Snap/LinkedNode.cs b/src/Umbraco.Web/PublishedCache/NuCache/Snap/LinkedNode.cs deleted file mode 100644 index 20d7e7ddcd..0000000000 --- a/src/Umbraco.Web/PublishedCache/NuCache/Snap/LinkedNode.cs +++ /dev/null @@ -1,20 +0,0 @@ -namespace Umbraco.Web.PublishedCache.NuCache.Snap -{ - internal class LinkedNode - where TValue : class - { - public LinkedNode(TValue value, long gen, LinkedNode next = null) - { - Value = value; - Gen = gen; - Next = next; - } - - public readonly long Gen; - - // reading & writing references is thread-safe on all .NET platforms - // mark as volatile to ensure we always read the correct value - public volatile TValue Value; - public volatile LinkedNode Next; - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs b/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs index c5b1df1206..30f6e7e638 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/SnapDictionary.cs @@ -5,7 +5,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; using Umbraco.Core.Scoping; -using Umbraco.Web.PublishedCache.NuCache.Snap; namespace Umbraco.Web.PublishedCache.NuCache { @@ -21,9 +20,9 @@ namespace Umbraco.Web.PublishedCache.NuCache // This class is optimized for many readers, few writers // Readers are lock-free - private readonly ConcurrentDictionary> _items; - private readonly ConcurrentQueue _genObjs; - private GenObj _genObj; + private readonly ConcurrentDictionary _items; + private readonly ConcurrentQueue _generationObjects; + private GenerationObject _generationObject; private readonly object _wlocko = new object(); private readonly object _rlocko = new object(); private long _liveGen, _floorGen; @@ -41,9 +40,9 @@ namespace Umbraco.Web.PublishedCache.NuCache public SnapDictionary() { - _items = new ConcurrentDictionary>(); - _genObjs = new ConcurrentQueue(); - _genObj = null; // no initial gen exists + _items = new ConcurrentDictionary(); + _generationObjects = new ConcurrentQueue(); + _generationObject = null; // no initial gen exists _liveGen = _floorGen = 0; _nextGen = false; // first time, must create a snapshot _collectAuto = true; // collect automatically by default @@ -87,13 +86,12 @@ namespace Umbraco.Web.PublishedCache.NuCache } // a scope contextual that represents a locked writer to the dictionary - private class ScopedWriteLock : ScopeContextualBase + private class SnapDictionaryWriter : ScopeContextualBase { private readonly WriteLockInfo _lockinfo = new WriteLockInfo(); - private readonly SnapDictionary _dictionary; - private int _released; + private SnapDictionary _dictionary; - public ScopedWriteLock(SnapDictionary dictionary, bool scoped) + public SnapDictionaryWriter(SnapDictionary dictionary, bool scoped) { _dictionary = dictionary; dictionary.Lock(_lockinfo, scoped); @@ -101,19 +99,17 @@ namespace Umbraco.Web.PublishedCache.NuCache public override void Release(bool completed) { - if (Interlocked.CompareExchange(ref _released, 1, 0) != 0) - return; + if (_dictionary == null) return; _dictionary.Release(_lockinfo, completed); + _dictionary = null; } } // gets a scope contextual representing a locked writer to the dictionary - // the dict is write-locked until the write-lock is released - // which happens when it is disposed (non-scoped) - // or when the scope context exits (scoped) - public IDisposable GetScopedWriteLock(IScopeProvider scopeProvider) + // GetScopedWriter? should the dict have a ref onto the scope provider? + public IDisposable GetWriter(IScopeProvider scopeProvider) { - return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped)); + return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new SnapDictionaryWriter(this, scoped)); } private void Lock(WriteLockInfo lockInfo, bool forceGen = false) @@ -133,18 +129,14 @@ namespace Umbraco.Web.PublishedCache.NuCache //RuntimeHelpers.PrepareConstrainedRegions(); 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 && _wlocked == 1)) // if true already... ok to have "holes" in generation objects { // because we are changing things, a new generation // is created, which will trigger a new snapshot - if (_nextGen) - _genObjs.Enqueue(_genObj = new GenObj(_liveGen)); + _nextGen = true; _liveGen += 1; - _nextGen = true; // this is the ONLY place where _nextGen becomes true } } } @@ -161,10 +153,6 @@ namespace Umbraco.Web.PublishedCache.NuCache private void Release(WriteLockInfo lockInfo, bool commit = true) { - // if the lock wasn't taken in the first place, do nothing - if (!lockInfo.Taken) - return; - if (commit == false) { var rtaken = false; @@ -173,7 +161,6 @@ namespace Umbraco.Web.PublishedCache.NuCache Monitor.Enter(_rlocko, ref rtaken); try { } finally { - // forget about the temp. liveGen _nextGen = false; _liveGen -= 1; } @@ -196,9 +183,8 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - // decrement the lock count, if counting, then exit the lock if (lockInfo.Count) _wlocked--; - Monitor.Exit(_wlocko); + if (lockInfo.Taken) Monitor.Exit(_wlocko); } private void Release(ReadLockInfo lockInfo) @@ -212,9 +198,9 @@ namespace Umbraco.Web.PublishedCache.NuCache public int Count => _items.Count; - private LinkedNode GetHead(TKey key) + private LinkedNode GetHead(TKey key) { - _items.TryGetValue(key, out var link); // else null + _items.TryGetValue(key, out LinkedNode link); // else null return link; } @@ -235,7 +221,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // 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); + _items.TryUpdate(key, new LinkedNode(value, _liveGen, link), link); } else { @@ -249,7 +235,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } else { - _items.TryAdd(key, new LinkedNode(value, _liveGen)); + _items.TryAdd(key, new LinkedNode(value, _liveGen)); } } finally @@ -275,7 +261,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { if (kvp.Value.Gen < _liveGen) { - var link = new LinkedNode(null, _liveGen, kvp.Value); + var link = new LinkedNode(null, _liveGen, kvp.Value); _items.TryUpdate(kvp.Key, link, kvp.Value); } else @@ -351,12 +337,12 @@ namespace Umbraco.Web.PublishedCache.NuCache { 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) - return new Snapshot(this, _genObj.GetGenRef()); + // if no next generation is required, and we already have one, + // use it and create a new snapshot + if (_nextGen == false && _generationObject != null) + return new Snapshot(this, _generationObject.GetReference()); - // else we need to try to create a new gen object + // 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 -- @@ -364,32 +350,29 @@ namespace Umbraco.Web.PublishedCache.NuCache // write-locked, cannot use latest gen (at least 1) so use previous var snapGen = _nextGen ? _liveGen - 1 : _liveGen; - // create a new gen object if we don't already have one - // (happens the first time a snapshot is created) - if (_genObj == null) - _genObjs.Enqueue(_genObj = new GenObj(snapGen)); - - // if we have one already, ensure it's consistent - else if (_genObj.Gen != snapGen) + // create a new gen ref unless we already have it + if (_generationObject == null) + _generationObjects.Enqueue(_generationObject = new GenerationObject(snapGen)); + else if (_generationObject.Gen != snapGen) throw new Exception("panic"); } else { - // not write-locked, can use latest gen (_liveGen), create a corresponding new gen object - _genObjs.Enqueue(_genObj = new GenObj(_liveGen)); + // not write-locked, can use latest gen, create a new gen ref + _generationObjects.Enqueue(_generationObject = new GenerationObject(_liveGen)); _nextGen = false; // this is the ONLY thing that triggers a _liveGen++ } // so... - // the genObj has a weak ref to the genRef, and is queued - // the snapshot has a ref to the genRef, which has a ref to the genObj - // when the snapshot is disposed, it decreases genObj counter + // the genRefRef has a weak ref to the genRef, and is queued + // the snapshot has a ref to the genRef, which has a ref to the genRefRef + // when the snapshot is disposed, it decreases genRefRef counter // so after a while, one of these conditions is going to be true: - // - genObj.Count is zero because all snapshots have properly been disposed - // - genObj.WeakGenRef is dead because all snapshots have been collected + // - the genRefRef counter is zero because all snapshots have properly been disposed + // - the genRefRef weak ref is dead because all snapshots have been collected // in both cases, we will dequeue and collect - var snapshot = new Snapshot(this, _genObj.GetGenRef()); + var snapshot = new Snapshot(this, _generationObject.GetReference()); // reading _floorGen is safe if _collectTask is null if (_collectTask == null && _collectAuto && _liveGen - _floorGen > CollectMinGenDelta) @@ -433,16 +416,16 @@ namespace Umbraco.Web.PublishedCache.NuCache private void Collect() { // see notes in CreateSnapshot - while (_genObjs.TryPeek(out var genObj) && (genObj.Count == 0 || genObj.WeakGenRef.IsAlive == false)) + while (_generationObjects.TryPeek(out GenerationObject generationObject) && (generationObject.Count == 0 || generationObject.WeakReference.IsAlive == false)) { - _genObjs.TryDequeue(out genObj); // cannot fail since TryPeek has succeeded - _floorGen = genObj.Gen; + _generationObjects.TryDequeue(out generationObject); // cannot fail since TryPeek has succeeded + _floorGen = generationObject.Gen; } Collect(_items); } - private void Collect(ConcurrentDictionary> dict) + private void Collect(ConcurrentDictionary dict) { // it is OK to enumerate a concurrent dictionary and it does not lock // it - and here it's not an issue if we skip some items, they will be @@ -477,7 +460,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // not live, null value, no next link = remove that one -- but only if // the dict has not been updated, have to do it via ICollection<> (thanks // Mr Toub) -- and if the dict has been updated there is nothing to collect - var idict = dict as ICollection>>; + var idict = dict as ICollection>; /*var removed =*/ idict.Remove(kvp); //Console.WriteLine("remove (" + (removed ? "true" : "false") + ")"); continue; @@ -502,14 +485,14 @@ namespace Umbraco.Web.PublishedCache.NuCache { task = _collectTask; } - return task ?? Task.CompletedTask; + return task ?? Task.FromResult(0); //if (task != null) // await task; } - public long GenCount => _genObjs.Count; + public long GenCount => _generationObjects.Count; - public long SnapCount => _genObjs.Sum(x => x.Count); + public long SnapCount => _generationObjects.Sum(x => x.Count); #endregion @@ -530,7 +513,6 @@ 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 CollectAuto { @@ -538,15 +520,13 @@ namespace Umbraco.Web.PublishedCache.NuCache set => _dict._collectAuto = value; } - public GenObj GenObj => _dict._genObj; - - public ConcurrentQueue GenObjs => _dict._genObjs; + public ConcurrentQueue GenerationObjects => _dict._generationObjects; public Snapshot LiveSnapshot => new Snapshot(_dict, _dict._liveGen); public GenVal[] GetValues(TKey key) { - _dict._items.TryGetValue(key, out var link); // else null + _dict._items.TryGetValue(key, out LinkedNode link); // else null if (link == null) return new GenVal[0]; @@ -579,23 +559,35 @@ namespace Umbraco.Web.PublishedCache.NuCache #region Classes + private class LinkedNode + { + public LinkedNode(TValue value, long gen, LinkedNode next = null) + { + Value = value; + Gen = gen; + Next = next; + } + + internal readonly long Gen; + + // reading & writing references is thread-safe on all .NET platforms + // mark as volatile to ensure we always read the correct value + internal volatile TValue Value; + internal volatile LinkedNode Next; + } + public class Snapshot : IDisposable { private readonly SnapDictionary _store; - private readonly GenRef _genRef; - private readonly long _gen; // copied for perfs - private int _disposed; + private readonly GenerationReference _generationReference; + private long _gen; // copied for perfs - //private static int _count; - //private readonly int _thisCount; - - internal Snapshot(SnapDictionary store, GenRef genRef) + internal Snapshot(SnapDictionary store, GenerationReference generationReference) { _store = store; - _genRef = genRef; - _gen = genRef.GenObj.Gen; - _genRef.GenObj.Reference(); - //_thisCount = _count++; + _generationReference = generationReference; + _gen = generationReference.GenerationObject.Gen; + _generationReference.GenerationObject.Reference(); } internal Snapshot(SnapDictionary store, long gen) @@ -604,21 +596,17 @@ namespace Umbraco.Web.PublishedCache.NuCache _gen = gen; } - private void EnsureNotDisposed() - { - if (_disposed > 0) - throw new ObjectDisposedException("snapshot" /*+ " (" + _thisCount + ")"*/); - } - public TValue Get(TKey key) { - EnsureNotDisposed(); + if (_gen < 0) + throw new ObjectDisposedException("snapshot" /*+ " (" + _thisCount + ")"*/); return _store.Get(key, _gen); } public IEnumerable GetAll() { - EnsureNotDisposed(); + if (_gen < 0) + throw new ObjectDisposedException("snapshot" /*+ " (" + _thisCount + ")"*/); return _store.GetAll(_gen); } @@ -626,7 +614,8 @@ namespace Umbraco.Web.PublishedCache.NuCache { get { - EnsureNotDisposed(); + if (_gen < 0) + throw new ObjectDisposedException("snapshot" /*+ " (" + _thisCount + ")"*/); return _store.IsEmpty(_gen); } } @@ -635,20 +624,63 @@ namespace Umbraco.Web.PublishedCache.NuCache { get { - EnsureNotDisposed(); + if (_gen < 0) + throw new ObjectDisposedException("snapshot" /*+ " (" + _thisCount + ")"*/); return _gen; } } public void Dispose() { - if (Interlocked.CompareExchange(ref _disposed, 1, 0) != 0) - return; - _genRef?.GenObj.Release(); + if (_gen < 0) return; + _gen = -1; + _generationReference?.GenerationObject.Release(); GC.SuppressFinalize(this); } } + internal class GenerationObject + { + public GenerationObject(long gen) + { + Gen = gen; + WeakReference = new WeakReference(null); + } + + public GenerationReference GetReference() + { + // not thread-safe but always invoked from within a lock + var generationReference = (GenerationReference) WeakReference.Target; + if (generationReference == null) + WeakReference.Target = generationReference = new GenerationReference(this); + return generationReference; + } + + public readonly long Gen; + public readonly WeakReference WeakReference; + public int Count; + + public void Reference() + { + Interlocked.Increment(ref Count); + } + + public void Release() + { + Interlocked.Decrement(ref Count); + } + } + + internal class GenerationReference + { + public GenerationReference(GenerationObject generationObject) + { + GenerationObject = generationObject; + } + + public readonly GenerationObject GenerationObject; + } + #endregion } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index c6cbc7cbaa..09cc7d856a 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -205,9 +205,6 @@ - - -