Use a dedicated Lock for the export claim, not Umbraco's dictionary

uSync.BackOffice/SyncHandlers/Handlers/PublishableContentHandlerBase.cs

@@ -135,6 +135,20 @@ protected async Task ProcessItem(EnumerableObjectNotification<TObject> notificat
         }
     }
 
+    /// <summary>
+    ///  Guards the claim below.
+    /// </summary>
+    /// <remarks>
+    ///  the notification state we track claims in is Umbraco's dictionary, not ours, so we can't
+    ///  lock on it - anything else holding a reference could contend with us on an object neither
+    ///  side knows the other is using. This is our own lock instead.
+    ///  <para>
+    ///   statics on a generic type are per closed type, so documents and elements get one each.
+    ///   That suits us: they never share a notification state, so they have nothing to contend over.
+    ///  </para>
+    /// </remarks>
+    private static readonly Lock _claimLock = new();
+
     /// <summary>
     ///  Claim an item for export, returning false if it has already been exported in this operation.
     /// </summary>
@@ -155,12 +169,18 @@ protected async Task ProcessItem(EnumerableObjectNotification<TObject> notificat
     ///   persisted the item - including its publish state - so the export reflects the finished
     ///   operation whichever notification triggers it.
     ///  </para>
+    ///  <para>
+    ///   Umbraco raises the notifications for one operation one after another, so the lock is
+    ///   belt-and-braces rather than something we expect to contend on - but the state dictionary
+    ///   is a plain Dictionary, and SyncScopedNotificationPublisher can dispatch on a queued
+    ///   background thread when BackgroundNotifications is on.
+    ///  </para>
     /// </remarks>
     internal static bool ClaimItemForExport(EnumerableObjectNotification<TObject> notification, TObject item)
     {
         var state = notification.State;
 
-        lock (state)
+        lock (_claimLock)
         {
             if (state.TryGetValue(uSync.EventExportedItemsKey, out var value) is false
                 || value is not HashSet<Guid> exported)

uSync.Tests/SyncHandlers/ExportDeduplicationTests.cs

@@ -1,5 +1,7 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
 
 using Moq;
 
@@ -135,6 +137,35 @@ public void A_Later_Operation_Claims_The_Item_Again()
         });
     }
 
+    [Test]
+    public void Concurrent_Claims_Only_Let_One_Caller_Through()
+    {
+        // Umbraco raises the notifications for an operation one after another, so this shouldn't
+        // happen in practice - but the state dictionary is a plain Dictionary, and the claim is
+        // what stops a torn read of it turning into a duplicate export. Hammer it to prove the
+        // lock does its job.
+        const int itemCount = 50;
+        const int threadsPerItem = 8;
+
+        var items = Enumerable.Range(0, itemCount).Select(_ => MakeContent(Guid.NewGuid())).ToArray();
+        var messages = new EventMessages();
+        var state = new Dictionary<string, object>();
+
+        // every thread gets its own notification, all sharing one state - as they do in an operation.
+        var claims = items
+            .SelectMany(item => Enumerable.Range(0, threadsPerItem).Select(_ => (Item: item, Notification: new ContentSavedNotification(item, messages) { State = state })))
+            .ToArray();
+
+        var results = new bool[claims.Length];
+
+        Parallel.For(0, claims.Length, i =>
+            results[i] = PublishableContentHandlerBase<IContent>.ClaimItemForExport(claims[i].Notification, claims[i].Item));
+
+        Assert.That(
+            results.Count(x => x), Is.EqualTo(itemCount),
+            "exactly one claim per item should succeed, however many threads race for it");
+    }
+
     [Test]
     public void Existing_Notification_State_Is_Preserved()
     {