Skip to content

Commit cbbdeae

Browse files
SteveSandersonMSSteveSandersonMS
authored
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
1 parent 0b8e16f commit cbbdeae

File tree

9 files changed

+263
-19
lines changed

9 files changed

+263
-19
lines changed

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

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public RemoteRenderer(
5252
_rendererRegistry = rendererRegistry;
5353
_jsRuntime = jsRuntime;
5454
_client = client;
55-
_syncContext = syncContext;
55+
_syncContext = syncContext ?? throw new ArgumentNullException(nameof(syncContext));
5656

5757
_id = _rendererRegistry.Add(this);
5858
}
@@ -90,6 +90,36 @@ public void AddComponent(Type componentType, string domElementSelector)
9090
RenderRootComponent(componentId);
9191
}
9292

93+
/// <inheritdoc />
94+
public override Task Invoke(Action workItem)
95+
{
96+
if (SynchronizationContext.Current == _syncContext)
97+
{
98+
// No need to dispatch. Avoid deadlock by invoking directly.
99+
return base.Invoke(workItem);
100+
}
101+
else
102+
{
103+
var syncContext = (CircuitSynchronizationContext)_syncContext;
104+
return syncContext.Invoke(workItem);
105+
}
106+
}
107+
108+
/// <inheritdoc />
109+
public override Task InvokeAsync(Func<Task> workItem)
110+
{
111+
if (SynchronizationContext.Current == _syncContext)
112+
{
113+
// No need to dispatch. Avoid deadlock by invoking directly.
114+
return base.InvokeAsync(workItem);
115+
}
116+
else
117+
{
118+
var syncContext = (CircuitSynchronizationContext)_syncContext;
119+
return syncContext.InvokeAsync(workItem);
120+
}
121+
}
122+
93123
/// <summary>
94124
/// Disposes the instance.
95125
/// </summary>
@@ -101,14 +131,18 @@ public void Dispose()
101131
protected override void AddToRenderQueue(int componentId, RenderFragment renderFragment)
102132
{
103133
// Render operations are not thread-safe, so they need to be serialized.
104-
// This also ensures that when the renderer invokes component lifecycle
105-
// methods, it does so on the expected sync context.
106-
// We have to use "Post" (for async execution) because if it blocked, it
107-
// could deadlock when a child triggers a parent re-render.
108-
_syncContext.Post(_ =>
134+
// Plus, any other logic that mutates state accessed during rendering also
135+
// needs not to run concurrently with rendering so should be dispatched to
136+
// the renderer's sync context.
137+
if (SynchronizationContext.Current != _syncContext)
109138
{
110-
base.AddToRenderQueue(componentId, renderFragment);
111-
}, null);
139+
throw new RemoteRendererException(
140+
"The current thread is not associated with the renderer's synchronization context. " +
141+
"Use Invoke() or InvokeAsync() to switch execution to the renderer's synchronization " +
142+
"context when triggering rendering or modifying any state accessed during rendering.");
143+
}
144+
145+
base.AddToRenderQueue(componentId, renderFragment);
112146
}
113147

114148
/// <inheritdoc />

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,16 @@ protected void StateHasChanged()
105105
if (_hasNeverRendered || ShouldRender())
106106
{
107107
_hasPendingQueuedRender = true;
108-
_renderHandle.Render(_renderFragment);
108+
109+
try
110+
{
111+
_renderHandle.Render(_renderFragment);
112+
}
113+
catch
114+
{
115+
_hasPendingQueuedRender = false;
116+
throw;
117+
}
109118
}
110119
}
111120

@@ -132,6 +141,22 @@ protected virtual void OnAfterRender()
132141
protected virtual Task OnAfterRenderAsync()
133142
=> Task.CompletedTask;
134143

