Fix the intermittent Interop unit test failures

src/componentsBase/BaseRendererControl.cs

@@ -152,6 +152,15 @@ internal DataSourceManager DataSourceManager
         }
         private LinkedList<RendererMessage> _messageQueue = new LinkedList<RendererMessage>();
 
+        /// <summary>
+        /// Guards <see cref="_messageQueue"/> and the <c>_updateQueued</c> flag deciding when it is
+        /// flushed. Two paths flush it - <c>SendMessageImmediate</c> inline on the caller's thread,
+        /// <c>QueueUpdate</c> on the renderer's - so a component driven from off the Blazor
+        /// dispatcher (a timer, a bound collection filled in the background) gets both at once,
+        /// which tore the list apart: a throw, a lost message, or two arriving swapped.
+        /// </summary>
+        private readonly object _messageQueueLock = new object();
+
         private string _containerId = Guid.NewGuid().ToString();
 
         internal string ContainerId
@@ -845,7 +854,7 @@ private void SendDescriptionMessage()
             }
             RendererMessage m = new RendererMessage();
             m.Type = ("description");
-            _messageQueue.AddLast(m);
+            Enqueue(m);
             QueueUpdate();
         }
 
@@ -947,6 +956,9 @@ public string Serialize()
         private Dictionary<long, Object> _methodReturns = new Dictionary<long, Object>();
         private Object _semLock = new Object();
 
