Fix the intermittent Interop unit test failures

src/componentsBase/BaseRendererControl.cs

@@ -155,6 +155,14 @@ internal DataSourceManager DataSourceManager
         }
         private LinkedList<RendererMessage> _messageQueue = new LinkedList<RendererMessage>();
 
+        /// <summary>
+        /// Guards <see cref="_messageQueue"/> and the <c>_updateQueued</c> flag that decides when it
+        /// is flushed. Two paths flush it - <c>SendMessageImmediate</c> on the caller's thread,
+        /// <c>QueueUpdate</c> on the renderer's - and a component driven from off the Blazor
+        /// dispatcher (a timer, a bound collection filled in the background) hits both at once.
+        /// </summary>
+        private readonly object _messageQueueLock = new object();
+
         private string _containerId = Guid.NewGuid().ToString();
 
         internal string ContainerId
@@ -852,7 +860,7 @@ private void SendDescriptionMessage()
             }
             RendererMessage m = new RendererMessage();
             m.Type = ("description");
-            _messageQueue.AddLast(m);
+            Enqueue(m);
             QueueUpdate();
         }
 
@@ -954,6 +962,13 @@ public string Serialize()
         private Dictionary<long, Object> _methodReturns = new Dictionary<long, Object>();
         private Object _semLock = new Object();
 
+        /// <summary>
+        /// Shared by every instance because the WebView callback paths return only the invoke ID,
+        /// without a container ID to identify the originating component.
+        /// </summary>
+        /// <remarks>
+        /// Only use <see cref="Interlocked.Increment" /> as this is incremented from any thread.
+        /// </remarks>
         static long _invokeId = 0;
         protected async Task<object> InvokeMethod(string methodName, object[] arguments, string[] types, ElementReference[] nativeElements = null)
         {
@@ -995,7 +1010,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]);
@@ -1045,7 +1060,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]);
@@ -1574,7 +1589,7 @@ private void SendMessage(RendererMessage m)
                 return;
             }
             //Console.WriteLine("sending message");
-            _messageQueue.AddLast(m);
+            Enqueue(m);
             QueueUpdate();
         }
 
@@ -1585,8 +1600,14 @@ private async Task<object> SendMessageImmediate(RendererMessage m)
                 return null;
             }
 
-            Update();
-            return await SendJsonImmediate(m);
+            // The send must start under this lock.
+            Task<object> sent;
+            lock (_messageQueueLock)
+            {
+                Update();
+                sent = SendJsonImmediate(m);
+            }
+            return await sent;
         }
 
         private object SendMessageSyncImmediate(RendererMessage m)
@@ -1595,51 +1616,79 @@ private object SendMessageSyncImmediate(RendererMessage m)
             {
                 return null;
             }
-            UpdateSync();
-            return SendJsonImmediateSync(m);
+            lock (_messageQueueLock)
+            {
+                UpdateSync();
+                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, so two flushes cannot interleave their
+            // sends. A thread already holding it can take it again, so nesting 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);
+                }
             }
         }
 
@@ -3177,7 +3226,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

@@ -19,12 +19,12 @@ public abstract class BlazorComponentTestBase : BunitContext
     /// <summary>The interop harness for components on the default (current) interop stack.</summary>
     protected InteropHarness Interop { get; }
 
-    private Dictionary<Func<BunitJSInterop, InteropHarness>, InteropHarness>? _overrideHarnesses;
+    private Dictionary<InteropHarnessRegistry.HarnessFactory, InteropHarness>? _overrideHarnesses;
 
     protected BlazorComponentTestBase()
     {
         JSInterop.Mode = JSRuntimeMode.Loose;
-        Interop = InteropHarnessRegistry.CreateDefault(JSInterop);
+        Interop = InteropHarnessRegistry.CreateDefault(JSInterop, () => Renderer.Dispatcher);
         Interop.ConfigureServices(Services);
         IgniteUIBlazor = Interop.Service;
     }
@@ -50,7 +50,7 @@ protected InteropHarness InteropFor(Type componentType)
         _overrideHarnesses ??= new();
         if (!_overrideHarnesses.TryGetValue(factory, out var harness))
         {
-            harness = factory(JSInterop);
+            harness = factory(JSInterop, () => 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);
         }