144+
/// <summary>
145+
/// Executes the supplied work item on the associated renderer's
146+
/// synchronization context.
147+
/// </summary>
148+
/// <param name="workItem">The work item to execute.</param>
149+
protected Task Invoke(Action workItem)
150+
=> _renderHandle.Invoke(workItem);
151+
152+
/// <summary>
153+
/// Executes the supplied work item on the associated renderer's
154+
/// synchronization context.
155+
/// </summary>
156+
/// <param name="workItem">The work item to execute.</param>
157+
protected Task InvokeAsync(Func<Task> workItem)
158+
=> _renderHandle.InvokeAsync(workItem);
159+
135160
void IComponent.Init(RenderHandle renderHandle)
136161
{
137162
// This implicitly means a ComponentBase can only be associated with a single

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Microsoft.AspNetCore.Components.Rendering;
55
using Microsoft.AspNetCore.Components.RenderTree;
66
using System;
7+
using System.Threading.Tasks;
78

89
namespace Microsoft.AspNetCore.Components
910
{
@@ -41,5 +42,21 @@ public void Render(RenderFragment renderFragment)
4142

4243
_renderer.AddToRenderQueue(_componentId, renderFragment);
4344
}
45+
46+
/// <summary>
47+
/// Executes the supplied work item on the renderer's
48+
/// synchronization context.
49+
/// </summary>
50+
/// <param name="workItem">The work item to execute.</param>
51+
public Task Invoke(Action workItem)
52+
=> _renderer.Invoke(workItem);
53+
54+
/// <summary>
55+
/// Executes the supplied work item on the renderer's
56+
/// synchronization context.
57+
/// </summary>
58+
/// <param name="workItem">The work item to execute.</param>
59+
public Task InvokeAsync(Func<Task> workItem)
60+
=> _renderer.InvokeAsync(workItem);
4461
}
4562
}

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNetCore.Components;
54
using Microsoft.AspNetCore.Components.RenderTree;
65
using System;
76
using System.Collections.Generic;
8-
using System.Linq;
97
using System.Threading.Tasks;
108

119
namespace Microsoft.AspNetCore.Components.Rendering
@@ -131,6 +129,29 @@ public void DispatchEvent(int componentId, int eventHandlerId, UIEventArgs event
131129
}
132130
}
133131

132+
/// <summary>
133+
/// Executes the supplied work item on the renderer's
134+
/// synchronization context.
135+
/// </summary>
136+
/// <param name="workItem">The work item to execute.</param>
137+
public virtual Task Invoke(Action workItem)
138+
{
139+
// Base renderer has nothing to dispatch to, so execute directly
140+
workItem();
141+
return Task.CompletedTask;
142+
}
143+
144+
/// <summary>
145+
/// Executes the supplied work item on the renderer's
146+
/// synchronization context.
147+
/// </summary>
148+
/// <param name="workItem">The work item to execute.</param>
149+
public virtual Task InvokeAsync(Func<Task> workItem)
150+
{
151+
// Base renderer has nothing to dispatch to, so execute directly
152+
return workItem();
153+
}
154+
134155
internal void InstantiateChildComponentOnFrame(ref RenderTreeFrame frame, int parentComponentId)
135156
{
136157
if (frame.FrameType != RenderTreeFrameType.Component)

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using BasicTestApp;
5+
using Microsoft.AspNetCore.Components.Browser.Rendering;
6+
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
7+
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
8+
using Microsoft.AspNetCore.Components.E2ETest.Tests;
9+
using OpenQA.Selenium;
10+
using System.Threading.Tasks;
11+
using Xunit;
12+
using Xunit.Abstractions;
13+
14+
namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
15+
{
16+
// By inheriting from ComponentRenderingTest, this test class also copies
17+
// all the test cases shared with client-side rendering
18+
19+
public class ServerComponentRenderingTest : ComponentRenderingTest
20+
{
21+
public ServerComponentRenderingTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)
22+
: base(browserFixture, serverFixture.WithServerExecution(), output)
23+
{
24+
}
25+
26+
[Fact]
27+
public void ThrowsIfRenderIsRequestedOutsideSyncContext()
28+
{
29+
var appElement = MountTestComponent<DispatchingComponent>();
30+
var result = appElement.FindElement(By.Id("result"));
31+
32+
appElement.FindElement(By.Id("run-without-dispatch")).Click();
33+
34+
WaitAssert.Contains(
35+
$"{typeof(RemoteRendererException).FullName}: The current thread is not associated with the renderer's synchronization context",
36+
() => result.Text);
37+
}
38+
}
39+
}

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,6 @@
99

1010
namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
1111
{
12-
public class ServerComponentRenderingTest : ComponentRenderingTest
13-
{
14-
public ServerComponentRenderingTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)
15-
: base(browserFixture, serverFixture.WithServerExecution(), output)
16-
{
17-
}
18-
}
19-
2012
public class ServerBindTest : BindTest
2113
{
2214
public ServerBindTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,39 @@ public async Task CanAcceptSimultaneousRenderRequests()
552552
WaitAssert.Equal(expectedOutput, () => outputElement.Text);
553553
}
554554