+        // Deliberately shared by every instance: the WebView hosts are handed a return with only
+        // the invokeId to match it by. Incremented from whichever thread invokes, and a plain ++
+        // can hand two calls one id, which collides in _methodTasks.
         static long _invokeId = 0;
         protected async Task<object> InvokeMethod(string methodName, object[] arguments, string[] types, ElementReference[] nativeElements = null)
         {
@@ -988,7 +1000,7 @@ internal object InvokeMethodHelperSync(string target, string methodName, object[
             m.Type = ("invokeMethod");
             string[] args = new string[arguments.Length];
             string[] typeStrings = new string[arguments.Length];
-            long invokeId = _invokeId++;
+            long invokeId = Interlocked.Increment(ref _invokeId);
             for (int i = 0; i < arguments.Length; i++)
             {
                 args[i] = GetStringArg(arguments[i], types[i]);
@@ -1038,7 +1050,7 @@ internal async Task<object> InvokeMethodHelper(string target, string methodName,
             m.Type = ("invokeMethod");
             string[] args = new string[arguments.Length];
             string[] typeStrings = new string[arguments.Length];
-            long invokeId = _invokeId++;
+            long invokeId = Interlocked.Increment(ref _invokeId);
             for (int i = 0; i < arguments.Length; i++)
             {
                 args[i] = GetStringArg(arguments[i], types[i]);
@@ -1567,7 +1579,7 @@ private void SendMessage(RendererMessage m)
                 return;
             }
             //Console.WriteLine("sending message");
-            _messageQueue.AddLast(m);
+            Enqueue(m);
             QueueUpdate();
         }
 
@@ -1592,47 +1604,73 @@ private object SendMessageSyncImmediate(RendererMessage m)
             return SendJsonImmediateSync(m);
         }
 
-        private void QueueUpdate()
+        private void Enqueue(RendererMessage m)
         {
-            if (!_updateQueued && _ready)
+            lock (_messageQueueLock)
             {
-                _updateQueued = true;
-                Task.Delay(0).ContinueWith((t) => InvokeAsync(Update));
+                _messageQueue.AddLast(m);
             }
         }
 
-        private void Update()
+        private void QueueUpdate()
         {
-            this._updateQueued = false;
-
-            if (!_ready)
+            bool schedule = false;
+            lock (_messageQueueLock)
             {
-                return;
+                if (!_updateQueued && _ready)
+                {
+                    _updateQueued = true;
+                    schedule = true;
+                }
             }
 
-            //Console.WriteLine("updateing: " + this.GetType().Name + " " + _messageQueue.Count);
-            while (_messageQueue.Count > 0)
+            if (schedule)
             {
-                RendererMessage m = _messageQueue.First.Value;
-                _messageQueue.RemoveFirst();
-                ProcessMessage(m);
+                Task.Delay(0).ContinueWith((t) => InvokeAsync(Update));
             }
         }
 
-        private void UpdateSync()
+        private void Update()
         {
-            this._updateQueued = false;
-
-            if (!_ready)
+            // Spans the whole drain, not just the dequeue: two flushes that each took one message
+            // would still reach the client in whichever order they got there. Reentrant, so
+            // processing a message that queues another is fine.
+            lock (_messageQueueLock)
             {
-                return;
+                this._updateQueued = false;
+
+                if (!_ready)
+                {
+                    return;
+                }
+
+                //Console.WriteLine("updateing: " + this.GetType().Name + " " + _messageQueue.Count);
+                while (_messageQueue.Count > 0)
+                {
+                    RendererMessage m = _messageQueue.First.Value;
+                    _messageQueue.RemoveFirst();
+                    ProcessMessage(m);
+                }
             }
+        }
 
-            while (_messageQueue.Count > 0)
+        private void UpdateSync()
+        {
+            lock (_messageQueueLock)
             {
-                RendererMessage m = _messageQueue.First.Value;
-                _messageQueue.RemoveFirst();
-                ProcessMessageSync(m);
+                this._updateQueued = false;
+
+                if (!_ready)
+                {
+                    return;
+                }
+
+                while (_messageQueue.Count > 0)
+                {
+                    RendererMessage m = _messageQueue.First.Value;
+                    _messageQueue.RemoveFirst();
+                    ProcessMessageSync(m);
+                }
             }
         }
 
@@ -3181,7 +3219,10 @@ private async Task TrySendCleanupAsync()
                 RendererMessage m = new RendererMessage();
                 m.Type = ("cleanup");
 
-                _messageQueue.Clear();
+                lock (_messageQueueLock)
+                {
+                    _messageQueue.Clear();
+                }
                 await SendMessageImmediate(m).ConfigureAwait(false);
             }
             catch (JSDisconnectedException ex)

tests/IgniteUI.Blazor.Tests/BlazorComponentTestBase.cs

@@ -25,6 +25,7 @@ protected BlazorComponentTestBase()
     {
         JSInterop.Mode = JSRuntimeMode.Loose;
         Interop = InteropHarnessRegistry.CreateDefault(JSInterop);
+        Interop.UseDispatcher(() => Renderer.Dispatcher);
         Interop.ConfigureServices(Services);
         IgniteUIBlazor = Interop.Service;
     }
@@ -51,6 +52,7 @@ protected InteropHarness InteropFor(Type componentType)
         if (!_overrideHarnesses.TryGetValue(factory, out var harness))
         {
             harness = factory(JSInterop);
+            harness.UseDispatcher(() => Renderer.Dispatcher);
             harness.ConfigureServices(Services);
             _overrideHarnesses[factory] = harness;
         }

tests/IgniteUI.Blazor.Tests/ComponentWithContractTestBase.cs

@@ -174,7 +174,9 @@ private static async Task RunSpec(
             ? $"no new current-state read was issued for \"{method.ReadsProperty}\""
             : $"\"{method.JsName}\" sent no new invocation";
 
-        var (call, result) = await InvokeExpectingNewCall(matching, () => method.Invoke(cut.Instance), noNewCall);
+        // On the dispatcher, as application code calling a component API is - see OnDispatcher.
+        var (call, result) = await InvokeExpectingNewCall(
+            matching, () => harness.OnDispatcher(() => method.Invoke(cut.Instance)), noNewCall);
         AssertObserved(harness, cut, scope, method, call, result);
 
         if (method.SyncInvoke is not null)
@@ -183,7 +185,7 @@ private static async Task RunSpec(
             // (the stub persists) to the same result.
             var (syncCall, syncResult) = await InvokeExpectingNewCall(
                 matching,
-                () => Task.FromResult(method.SyncInvoke(cut.Instance)),
+                () => Task.FromResult(harness.OnDispatcher(() => method.SyncInvoke(cut.Instance))),
                 "sync twin: " + noNewCall);
             AssertObserved(harness, cut, scope, method, syncCall, syncResult);
         }

tests/IgniteUI.Blazor.Tests/Interop/InteropHarness.cs

@@ -98,6 +98,35 @@ private InteropReturn(InteropReturnKind kind, object? value = null, string? type
 /// </summary>
 public abstract class InteropHarness
 {
+    private Func<Dispatcher>? _dispatcher;
+
+    /// <summary>
+    /// Supplies the renderer's dispatcher, resolved on use: the harness is built while the test's
+    /// services are still being configured, and asking for the renderer then settles the container.
+    /// </summary>
+    internal void UseDispatcher(Func<Dispatcher> dispatcher) => _dispatcher = dispatcher;
+
+    /// <summary>
+    /// Runs <paramref name="work"/> on the renderer's dispatcher, where Blazor delivers real
+    /// JS-to-.NET calls and application code invokes component APIs. Anything that makes a component
+    /// transmit belongs here: off the dispatcher the send races the renderer's own flush, and bUnit
+    /// records both on one unsynchronized list, so a raced message can be lost outright.
+    /// </summary>
+    public void OnDispatcher(Action work) =>
+        Dispatcher().InvokeAsync(work).GetAwaiter().GetResult();
+
+    /// <summary>
+    /// <inheritdoc cref="OnDispatcher(Action)" path="/summary"/>
+    /// Hands back a still-running task instead of awaiting it on the dispatcher, which would hold
+    /// the dispatcher until it completes and deadlock a deferred return needing it to get there.
+    /// An interop call transmits before it yields, so the send still happens here.
+    /// </summary>
+    public T OnDispatcher<T>(Func<T> work) => Dispatcher().InvokeAsync(work).GetAwaiter().GetResult();
+
+    private Dispatcher Dispatcher() =>
+        (_dispatcher ?? throw new InvalidOperationException(
+            "The harness has no renderer dispatcher — it must be created through BlazorComponentTestBase."))();
+
     /// <summary>The service instance components resolve via DI.</summary>
     public abstract IIgniteUIBlazor Service { get; }
 
@@ -165,6 +194,13 @@ public abstract class InteropHarness
     /// </summary>
     public abstract void ClearObserved();
 
+    /// <summary>
+    /// The positions of the item insertions transmitted for the instance's bound data, in the order
+    /// the client received them, once the instance has stopped transmitting. How an insertion is
+    /// spelled on the wire is implementation-specific; that every one arrives, in order, is not.
+    /// </summary>
+    public abstract IReadOnlyList<int> DataItemInsertions(string containerId);
+
     public IEnumerable<InteropMethodCall> CallsOf(string methodName, string? containerId = null) =>
         MethodCalls.Where(c => c.MethodName == methodName && (containerId is null || c.ContainerId == containerId));
 

tests/IgniteUI.Blazor.Tests/Interop/RendererMessageInteropHarness.cs

@@ -62,7 +62,8 @@ public override void PrimeReady()
         _js.SetupVoid("igWaitForLoaded", _ => true).SetVoidResult();
     }
 
-    public override void MakeReady() => _service.WebCallback.OnReady();
+    // The JS-to-.NET entries below run on the dispatcher, where Blazor delivers the real ones.
+    public override void MakeReady() => OnDispatcher(_service.WebCallback.OnReady);
 
     public override string ContainerIdOf(IRenderedComponent<IComponent> cut) =>
         cut.Find("[data-ig-id]").GetAttribute("data-ig-id")
@@ -144,11 +145,11 @@ public override IEnumerable<InteropMethodCall> PropertyReads(string containerId,
     public override void RaiseEvent(string containerId, string eventName, string argsJson = "{}", string targetName = "mainControl")
     {
         var payload = $$"""{"sender": {"refType": "name", "id": "{{targetName}}"}, "args": {{argsJson}}}""";
-        _service.WebCallback.OnRaiseEvent(containerId, targetName, eventName, payload);
+        OnDispatcher(() => _service.WebCallback.OnRaiseEvent(containerId, targetName, eventName, payload));
     }
 
     public override void CompleteDeferred(InteropMethodCall call, InteropReturn result) =>
-        _service.WebCallback.OnInvokeReturn(call.ContainerId, call.InvokeId, ToResultPayload(result));
+        OnDispatcher(() => _service.WebCallback.OnInvokeReturn(call.ContainerId, call.InvokeId, ToResultPayload(result)));
 
     public override JsonElement? FindPropertyUpdate(string containerId, string wireName)
     {
@@ -162,8 +163,9 @@ public override void CompleteDeferred(InteropMethodCall call, InteropReturn resu
         // can starve the queue-flush continuations well past their usual few milliseconds.
         for (var attempt = 0; attempt < 80; attempt++)
         {
-            // One snapshot+parse per attempt, scanned newest-first.
-            var messages = Messages().Where(m => m.ContainerId == containerId).Reverse().ToList();
+            // One snapshot+parse per attempt, newest-first, taken once the instance stops
+            // transmitting: mid-flush the newest update recorded is not yet the newest one sent.
+            var messages = SettledMessagesFor(containerId);
 
             string? dataRefId = null;
             foreach (var (_, message, _) in messages)
@@ -202,6 +204,49 @@ public override void CompleteDeferred(InteropMethodCall call, InteropReturn resu
         return null;
     }
 
+    /// <summary>
+    /// This instance's traffic, newest first, taken once it stops transmitting. A flush hands its
+    /// messages to JS one at a time, so a snapshot can land between two and show an update the next
+    /// one supersedes - how a script ref read back as the event registration sharing its ref name.
+    /// Only this instance's traffic can supersede it, so other components never hold it up.
+    /// </summary>
+    private List<(string ContainerId, JsonElement Message, IReadOnlyList<ElementReference> Elements)> SettledMessagesFor(string containerId)
+    {
+        // Bounded, so a component that never stops transmitting cannot hang the test; the caller
+        // has its own budget for concluding absence.
+        for (var attempt = 0; attempt < 40; attempt++)
+        {
+            // Barrier first, and it is what actually settles a flush: it queues behind the flush's
+            // own dispatcher work item, so one that has started sending has finished by the time
+            // this returns - however long it was preempted between two sends. A lull cannot promise
+            // that, since under contention the renderer can be descheduled mid-flush for longer
+            // than any lull worth waiting for. Going first also means the counts below read a
+            // record nothing is appending to.
+            OnDispatcher(() => { });
+            var sends = SendCountFor(containerId);
+            // The lull covers what the barrier cannot see yet: a flush still waiting on the
+            // thread-pool hop that posts it. Counting needs no parsing, so only the settled record is.
+            Thread.Sleep(1);
+            if (SendCountFor(containerId) == sends)
+            {
+                break;
+            }
+        }
+        return Messages().Where(m => m.ContainerId == containerId).Reverse().ToList();
+    }
+
+    /// <summary>On this stack an insertion is a refNotifyInsertItem message carrying its index.</summary>
+    public override IReadOnlyList<int> DataItemInsertions(string containerId) =>
+        [.. SettledMessagesFor(containerId)
+            .AsEnumerable()
+            .Reverse()
+            .Where(m => m.Message.GetProperty("type").GetString() == "refNotifyInsertItem")
+            .Select(m => m.Message.GetProperty("index").GetInt32())];
+
+    private int SendCountFor(string containerId) =>
+        SnapshotInvocations().Count(i =>
+            i.Identifier == SendMessage && i.Arguments.Count > 0 && i.Arguments[0] as string == containerId);
+
     /// <summary>
     /// refChanged values embed their payload as prefixed strings
     /// (<c>localJson:::{...}</c>, <c>json:::{...}</c>); unwrap to the actual JSON value.
@@ -287,8 +332,10 @@ invocation.Arguments[0] is not string containerId ||
     }
 
     /// <summary>
-    /// Components flush queued messages from background continuations, so bUnit's
-    /// append-only invocation record can grow while we read it. Snapshot with retry.
+    /// Components flush queued messages from background continuations, so bUnit's append-only
+    /// invocation record can grow while we read it - and the longer the record, the longer each
+    /// attempt is exposed, so a component mid-flush can beat several in a row. Yielding is enough
+    /// once the flush ends; a real pause is what gets us there.
     /// </summary>
     private IReadOnlyList<JSRuntimeInvocation> SnapshotInvocations()
     {
@@ -298,9 +345,16 @@ private IReadOnlyList<JSRuntimeInvocation> SnapshotInvocations()
             {
                 return [.. _js.Invocations];
             }
-            catch (InvalidOperationException) when (attempt < 10)
+            catch (InvalidOperationException) when (attempt < 200)
             {
-                Thread.Yield();
+                if (attempt < 10)
+                {
+                    Thread.Yield();
+                }
+                else
+                {
+                    Thread.Sleep(1);
+                }
             }
         }
     }