Enforce correct sync context on render, and allow explicit dispatch to sync context. Fixes #5639 (#6604)

* Only use async marshalling to renderer sync context when necessary

Note that the lifecycle methods already take care of capturing the correct sync context, so continuations will already be serialized.
Avoiding an extra layer of asynchrony keeps the semantics of rendering closer to the WebAssembly cases, and will fix a range of intermittent errors in the wild.

* Add E2E test of triggering rendering from outside the sync context

* Actually throw if attempting to render from incorrect sync context

* Add "Dispatch" API

* Handle dispatch within dispatch. Also test Dispatch on WebAssembly.

* Avoid heap allocation

* Simplify E2E test

* Replace Dispatch() with Invoke() and InvokeAsync()

* Add E2E test to validate async execution order

* Clean up

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

@@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.Components.Browser.Rendering
             _rendererRegistry = rendererRegistry;
             _jsRuntime = jsRuntime;
             _client = client;
-            _syncContext = syncContext;
+            _syncContext = syncContext ?? throw new ArgumentNullException(nameof(syncContext));
 
             _id = _rendererRegistry.Add(this);
         }
@@ -90,6 +90,36 @@ namespace Microsoft.AspNetCore.Components.Browser.Rendering
             RenderRootComponent(componentId);
         }
 
+        /// <inheritdoc />
+        public override Task Invoke(Action workItem)
+        {
+            if (SynchronizationContext.Current == _syncContext)
+            {
+                // No need to dispatch. Avoid deadlock by invoking directly.
+                return base.Invoke(workItem);
+            }
+            else
+            {
+                var syncContext = (CircuitSynchronizationContext)_syncContext;
+                return syncContext.Invoke(workItem);
+            }
+        }
+
+        /// <inheritdoc />
+        public override Task InvokeAsync(Func<Task> workItem)
+        {
+            if (SynchronizationContext.Current == _syncContext)
+            {
+                // No need to dispatch. Avoid deadlock by invoking directly.
+                return base.InvokeAsync(workItem);
+            }
+            else
+            {
+                var syncContext = (CircuitSynchronizationContext)_syncContext;
+                return syncContext.InvokeAsync(workItem);
+            }
+        }
+
         /// <summary>
         /// Disposes the instance.
         /// </summary>
@@ -101,14 +131,18 @@ namespace Microsoft.AspNetCore.Components.Browser.Rendering
         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(_ =>
+            // Plus, any other logic that mutates state accessed during rendering also
+            // needs not to run concurrently with rendering so should be dispatched to
+            // the renderer's sync context.
+            if (SynchronizationContext.Current != _syncContext)
             {
-                base.AddToRenderQueue(componentId, renderFragment);
-            }, null);
+                throw new RemoteRendererException(
+                    "The current thread is not associated with the renderer's synchronization context. " +
+                    "Use Invoke() or InvokeAsync() to switch execution to the renderer's synchronization " +
+                    "context when triggering rendering or modifying any state accessed during rendering.");
+            }
+
+            base.AddToRenderQueue(componentId, renderFragment);
         }
 
         /// <inheritdoc />

src/Components/src/Microsoft.AspNetCore.Components/ComponentBase.cs

@@ -105,7 +105,16 @@ namespace Microsoft.AspNetCore.Components
             if (_hasNeverRendered || ShouldRender())
             {
                 _hasPendingQueuedRender = true;
-                _renderHandle.Render(_renderFragment);
+
+                try
+                {
+                    _renderHandle.Render(_renderFragment);
+                }
+                catch
+                {
+                    _hasPendingQueuedRender = false;
+                    throw;
+                }
             }
         }
 
@@ -132,6 +141,22 @@ namespace Microsoft.AspNetCore.Components
         protected virtual Task OnAfterRenderAsync()
             => Task.CompletedTask;
 