555+
[Fact]
556+
public void CanDispatchRenderToSyncContext()
557+
{
558+
var appElement = MountTestComponent<DispatchingComponent>();
559+
var result = appElement.FindElement(By.Id("result"));
560+
561+
appElement.FindElement(By.Id("run-with-dispatch")).Click();
562+
563+
WaitAssert.Equal("Success (completed synchronously)", () => result.Text);
564+
}
565+
566+
[Fact]
567+
public void CanDoubleDispatchRenderToSyncContext()
568+
{
569+
var appElement = MountTestComponent<DispatchingComponent>();
570+
var result = appElement.FindElement(By.Id("result"));
571+
572+
appElement.FindElement(By.Id("run-with-double-dispatch")).Click();
573+
574+
WaitAssert.Equal("Success (completed synchronously)", () => result.Text);
575+
}
576+
577+
[Fact]
578+
public void CanDispatchAsyncWorkToSyncContext()
579+
{
580+
var appElement = MountTestComponent<DispatchingComponent>();
581+
var result = appElement.FindElement(By.Id("result"));
582+
583+
appElement.FindElement(By.Id("run-async-with-dispatch")).Click();
584+
585+
WaitAssert.Equal("First Second Third Fourth Fifth", () => result.Text);
586+
}
587+
555588
static IAlert SwitchToAlert(IWebDriver driver)
556589
{
557590
try

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<h1>Dispatching</h1>
2+
3+
<p>
4+
Sometimes, renders need to be triggered in response to non-lifecyle events.
5+
The current thread will not be associated with the renderer's sync context,
6+
so the render request has to be marshalled onto that sync context.
7+
</p>
8+
9+
<p>
10+
Result: <strong id="result">@result</strong>
11+
</p>
12+
13+
<button id="run-without-dispatch" onclick=@RunWithoutDispatch>Run without dispatch</button>
14+
<button id="run-with-dispatch" onclick=@RunWithDispatch>Run with dispatch</button>
15+
<button id="run-with-double-dispatch" onclick=@RunWithDoubleDispatch>Run with double dispatch</button>
16+
<button id="run-async-with-dispatch" onclick=@RunAsyncWorkWithDispatch>Run async work with dispatch</button>
17+
18+
@functions {
19+
string result;
20+
21+
async Task RunWithoutDispatch()
22+
{
23+
await Task.Delay(1).ConfigureAwait(false);
24+
AttemptToRender();
25+
}
26+
27+
async Task RunWithDispatch()
28+
{
29+
await Task.Delay(1).ConfigureAwait(false);
30+
await Invoke(AttemptToRender);
31+
32+
// So we can observe that the dispatched work item completed by now
33+
if (result == "Success")
34+
{
35+
result += " (completed synchronously)";
36+
}
37+
}
38+
39+
async Task RunWithDoubleDispatch()
40+
{
41+
await Task.Delay(1).ConfigureAwait(false);
42+
await InvokeAsync(() => Invoke(AttemptToRender));
43+
44+
// So we can observe that the dispatched work item completed by now
45+
if (result == "Success")
46+
{
47+
result += " (completed synchronously)";
48+
}
49+
}
50+
51+
async Task RunAsyncWorkWithDispatch()
52+
{
53+
await Task.Delay(1).ConfigureAwait(false);
54+
55+
result = "First";
56+
57+
var invokeTask = InvokeAsync(async () =>
58+
{
59+
// When the sync context is idle, queued work items start synchronously
60+
result += " Second";
61+
await Task.Delay(250);
62+
result += " Fourth";
63+
});
64+
65+
result += " Third";
66+
await invokeTask;
67+
result += " Fifth";
68+
}
69+
70+
void AttemptToRender()
71+
{
72+
try
73+
{
74+
result = "Success";
75+
StateHasChanged();
76+
}
77+
catch (Exception ex)
78+
{
79+
result = ex.ToString();
80+
}
81+
}
82+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
<option value="BasicTestApp.MultipleChildContent">Multiple child content</option>
4545
<option value="BasicTestApp.CascadingValueTest.CascadingValueSupplier">Cascading values</option>
4646
<option value="BasicTestApp.ConcurrentRenderParent">Concurrent rendering</option>
47+
<option value="BasicTestApp.DispatchingComponent">Dispatching to sync context</option>
4748
</select>
4849

4950
@if (SelectedComponentType != null)

0 commit comments

Comments
 (0)