@@ -365,7 +367,7 @@ void Sink(object? value)
                 var registration = harness.FindPropertyUpdate(containerId, WireMemberName(bind.DrivingEvent))
                     ?? throw new XunitException(
                         $"binding transmitted no \"{bind.DrivingEvent}\" event registration — without it the " +
-                        "client never reports changes, so the binding can never fire");
+                        $"client never reports changes, so the binding can never fire ({harness.DescribeTraffic(containerId)})");
                 Assert.Equal(bind.DrivingEvent, registration.GetString());
 
                 harness.RaiseEvent(containerId, bind.DrivingEvent, bind.ArgsJson.Get(harness, cut));
@@ -485,7 +487,8 @@ protected void VerifyEventContract()
                 Assert.Equal(bound, evt.Get(cut.Instance));
                 var wireName = WireMemberName(evt.EventName);
                 var registration = harness.FindPropertyUpdate(containerId, wireName)
-                    ?? throw new XunitException("no event-handler registration transmission was observed");
+                    ?? throw new XunitException(
+                        "no event-handler registration transmission was observed — " + harness.DescribeTraffic(containerId));
                 Assert.Equal(evt.EventName, registration.GetString());
 
                 var argsJson = evt.ArgsJson.Get(harness, cut);
@@ -555,7 +558,7 @@ void Rebind()
                     {
                         throw new XunitException(
                             $"re-binding \"{evt.EventName}\" transmitted {rebound?.ToString() ?? "no registration"} — " +
-                            "the client would never resubscribe");
+                            $"the client would never resubscribe ({harness.DescribeTraffic(containerId)})");
                     }
                 }
             }

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

@@ -98,6 +98,34 @@ private InteropReturn(InteropReturnKind kind, object? value = null, string? type
 /// </summary>
 public abstract class InteropHarness
 {
+    private readonly Func<Dispatcher> _dispatcher;
+
+    /// <param name="dispatcher">
+    /// Resolves the renderer's dispatcher. Taken as a required argument rather than set afterwards
+    /// so a harness cannot exist without one, and resolved on use rather than up front because the
+    /// harness is built while the test's services are still being configured - asking for the
+    /// renderer that early settles the container.
+    /// </param>
+    protected InteropHarness(Func<Dispatcher> dispatcher) =>
+        _dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(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();
+
     /// <summary>The service instance components resolve via DI.</summary>
     public abstract IIgniteUIBlazor Service { get; }
 
@@ -165,6 +193,24 @@ 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. Transmission is asynchronous and nothing observable says it has
+    /// finished, so <paramref name="expected"/> - what the caller is waiting for - is what the wait
+    /// is pinned to. Everything transmitted comes back, so both a shortfall and an overshoot are
+    /// returned to be asserted on rather than hidden. How an insertion is spelled on the wire is
+    /// implementation-specific; that every one arrives exactly once, in order, is not.
+    /// </summary>
+    public abstract IReadOnlyList<int> DataItemInsertions(string containerId, int expected);
+
+    /// <summary>
+    /// A short account of what the instance has transmitted, to report alongside an expected
+    /// transmission that never showed up. Absence on its own is ambiguous: a message that was never
+    /// queued reads exactly like one that was queued and never flushed, and only the second is a
+    /// timing problem. "Sent nothing at all" separates them.
+    /// </summary>
+    public abstract string DescribeTraffic(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/InteropHarnessRegistry.cs

@@ -12,15 +12,18 @@ namespace IgniteUI.Blazor.Tests.Interop;
 /// </summary>
 public static class InteropHarnessRegistry
 {
-    private static readonly Dictionary<Type, Func<BunitJSInterop, InteropHarness>> Overrides = [];
+    private static readonly Dictionary<Type, HarnessFactory> Overrides = [];
 
-    public static void Register<TComponent>(Func<BunitJSInterop, InteropHarness> factory)
+    /// <summary>Builds a harness from the test's JS interop and its renderer's dispatcher.</summary>
+    public delegate InteropHarness HarnessFactory(BunitJSInterop jsInterop, Func<Dispatcher> dispatcher);
+
+    public static void Register<TComponent>(HarnessFactory factory)
         where TComponent : IComponent
         => Overrides[typeof(TComponent)] = factory;
 
-    public static InteropHarness CreateDefault(BunitJSInterop jsInterop) =>
-        new RendererMessageInteropHarness(jsInterop);
+    public static InteropHarness CreateDefault(BunitJSInterop jsInterop, Func<Dispatcher> dispatcher) =>
+        new RendererMessageInteropHarness(jsInterop, dispatcher);
 
-    internal static Func<BunitJSInterop, InteropHarness>? OverrideFor(Type componentType) =>
+    internal static HarnessFactory? OverrideFor(Type componentType) =>
         Overrides.TryGetValue(componentType, out var factory) ? factory : null;
 }