+        /// <summary>
+        /// Executes the supplied work item on the associated renderer's
+        /// synchronization context.
+        /// </summary>
+        /// <param name="workItem">The work item to execute.</param>
+        protected Task Invoke(Action workItem)
+            => _renderHandle.Invoke(workItem);
+
+        /// <summary>
+        /// Executes the supplied work item on the associated renderer's
+        /// synchronization context.
+        /// </summary>
+        /// <param name="workItem">The work item to execute.</param>
+        protected Task InvokeAsync(Func<Task> workItem)
+            => _renderHandle.InvokeAsync(workItem);
+
         void IComponent.Init(RenderHandle renderHandle)
         {
             // This implicitly means a ComponentBase can only be associated with a single

src/Components/src/Microsoft.AspNetCore.Components/RenderHandle.cs

@@ -4,6 +4,7 @@
 using Microsoft.AspNetCore.Components.Rendering;
 using Microsoft.AspNetCore.Components.RenderTree;
 using System;
+using System.Threading.Tasks;
 
 namespace Microsoft.AspNetCore.Components
 {
@@ -41,5 +42,21 @@ namespace Microsoft.AspNetCore.Components
 
             _renderer.AddToRenderQueue(_componentId, renderFragment);
         }
+
+        /// <summary>
+        /// Executes the supplied work item on the renderer's
+        /// synchronization context.
+        /// </summary>
+        /// <param name="workItem">The work item to execute.</param>
+        public Task Invoke(Action workItem)
+            => _renderer.Invoke(workItem);
+
+        /// <summary>
+        /// Executes the supplied work item on the renderer's
+        /// synchronization context.
+        /// </summary>
+        /// <param name="workItem">The work item to execute.</param>
+        public Task InvokeAsync(Func<Task> workItem)
+            => _renderer.InvokeAsync(workItem);
     }
 }

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

@@ -1,11 +1,9 @@
 // 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 Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.RenderTree;
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 
 namespace Microsoft.AspNetCore.Components.Rendering
@@ -131,6 +129,29 @@ namespace Microsoft.AspNetCore.Components.Rendering
             }
         }
 
+        /// <summary>
+        /// Executes the supplied work item on the renderer's
+        /// synchronization context.
+        /// </summary>
+        /// <param name="workItem">The work item to execute.</param>
+        public virtual Task Invoke(Action workItem)
+        {
+            // Base renderer has nothing to dispatch to, so execute directly
+            workItem();
+            return Task.CompletedTask;
+        }
+
+        /// <summary>
+        /// Executes the supplied work item on the renderer's
+        /// synchronization context.
+        /// </summary>
+        /// <param name="workItem">The work item to execute.</param>
+        public virtual Task InvokeAsync(Func<Task> workItem)
+        {
+            // Base renderer has nothing to dispatch to, so execute directly
+            return workItem();
+        }
+
         internal void InstantiateChildComponentOnFrame(ref RenderTreeFrame frame, int parentComponentId)
         {
             if (frame.FrameType != RenderTreeFrameType.Component)

src/Components/test/Microsoft.AspNetCore.Components.E2ETest/ServerExecutionTests/ServerComponentRenderingTest.cs

@@ -0,0 +1,39 @@
+// 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 BasicTestApp;
+using Microsoft.AspNetCore.Components.Browser.Rendering;
+using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
+using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
+using Microsoft.AspNetCore.Components.E2ETest.Tests;
+using OpenQA.Selenium;
+using System.Threading.Tasks;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
+{
+    // By inheriting from ComponentRenderingTest, this test class also copies
+    // all the test cases shared with client-side rendering
+
+    public class ServerComponentRenderingTest : ComponentRenderingTest
+    {
+        public ServerComponentRenderingTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)
+            : base(browserFixture, serverFixture.WithServerExecution(), output)
+        {
+        }
+
+        [Fact]
+        public void ThrowsIfRenderIsRequestedOutsideSyncContext()
+        {
+            var appElement = MountTestComponent<DispatchingComponent>();
+            var result = appElement.FindElement(By.Id("result"));
+
+            appElement.FindElement(By.Id("run-without-dispatch")).Click();
+
+            WaitAssert.Contains(
+                $"{typeof(RemoteRendererException).FullName}: The current thread is not associated with the renderer's synchronization context",
+                () => result.Text);
+        }
+    }
+}

src/Components/test/Microsoft.AspNetCore.Components.E2ETest/ServerExecutionTests/TestSubclasses.cs

@@ -9,14 +9,6 @@ using Xunit.Abstractions;
 
 namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
 {
-    public class ServerComponentRenderingTest : ComponentRenderingTest
-    {
-        public ServerComponentRenderingTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)
-            : base(browserFixture, serverFixture.WithServerExecution(), output)
-        {
-        }
-    }
-
     public class ServerBindTest : BindTest
     {
         public ServerBindTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)

src/Components/test/Microsoft.AspNetCore.Components.E2ETest/Tests/ComponentRenderingTest.cs

