From d2f7c11e89742b0daf4adaae5edcbc3afca71282 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 21 Aug 2020 00:02:35 +1000 Subject: [PATCH] Fixes issue with scopes being disposed or referenced incorrectly --- .../UmbracoContentIndex.cs | 4 ++-- .../Search/ExamineComponent.cs | 22 ++++++++++++++++++- .../Sync/DatabaseServerMessenger.cs | 13 ++++++----- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs b/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs index 904bf68623..dc2a5570a6 100644 --- a/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs +++ b/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs @@ -86,7 +86,7 @@ namespace Umbraco.Examine || !validator.ValidateProtectedContent(path, v.Category)) ? 0 : 1; - }); + }).ToList(); var hasDeletes = false; var hasUpdates = false; @@ -105,7 +105,7 @@ namespace Umbraco.Examine { hasUpdates = true; //these are the valid ones, so just index them all at once - base.PerformIndexItems(group, onComplete); + base.PerformIndexItems(group.ToList(), onComplete); } } diff --git a/src/Umbraco.Infrastructure/Search/ExamineComponent.cs b/src/Umbraco.Infrastructure/Search/ExamineComponent.cs index 037981d8b4..35a8804ecb 100644 --- a/src/Umbraco.Infrastructure/Search/ExamineComponent.cs +++ b/src/Umbraco.Infrastructure/Search/ExamineComponent.cs @@ -583,6 +583,26 @@ namespace Umbraco.Web.Search public static void Execute(ExamineComponent examineComponent, IContent content, bool isPublished) { + // TODO: This is ugly, it is going to build the value set 3x and for 2 of those times it will be the same value + // set. We can do better. + + // TODO: We are .ToList() ing each of the calls to GetValueSets here. This is currently required but isn't the way + // that this was intended to work. Ideally, the IEnumerable package gets passed to Examine and it is only iterated/executed + // when the indexing takes place which would occur on a background thread. This is problematic with how the ContentValueSetBuilder + // in combination with UmbracoContentIndex.PerformIndexItems works because (at least what I've come to believe) we are using yield + // return in the GetValueSets call in combination with trying to lazily resolve the enumerable but because we GroupBy in + // UmbracoContentIndex.PerformIndexItems it's eagerly executed but then lazily executed again on the background thread and I believe + // that in doing this when the call is made to _userService.GetProfilesById it's trying to resolve a scope from an AsyncLocal instance + // that has already been disposed higher up it's chain. I 'think' to how the eager/lazy enumeration happens with yield return that it's + // capturing a scope/AsyncLocal instance that it shouldn't really be using. + + // TODO: We don't want these value sets to be eagerly built in this thread since this is most likely going to be a request thread. + // This is why the lazy execution of the Enumerable had the intended affect of executing only when requested on the background thread. + // This could still be acheived: Either we have a custom Enumerable/Enumerator to do this, or we simply call the below code + // on a background thread... which would be much easier! + + // TODO: I think this is an issue in v8 too! + foreach (var index in examineComponent._examineManager.Indexes.OfType() //filter the indexers .Where(x => isPublished || !x.PublishedValuesOnly) @@ -593,7 +613,7 @@ namespace Umbraco.Web.Search ? examineComponent._publishedContentValueSetBuilder : (IValueSetBuilder)examineComponent._contentValueSetBuilder; - index.IndexItems(builder.GetValueSets(content)); + index.IndexItems(builder.GetValueSets(content).ToList()); } } } diff --git a/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs index c915013162..2f6bb61e42 100644 --- a/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs +++ b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs @@ -63,6 +63,13 @@ namespace Umbraco.Core.Sync _lastPruned = _lastSync = DateTime.UtcNow; _syncIdle = new ManualResetEvent(true); _distCacheFilePath = new Lazy(() => GetDistCacheFilePath(hostingEnvironment)); + + // See notes on LocalIdentity + LocalIdentity = NetworkHelper.MachineName // eg DOMAIN\SERVER + + "/" + _hostingEnvironment.ApplicationId // eg /LM/S3SVC/11/ROOT + + " [P" + Process.GetCurrentProcess().Id // eg 1234 + + "/D" + AppDomain.CurrentDomain.Id // eg 22 + + "] " + Guid.NewGuid().ToString("N").ToUpper(); // make it truly unique } protected ILogger Logger { get; } @@ -526,11 +533,7 @@ namespace Umbraco.Core.Sync /// Practically, all we really need is the guid, the other infos are here for information /// and debugging purposes. /// - protected string LocalIdentity => NetworkHelper.MachineName // eg DOMAIN\SERVER - + "/" + _hostingEnvironment.ApplicationId // eg /LM/S3SVC/11/ROOT - + " [P" + Process.GetCurrentProcess().Id // eg 1234 - + "/D" + AppDomain.CurrentDomain.Id // eg 22 - + "] " + Guid.NewGuid().ToString("N").ToUpper(); // make it truly unique + protected string LocalIdentity { get; } private string GetDistCacheFilePath(IHostingEnvironment hostingEnvironment) {