Performance: Fix thread safety and optimize cache updates in PublishStatusService after content changes (#21415)

* Resolved potential thread safety issues with PublishStatusService.

* Only update published status in content cache refresher if within a publish or unpublish operation.

src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs

@@ -399,7 +399,6 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
 
     private async Task HandlePublishedAsync(JsonPayload payload, CancellationToken cancellationToken)
     {
-
         if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll))
         {
             await _publishStatusManagementService.InitializeAsync(cancellationToken);
@@ -414,16 +413,20 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
         {
             await _publishStatusManagementService.RemoveAsync(payload.Key.Value, cancellationToken);
         }
-        else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshNode))
+        else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshNode) && HasPublishStatusUpdates(payload))
         {
             await _publishStatusManagementService.AddOrUpdateStatusAsync(payload.Key.Value, cancellationToken);
         }
-        else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch))
+        else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch) && HasPublishStatusUpdates(payload))
         {
             await _publishStatusManagementService.AddOrUpdateStatusWithDescendantsAsync(payload.Key.Value, cancellationToken);
         }
     }
 
+    private static bool HasPublishStatusUpdates(JsonPayload payload) =>
+        (payload.PublishedCultures is not null && payload.PublishedCultures.Length > 0) ||
+        (payload.UnpublishedCultures is not null && payload.UnpublishedCultures.Length > 0);
+
     private void HandleIdKeyMap(JsonPayload payload)
     {
         // We only need to flush the ID/Key map when content is deleted.

src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs

@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using Microsoft.Extensions.Logging;
 using Umbraco.Cms.Core.Persistence.Repositories;
 using Umbraco.Cms.Core.Scoping;
@@ -16,7 +17,7 @@ public class PublishStatusService : IPublishStatusManagementService, IPublishSta
     private readonly ILanguageService _languageService;
     private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
 
-    private readonly IDictionary<Guid, ISet<string>> _publishedCultures = new Dictionary<Guid, ISet<string>>();
+    private readonly ConcurrentDictionary<Guid, ISet<string>> _publishedCultures = new();
 
     private string? DefaultCulture { get; set; }
 
@@ -126,7 +127,7 @@ public class PublishStatusService : IPublishStatusManagementService, IPublishSta
     /// <inheritdoc/>
     public Task RemoveAsync(Guid documentKey, CancellationToken cancellationToken)
     {
-        _publishedCultures.Remove(documentKey);
+        _publishedCultures.TryRemove(documentKey, out _);
         return Task.CompletedTask;
     }
 

tests/Umbraco.Tests.Integration/Umbraco.Core/Services/PublishStatusServiceTests.ThreadSafety.cs

@@ -0,0 +1,185 @@
+using NUnit.Framework;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Services;
+
+namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services;
+
+internal sealed partial class PublishStatusServiceTests
+{
+    [Test]
+    public async Task Concurrent_Reads_And_Writes_Do_Not_Throw()
+    {
+        // Arrange
+        var sut = CreatePublishedStatusService();
+        await sut.InitializeAsync(CancellationToken.None);
+
+        const int numberOfOperations = 1000;
+        var documentKeys = Enumerable.Range(0, 100).Select(_ => Guid.NewGuid()).ToArray();
+        var exceptions = new List<Exception>();
+        var lockObj = new object();
+
+        // Act - run concurrent reads and writes
+        var tasks = new List<Task>();
+
+        // Writers - AddOrUpdateStatusAsync
+        for (var i = 0; i < numberOfOperations; i++)
+        {
+            var key = documentKeys[i % documentKeys.Length];
+            tasks.Add(RunWithSuppressedExecutionContext(async () =>
+            {
+                try
+                {
+                    await sut.AddOrUpdateStatusAsync(key, CancellationToken.None);
+                }
+                catch (Exception ex)
+                {
+                    lock (lockObj)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }
+            }));
+        }
+
+        // Readers - IsDocumentPublished
+        for (var i = 0; i < numberOfOperations; i++)
+        {
+            var key = documentKeys[i % documentKeys.Length];
+            tasks.Add(RunWithSuppressedExecutionContext(() =>
+            {
+                try
+                {
+                    _ = sut.IsDocumentPublished(key, DefaultCulture);
+                }
+                catch (Exception ex)
+                {
+                    lock (lockObj)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }
+
+                return Task.CompletedTask;
+            }));
+        }
+
+        // Readers - IsDocumentPublishedInAnyCulture
+        for (var i = 0; i < numberOfOperations; i++)
+        {
+            var key = documentKeys[i % documentKeys.Length];
+            tasks.Add(RunWithSuppressedExecutionContext(() =>
+            {
+                try
+                {
+                    _ = sut.IsDocumentPublishedInAnyCulture(key);
+                }
+                catch (Exception ex)
+                {
+                    lock (lockObj)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }
+
+                return Task.CompletedTask;
+            }));
+        }
+
+        // Removers
+        for (var i = 0; i < numberOfOperations; i++)
+        {
+            var key = documentKeys[i % documentKeys.Length];
+            tasks.Add(RunWithSuppressedExecutionContext(async () =>
+            {
+                try
+                {
+                    await sut.RemoveAsync(key, CancellationToken.None);
+                }
+                catch (Exception ex)
+                {
+                    lock (lockObj)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }
+            }));
+        }
+
+        await Task.WhenAll(tasks);
+
+        // Assert
+        Assert.IsEmpty(exceptions, $"Expected no exceptions but got {exceptions.Count}: {string.Join(", ", exceptions.Select(e => e.Message))}");
+    }
+
+    [Test]
+    public async Task Concurrent_Initialize_And_Queries_Do_Not_Throw()
+    {
+        // Arrange
+        var sut = CreatePublishedStatusService();
+
+        // Publish some content first so InitializeAsync has data to load
+        ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
+
+        const int numberOfOperations = 100;
+        var exceptions = new List<Exception>();
+        var lockObj = new object();
+
+        // Act - run concurrent initializations and queries
+        var tasks = new List<Task>();
+
+        // Multiple initializations (simulates cache rebuild)
+        for (var i = 0; i < 10; i++)
+        {
+            tasks.Add(RunWithSuppressedExecutionContext(async () =>
+            {
+                try
+                {
+                    await sut.InitializeAsync(CancellationToken.None);
+                }
+                catch (Exception ex)
+                {
+                    lock (lockObj)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }
+            }));
+        }
+
+        // Concurrent reads during initialization
+        for (var i = 0; i < numberOfOperations; i++)
+        {
+            tasks.Add(RunWithSuppressedExecutionContext(() =>
+            {
+                try
+                {
+                    _ = sut.IsDocumentPublished(Textpage.Key, DefaultCulture);
+                    _ = sut.IsDocumentPublishedInAnyCulture(Subpage.Key);
+                    _ = sut.HasPublishedAncestorPath(Subpage2.Key);
+                }
+                catch (Exception ex)
+                {
+                    lock (lockObj)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }
+
+                return Task.CompletedTask;
+            }));
+        }
+
+        await Task.WhenAll(tasks);
+
+        // Assert
+        Assert.IsEmpty(exceptions, $"Expected no exceptions but got {exceptions.Count}: {string.Join(", ", exceptions.Select(e => e.Message))}");
+    }
+
+    private static Task RunWithSuppressedExecutionContext(Func<Task> action)
+    {
+        using (ExecutionContext.SuppressFlow())
+        {
+            return Task.Run(action);
+        }
+    }
+}

tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj

@@ -280,6 +280,9 @@
     <Compile Update="Umbraco.Core\Services\PublishStatusServiceTests.Query.cs">
       <DependentUpon>PublishStatusServiceTests.cs</DependentUpon>
     </Compile>
+    <Compile Update="Umbraco.Core\Services\PublishStatusServiceTests.ThreadSafety.cs">
+      <DependentUpon>PublishStatusServiceTests.cs</DependentUpon>
+    </Compile>
     <Compile Update="ManagementApi\Services\UserStartNodeEntitiesServiceTests.ChildUserAccessEntities.cs">
       <DependentUpon>UserStartNodeEntitiesServiceTests.cs</DependentUpon>
     </Compile>