@@ -552,6 +552,39 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
             WaitAssert.Equal(expectedOutput, () => outputElement.Text);
         }
 
+        [Fact]
+        public void CanDispatchRenderToSyncContext()
+        {
+            var appElement = MountTestComponent<DispatchingComponent>();
+            var result = appElement.FindElement(By.Id("result"));
+
+            appElement.FindElement(By.Id("run-with-dispatch")).Click();
+
+            WaitAssert.Equal("Success (completed synchronously)", () => result.Text);
+        }
+
+        [Fact]
+        public void CanDoubleDispatchRenderToSyncContext()
+        {
+            var appElement = MountTestComponent<DispatchingComponent>();
+            var result = appElement.FindElement(By.Id("result"));
+
+            appElement.FindElement(By.Id("run-with-double-dispatch")).Click();
+
+            WaitAssert.Equal("Success (completed synchronously)", () => result.Text);
+        }
+
+        [Fact]
+        public void CanDispatchAsyncWorkToSyncContext()
+        {
+            var appElement = MountTestComponent<DispatchingComponent>();
+            var result = appElement.FindElement(By.Id("result"));
+
+            appElement.FindElement(By.Id("run-async-with-dispatch")).Click();
+
+            WaitAssert.Equal("First Second Third Fourth Fifth", () => result.Text);
+        }
+
         static IAlert SwitchToAlert(IWebDriver driver)
         {
             try

src/Components/test/testapps/BasicTestApp/DispatchingComponent.cshtml

@@ -0,0 +1,82 @@
+<h1>Dispatching</h1>
+
+<p>
+    Sometimes, renders need to be triggered in response to non-lifecyle events.
+    The current thread will not be associated with the renderer's sync context,
+    so the render request has to be marshalled onto that sync context.
+</p>
+
+<p>
+    Result: <strong id="result">@result</strong>
+</p>
+
+<button id="run-without-dispatch" onclick=@RunWithoutDispatch>Run without dispatch</button>
+<button id="run-with-dispatch" onclick=@RunWithDispatch>Run with dispatch</button>
+<button id="run-with-double-dispatch" onclick=@RunWithDoubleDispatch>Run with double dispatch</button>
+<button id="run-async-with-dispatch" onclick=@RunAsyncWorkWithDispatch>Run async work with dispatch</button>
+
+@functions {
+    string result;
+
+    async Task RunWithoutDispatch()
+    {
+        await Task.Delay(1).ConfigureAwait(false);
+        AttemptToRender();
+    }
+
+    async Task RunWithDispatch()
+    {
+        await Task.Delay(1).ConfigureAwait(false);
+        await Invoke(AttemptToRender);
+
+        // So we can observe that the dispatched work item completed by now
+        if (result == "Success")
+        {
+            result += " (completed synchronously)";
+        }
+    }
+
+    async Task RunWithDoubleDispatch()
+    {
+        await Task.Delay(1).ConfigureAwait(false);
+        await InvokeAsync(() => Invoke(AttemptToRender));
+
+        // So we can observe that the dispatched work item completed by now
+        if (result == "Success")
+        {
+            result += " (completed synchronously)";
+        }
+    }
+
+    async Task RunAsyncWorkWithDispatch()
+    {
+        await Task.Delay(1).ConfigureAwait(false);
+
+        result = "First";
+
+        var invokeTask = InvokeAsync(async () =>
+        {
+            // When the sync context is idle, queued work items start synchronously
+            result += " Second";
+            await Task.Delay(250);
+            result += " Fourth";
+        });
+
+        result += " Third";
+        await invokeTask;
+        result += " Fifth";
+    }
+
+    void AttemptToRender()
+    {
+        try
+        {
+            result = "Success";
+            StateHasChanged();
+        }
+        catch (Exception ex)
+        {
+            result = ex.ToString();
+        }
+    }
+}

src/Components/test/testapps/BasicTestApp/Index.cshtml

@@ -44,6 +44,7 @@
         <option value="BasicTestApp.MultipleChildContent">Multiple child content</option>
         <option value="BasicTestApp.CascadingValueTest.CascadingValueSupplier">Cascading values</option>
         <option value="BasicTestApp.ConcurrentRenderParent">Concurrent rendering</option>
+        <option value="BasicTestApp.DispatchingComponent">Dispatching to sync context</option>
     </select>
 
   @if (SelectedComponentType != null)