Serialize server-side renders. Fixes #1573 (#1672)

* Add failing test to show the issue

* Make RemoteRenderer serialize render calls on sync context

src/Microsoft.AspNetCore.Blazor.Server/Circuits/DefaultCircuitFactory.cs

@@ -40,8 +40,8 @@ namespace Microsoft.AspNetCore.Blazor.Server.Circuits
             var scope = _scopeFactory.CreateScope();
             var jsRuntime = new RemoteJSRuntime(client);
             var rendererRegistry = new RendererRegistry();
-            var renderer = new RemoteRenderer(scope.ServiceProvider, rendererRegistry, jsRuntime, client);
             var synchronizationContext = new CircuitSynchronizationContext();
+            var renderer = new RemoteRenderer(scope.ServiceProvider, rendererRegistry, jsRuntime, client, synchronizationContext);
 
             var circuitHost = new CircuitHost(
                 scope,

src/Microsoft.AspNetCore.Blazor.Server/Circuits/RemoteRenderer.cs

@@ -24,6 +24,7 @@ namespace Microsoft.AspNetCore.Blazor.Browser.Rendering
         private readonly IClientProxy _client;
         private readonly IJSRuntime _jsRuntime;
         private readonly RendererRegistry _rendererRegistry;
+        private readonly SynchronizationContext _syncContext;
         private readonly ConcurrentDictionary<long, AutoCancelTaskCompletionSource<object>> _pendingRenders
             = new ConcurrentDictionary<long, AutoCancelTaskCompletionSource<object>>();
         private long _nextRenderId = 1;
@@ -40,16 +41,19 @@ namespace Microsoft.AspNetCore.Blazor.Browser.Rendering
         /// <param name="rendererRegistry">The <see cref="RendererRegistry"/>.</param>
         /// <param name="jsRuntime">The <see cref="IJSRuntime"/>.</param>
         /// <param name="client">The <see cref="IClientProxy"/>.</param>
+        /// <param name="syncContext">A <see cref="SynchronizationContext"/> that can be used to serialize renderer operations.</param>
         public RemoteRenderer(
             IServiceProvider serviceProvider,
             RendererRegistry rendererRegistry,
             IJSRuntime jsRuntime,
-            IClientProxy client)
+            IClientProxy client,
+            SynchronizationContext syncContext)
             : base(serviceProvider)
         {
             _rendererRegistry = rendererRegistry;
             _jsRuntime = jsRuntime;
             _client = client;
+            _syncContext = syncContext;
 
             _id = _rendererRegistry.Add(this);
         }
@@ -95,6 +99,19 @@ namespace Microsoft.AspNetCore.Blazor.Browser.Rendering
             _rendererRegistry.TryRemove(_id);
         }
 
+        protected override void AddToRenderQueue(int componentId, RenderFragment renderFragment)
+        {
+            // Render operations are not thread-safe, so they need to be serialized.
+            // This also ensures that when the renderer invokes component lifecycle
+            // methods, it does so on the expected sync context.
+            // We have to use "Post" (for async execution) because if it blocked, it
+            // could deadlock when a child triggers a parent re-render.
+            _syncContext.Post(_ =>
+            {
+                base.AddToRenderQueue(componentId, renderFragment);
+            }, null);
+        }
+
         /// <inheritdoc />
         protected override Task UpdateDisplayAsync(in RenderBatch batch)
         {

src/Microsoft.AspNetCore.Blazor/Rendering/Renderer.cs

@@ -140,7 +140,13 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
             frame = frame.WithAttributeEventHandlerId(id);
         }
 
-        internal void AddToRenderQueue(int componentId, RenderFragment renderFragment)
+        /// <summary>
+        /// Schedules a render for the specified <paramref name="componentId"/>. Its display
+        /// will be populated using the specified <paramref name="renderFragment"/>.
+        /// </summary>
+        /// <param name="componentId">The ID of the component to render.</param>
+        /// <param name="renderFragment">A <see cref="RenderFragment"/> that will supply the updated UI contents.</param>
+        protected internal virtual void AddToRenderQueue(int componentId, RenderFragment renderFragment)
         {
             var componentState = GetOptionalComponentState(componentId);
             if (componentState == null)

test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -6,6 +6,7 @@ using System.Collections.Generic;
 using System.Configuration.Assemblies;
 using System.Linq;
 using System.Numerics;
+using System.Threading.Tasks;
 using BasicTestApp;
 using BasicTestApp.HierarchicalImportsTest.Subdir;
 using Microsoft.AspNetCore.Blazor.E2ETest.Infrastructure;
@@ -534,6 +535,23 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
                 e => Assert.Equal("End", e.Text));
         }
 
+        [Fact]
+        public async Task CanAcceptSimultaneousRenderRequests()
+        {
+            var expectedOutput = string.Join(
+                string.Empty,
+                Enumerable.Range(0, 100).Select(_ => "😊"));
+
+            var appElement = MountTestComponent<ConcurrentRenderParent>();
+
+            // It's supposed to pause the rendering for this long. The WaitAssert below
+            // allows it to take up extra time if needed.
+            await Task.Delay(1000);
+
+            var outputElement = appElement.FindElement(By.Id("concurrent-render-output"));
+            WaitAssert.Equal(expectedOutput, () => outputElement.Text);
+        }
+
         static IAlert SwitchToAlert(IWebDriver driver)
         {
             try

test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs

@@ -1130,7 +1130,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             // Act/Assert 3: After we complete the first UI update in which a given
             // event handler ID is disposed, we can no longer reuse that event handler ID
             render1TCS.SetResult(null);
-            await Task.Delay(100); // From here we can't see when the async disposal is completed. Just give it plenty of time (Task.Yield isn't enough).
+            await Task.Delay(500); // From here we can't see when the async disposal is completed. Just give it plenty of time (Task.Yield isn't enough).
             var ex = Assert.Throws<ArgumentException>(() =>
             {
                 renderer.DispatchEvent(componentId, eventHandlerId, new UIEventArgs());

test/testapps/BasicTestApp/ConcurrentRenderChild.cshtml

@@ -0,0 +1,16 @@
+<span>@(isAfterDelay ? "😊" :"WAITING")</span>
+@functions
+{
+    protected bool isAfterDelay;
+
+    protected override async Task OnInitAsync()
+    {
+        // If there are lots of instances of this component, the following delay
+        // will result in a lot of them triggering a re-render simultaneously
+        // on different threads.
+        // This test is to verify that the renderer correctly accepts all the
+        // simultaneous render requests.
+        await Task.Delay(1000);
+        isAfterDelay = true;
+    }
+}

test/testapps/BasicTestApp/ConcurrentRenderParent.cshtml

@@ -0,0 +1,7 @@
+<p>
+    After a 1 second delay, the output should be 100x😊, with no remaining "WAITING" markers.
+</p>
+
+<div id="concurrent-render-output">
+    @for (var i = 0; i < 100; i++) {<ConcurrentRenderChild />}
+</div>

test/testapps/BasicTestApp/Index.cshtml

@@ -42,6 +42,7 @@
         <option value="BasicTestApp.RazorTemplates">Razor Templates</option>
         <option value="BasicTestApp.MultipleChildContent">Multiple child content</option>
         <option value="BasicTestApp.CascadingValueTest.CascadingValueSupplier">Cascading values</option>
+        <option value="BasicTestApp.ConcurrentRenderParent">Concurrent rendering</option>
     </select>
 
   @if (SelectedComponentType != null)