CR: Redesign

Author: SteveSandersonMS

src/Components/Server/src/Circuits/CircuitHost.cs

@@ -104,35 +104,35 @@ public Task<IEnumerable<string>> PrerenderComponentAsync(Type componentType, Par
             });
         }
 
-        public void Initialize(CancellationToken cancellationToken)
+        public async Task InitializeAsync(CancellationToken cancellationToken)
         {
-            // This Initialize method is fire-and-forget as far as the caller is concerned, because
-            // if it was to await completion, it would be blocking the SignalR message loop. This could
-            // lead to deadlock, e.g., if the init process itself waited for an incoming SignalR message
-            // such as the result of a JSInterop call.
-            Renderer.InvokeAsync(async () =>
+            await Renderer.InvokeAsync(async () =>
             {
                 try
                 {
                     SetCurrentCircuitHost(this);
+                    _initialized = true; // We're ready to accept incoming JSInterop calls from here on
 
+                    await OnCircuitOpenedAsync(cancellationToken);
+                    await OnConnectionUpAsync(cancellationToken);
+
+                    // We add the root components *after* the circuit is flagged as open.
+                    // That's because AddComponentAsync waits for quiescence, which can take
+                    // arbitrarily long. In the meantime we might need to be receiving and
+                    // processing incoming JSInterop calls or similar.
                     for (var i = 0; i < Descriptors.Count; i++)
                     {
                         var (componentType, domElementSelector) = Descriptors[i];
                         await Renderer.AddComponentAsync(componentType, domElementSelector);
                     }
-
-                    await OnCircuitOpenedAsync(cancellationToken);
-
-                    await OnConnectionUpAsync(cancellationToken);
                 }
                 catch (Exception ex)
                 {
+                    // We have to handle all our own errors here, because the upstream caller
+                    // has to fire-and-forget this
                     Renderer_UnhandledException(this, ex);
                 }
             });
-
-            _initialized = true;
         }
 
         public async void BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson)

src/Components/Server/src/ComponentHub.cs

@@ -76,7 +76,11 @@ public string StartCircuit(string uriAbsolute, string baseUriAbsolute)
 
             circuitHost.UnhandledException += CircuitHost_UnhandledException;
 
-            circuitHost.Initialize(Context.ConnectionAborted);
+            // Fire-and-forget the initialization process, because we can't block the
+            // SignalR message loop (we'd get a deadlock if any of the initialization
+            // logic relied on receiving a subsequent message from SignalR), and it will
+            // take care of its own errors anyway.
+            _ = circuitHost.InitializeAsync(Context.ConnectionAborted);
 
             _circuitRegistry.Register(circuitHost);
 

src/Components/Server/test/Circuits/CircuitHostTest.cs

@@ -40,7 +40,7 @@ public async Task DisposeAsync_DisposesResources()
         }
 
         [Fact]
-        public void Initialize_InvokesHandlers()
+        public async Task Initialize_InvokesHandlers()
         {
             // Arrange
             var cancellationToken = new CancellationToken();
@@ -75,15 +75,15 @@ public void Initialize_InvokesHandlers()
             var circuitHost = TestCircuitHost.Create(handlers: new[] { handler1.Object, handler2.Object });
 
             // Act
-            circuitHost.Initialize(cancellationToken);
+            await circuitHost.InitializeAsync(cancellationToken);
 
             // Assert
             handler1.VerifyAll();
             handler2.VerifyAll();
         }
 
         [Fact]
-        public void Initialize_ReportsAsyncExceptions()
+        public async Task Initialize_ReportsOwnAsyncExceptions()
         {
             // Arrange
             var handler = new Mock<CircuitHandler>(MockBehavior.Strict);
@@ -103,15 +103,21 @@ public void Initialize_ReportsAsyncExceptions()
             };
 
             // Act
-            circuitHost.Initialize(new CancellationToken());
-            handler.VerifyAll();
+            var initializeAsyncTask = circuitHost.InitializeAsync(new CancellationToken());
 
-            // Assert: there was no synchronous exception
+            // Assert: No synchronous exceptions
+            handler.VerifyAll();
             Assert.Empty(reportedErrors);
 
-            // Act/Assert: if the handler throws later, that gets reported
+            // Act: Trigger async exception
             var ex = new InvalidTimeZoneException();
             tcs.SetException(ex);
+
+            // Assert: The top-level task still succeeds, because the intended usage
+            // pattern is fire-and-forget.
+            await initializeAsyncTask;
+
+            // Assert: The async exception was reported via the side-channel
             Assert.Same(ex, reportedErrors.Single().ExceptionObject);
             Assert.False(reportedErrors.Single().IsTerminating);
         }