Harden Blazor WebView disposal races for JSRuntime and IPC#67026
Harden Blazor WebView disposal races for JSRuntime and IPC#67026lewing wants to merge 5 commits into
Conversation
Components performing JavaScript interop inside IAsyncDisposable handlers were producing unhandled JSException on Blazor WebView page reload / WebView shutdown, and outbound IPC traffic from background tasks could reach a CoreWebView2 whose underlying control was already disposed. RemoteJSRuntime (Blazor Server) has handled the equivalent disconnect cleanly since #32901; WebViewJSRuntime had no equivalent guard. This change extends the same Mark-As-Disconnected pattern to Blazor WebView across two lifecycle scopes: * Per-page (PageContext): WebViewJSRuntime.MarkAsDisconnected() is called at the start of PageContext.DisposeAsync(), before Renderer.DisposeAsync(). - BeginInvokeJS throws JSDisconnectedException (already swallowed by JSObjectReference.DisposeAsync per #49418, so IAsyncDisposable components no longer surface unhandled exceptions on page reload). - EndInvokeDotNet and SendByteArray silently no-op so stale replies to a gone JS context don't queue against the new page's renderer. * Per-WebView (WebViewManager): IpcSender.Dispose() is called at the start of WebViewManager.DisposeAsyncCore(), before the current PageContext is disposed. - DispatchMessageWithErrorHandling early-returns so in-flight render batches, navigation events, AttachToDocument, SendByteArray, and location-changing acks don't reach a torn-down platform WebView. - NotifyUnhandledException early-returns so a background-task exception that fires after window close can't crash the host via ExceptionDispatchInfo.Capture(...).Throw(). * Per-message (IpcReceiver): incoming messages targeting a page whose JSRuntime.IsDisposed is true are dropped, so JS-side handlers that fire during teardown don't route stale object IDs into the new page's renderer or invoke handlers on a disposed scope. Fixes #66255. Addresses the IAsyncDisposable + JS interop variant and the IpcSender outbound NullReferenceException variant of dotnet/maui#34855. Does not address that issue's WebView2CompositionControl disposal race (owned by the WebView2 product team) or the renderer component-ID variant (tracked at #66322). Supersedes #66259 and #66337 (both Copilot-agent-authored partial fixes; neither implemented the IpcSender guard, neither tested the EndInvokeDotNet / SendByteArray / IpcReceiver guards, and #66259 added a test-only internal accessor on WebViewManager that this PR does not need). Tests (WebViewManagerTests): * JSInteropDuringComponentDispose_OnPageReload_SeesJSDisconnectedException - the main user scenario from #66255 / mattleibow/MauiBlazorReloadIssue * JSInteropAfterWebViewManagerDispose_SeesJSDisconnectedException - shutdown path * PageContextDispose_MarksJSRuntimeAsDisconnected_BeforeRendererDispose - ordering invariant * EndInvokeDotNet_AfterRuntimeDisposed_DropsOutboundMessage - drives via DotNetDispatcher.BeginInvokeDotNet + [JSInvokable] * SendByteArray_AfterRuntimeDisposed_DropsOutboundMessage - protected override reached via reflection * IpcReceiver_AfterPageContextDisposed_DropsIncomingMessages - stale incoming IPC drop * IpcSender_AfterDispose_DropsOutboundDispatches - direct unit test against the sender across every outbound entry point * IpcSender_AfterDispose_DropsNotifyUnhandledException - separate dispatch path that also re-throws via ExceptionDispatchInfo All changes are internal — no public API surface is modified. The Photino sample consumer (and the WebView library itself) build clean with no warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extends Blazor Server’s “disconnected JS runtime” pattern to Blazor WebView so that JS interop during teardown (page reload/WebView shutdown) fails predictably with JSDisconnectedException, and stale inbound/outbound IPC is dropped instead of reaching disposed WebView controls.
Changes:
- Mark
WebViewJSRuntimeas disconnected at the start ofPageContext.DisposeAsync()soBeginInvokeJSthrowsJSDisconnectedExceptionandEndInvokeDotNet/SendByteArrayno-op after disposal. - Dispose
IpcSenderat the start ofWebViewManager.DisposeAsyncCore()and drop messages/exception notifications after disposal. - Drop incoming IPC messages in
IpcReceiverwhen the target page’s JS runtime is disposed; add unit tests covering the new teardown behaviors.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Components/WebView/WebView/test/WebViewManagerTests.cs | Adds tests for JS interop/IPC behavior during page reload and WebView disposal. |
| src/Components/WebView/WebView/src/WebViewManager.cs | Disposes the IPC sender before disposing the current page context to stop outbound traffic during shutdown. |
| src/Components/WebView/WebView/src/Services/WebViewJSRuntime.cs | Introduces a disconnected/disposed flag that throws JSDisconnectedException and drops late replies/transfers. |
| src/Components/WebView/WebView/src/PageContext.cs | Marks the JS runtime disconnected before renderer disposal to protect IAsyncDisposable component teardown. |
| src/Components/WebView/WebView/src/IpcSender.cs | Adds a disposed flag and guards to drop outbound dispatches and unhandled-exception notifications after disposal. |
| src/Components/WebView/WebView/src/IpcReceiver.cs | Drops incoming messages for disposed page contexts to avoid routing stale calls into torn-down scopes. |
Reviewers flagged several disposal-race holes that this PR's guards
fundamentally cannot fix without companion changes:
* WebViewManager.MessageReceived passed _currentPageContext into the
dispatcher callback by closure, so a message queued before a page
reload would run against the NEW page after AttachToPageAsync swapped
it in. The new IpcReceiver.IsDisposed check then evaluated the new
(not-disposed) runtime and let stale messages through. Capture
_currentPageContext at receipt time and pass the captured reference,
so stale messages target the original (now-disposed) runtime where
the guard fires.
* MessageReceived and AttachToPageAsync had no _disposed check, so a
late AttachPage IPC arriving after WebViewManager.DisposeAsync could
re-create a PageContext / scope / renderer / JS runtime against the
already-disposed IpcSender, effectively resurrecting the manager.
Guard both, and re-check _disposed in AttachToPageAsync after
awaiting the previous PageContext's disposal (the await is itself a
race window).
* TryDispatchAsync did not check _disposed and the WebViewManager
never nulls out _currentPageContext on disposal, so workItem could
run against a disposed scope. Add a _disposed check at the entry and
another inside the dispatcher delegate.
* IpcSender.DispatchMessageWithErrorHandling and NotifyUnhandledException
checked _disposed BEFORE _dispatcher.InvokeAsync — a TOCTOU race lets
a Dispose() that fires between the check and the delegate execution
enqueue work that still runs after disposal. Re-check _disposed
inside every queued dispatcher delegate. Also mark the two _disposed
fields volatile so a cross-thread Dispose write is visible to the
dispatcher-thread read without relying on incidental memory barriers.
* IpcReceiver's broad IsDisposed guard dropped EndInvokeJS, leaving
any pending InvokeAsync<T> task that was awaiting a JS reply hanging
forever. Refine the guard to only drop messages that invoke user
code on the disposed scope (BeginInvokeDotNet, ReceiveByteArrayFromJS,
OnLocationChanging). EndInvokeJS / OnRenderCompleted / OnLocationChanged
pass through: they only complete pending task-completion sources or
notify benign events that disposed framework objects already tolerate.
Tests added:
* AttachToPageAsync_AfterWebViewManagerDispose_DoesNotResurrectManager
- asserts no new AttachWebRendererInterop / AttachToDocument / render
batch traffic and no new scoped-service instance after a late
AttachPage that arrives post-disposal.
* TryDispatchAsync_AfterWebViewManagerDispose_ReturnsFalse
- asserts the workItem does not run against a disposed scope.
Known remaining limitation (out of scope for this PR): in-flight
InvokeAsync<T> calls that JS will never reply to (because the JS
context is gone) still hang. Fully fixing this requires the runtime
to fail-fast all pending tracker entries on MarkAsDisconnected, which
needs JSRuntime base-class cooperation. Tracking separately.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Round 2: addressed reviewer + rubber-duck feedback@copilot-pull-request-reviewer flagged two real TOCTOU races in Reviewer feedback addressed
Additional rubber-duck findings addressed
Tests
Known limitation (out of scope for this PR)In-flight |
Second rubber-duck pass on top of 637a7c5 found 4 more blocking issues that round 2 introduced or failed to fully address: * WebViewManager._disposed was still a plain bool. The round-2 guards in MessageReceived, AttachToPageAsync, and TryDispatchAsync depend on cross-thread visibility (DisposeAsyncCore runs from the host UI thread; reads happen from the WebView IPC thread / arbitrary user threads). Mark it volatile, matching IpcSender._disposed and WebViewJSRuntime._isDisposed from round 2. * AttachToPageAsync vs DisposeAsyncCore could both observe the same _currentPageContext and double-dispose it. PageContext.DisposeAsync is not idempotent — it disposes the scoped service provider exactly once. Atomically detach the context (capture into a local + null the field) before awaiting disposal, in BOTH paths. After this change a concurrent attach and dispose cannot race over the same context reference even if _disposed visibility lags. * IpcReceiver's refined guard was too optimistic for OnLocationChanged and OnRenderCompleted: - OnLocationChanged calls NavigationManager.NotifyLocationChanged which invokes arbitrary user subscribers — they may belong to disposed components and throw. Exactly what the guard is supposed to prevent. - OnRenderCompleted calls Renderer.NotifyRenderCompleted which Dequeue()s an unacknowledged batch. Stale duplicate / error / out-of-order acks from a torn-down page can throw. Tighten the guard to: drop EVERYTHING when JSRuntime.IsDisposed EXCEPT EndInvokeJS. EndInvokeJS remains the lone exception because it only completes pending InvokeAsync<T> task completion sources; dropping it would leave caller-awaited tasks hanging indefinitely. * IpcSender.NotifyUnhandledException did two independent dispatcher InvokeAsync calls — a disposal racing between them produced an inconsistent partial state (message-sent-without-rethrow, or vice versa). Combine them into a single dispatcher delegate with one _disposed re-check, making the disposed-or-not decision atomic at execution time. * TryDispatchAsync now uses the captured page-context reference after the identity check (was reading _currentPageContext after the check passed — functionally equivalent today, but fragile against a future refactor that drops the re-check). New test: * IpcReceiver_AllowsEndInvokeJSThroughAfterRuntimeDisposed_CompletesPendingInvocation - The most-important-to-not-skip test from the rubber-duck pass. Starts a real InvokeAsync<T> to register a pending tracker, extracts the asyncHandle from the outbound BeginInvokeJS, calls MarkAsDisconnected, delivers the JS-side EndInvokeJS reply, and asserts the original Task completes with the supplied result. This proves end-to-end that the EndInvokeJS pass-through actually works — without it, the rationale for the refined guard is only code-comment-deep. All 16 tests pass; library + Photino consumer build clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Round 3: second rubber-duck passA second independent rubber-duck pass on top of
New test
Proves end-to-end that the Final state
|
Two real bugs caught on the round-3 commits:
* WebViewManager._currentPageContext was a plain reference field, but
round 3 made it written from BOTH the dispatcher thread (in
AttachToPageAsync's atomic detach) AND the caller's thread (in
DisposeAsyncCore's atomic detach). Reads happen from MessageReceived,
TryDispatchAsync, and Add/RemoveRootComponentAsync — none of which
are guaranteed on the dispatcher. Without acquire/release semantics
the round-3 atomic-detach pattern is itself race-prone (a racing
reader can observe a stale non-null reference after the writer
already nulled it, then try to dispose it a second time). Mark the
field volatile.
* IpcReceiver's round-3 'allow only EndInvokeJS through' rule was too
narrow. When InvokeAsync<T> returns a byte[], the chunked-byte-array
transport sends the bytes via separate ReceiveByteArrayFromJS IPC
messages BEFORE the EndInvokeJS that references them by id.
DotNetDispatcher.ReceiveByteArray stores the bytes in the runtime's
internal cache (it does NOT invoke user code), so EndInvokeJS
deserialization can resolve the {"__byte[]":<id>} placeholder
against the cache. Dropping the byte-chunk messages would prevent
pending InvokeAsync<byte[]> calls from completing — defeating the
whole reason EndInvokeJS is on the allow list. Add
ReceiveByteArrayFromJS to the allow list.
New test:
* IpcReceiver_AllowsReceiveByteArrayFromJSThroughAfterRuntimeDisposed_CompletesPendingByteArrayInvocation
- Starts a real InvokeAsync<byte[]> to register a pending tracker,
captures the asyncHandle, MarkAsDisconnected, then delivers the
ReceiveByteArrayFromJS payload + EndInvokeJS placeholder reply.
Asserts the pending Task completes with the exact byte payload.
Proves end-to-end that the byte-array pass-through actually works
under the tightened guard.
All 17 tests pass; library + Photino consumer build clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Third rubber-duck pass identified that round 3's 'atomic detach' for _currentPageContext (capture-into-local, then null) wasn't actually atomic: two participants — AttachToPageAsync and DisposeAsyncCore — could both read the same non-null value before either wrote null, then both await DisposeAsync on the same context (PageContext.DisposeAsync is not idempotent). Round 4's volatile made the read atomic-visible but didn't sequence the read-modify-write. Round 4 also left a publish-after-dispose race intact: T1 (AttachToPageAsync) could pass its second _disposed check, then T2 (DisposeAsyncCore) could flip _disposed and exit while T1 was constructing the new PageContext, then T1 would assign the new context to _currentPageContext — leaking a scope + renderer + JS runtime permanently (nobody would ever dispose it). Adopting a SemaphoreSlim around both AttachToPageAsync and DisposeAsyncCore eliminates the entire race class. Holding the semaphore through the publish step means T2 cannot interleave between the second _disposed check and the assignment. Holding it through the await of previousPageContext.DisposeAsync means T1 and T2 cannot both observe the same non-null current context. Behavior change: a DisposeAsync() call that arrives while an AttachToPageAsync is in flight now waits for the attach to complete before proceeding (previously they raced). This is the safer semantics — it ensures the attach's scope + renderer are fully constructed before they get torn down, and that no scope is leaked. Also added a test exercising the renderer-TCS-hang scenario that the rubber-duck flagged as a possible concern (round 2 leftover): trigger an attach + render, dispose the WebViewManager without ever sending an OnRenderCompleted ack. The test passes immediately, confirming that the renderer's pending render-batch TCSs are not awaited by its disposal path and the hang is theoretical. Documenting via test rather than adding production code that wouldn't actually fix anything. Also hardened the EndInvokeJS + ReceiveByteArrayFromJS pass-through tests to find the BeginInvokeJS IPC by message type rather than relying on SentIpcMessages.Last() (per the round-4 rubber-duck nit about fragile .Last() assumptions in tests). All 18 tests pass; library + Photino consumer build clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Round 5: third rubber-duck passThe third rubber-duck pass found that round 3's "atomic detach" wasn't actually atomic — two participants could both read the same non-null Round 5 (
What changed semanticallyA Final state
Known limitations (out of scope for this PR)
|
Improve Blazor WebView disconnect/disposal behavior and race safety
Description
This updates Blazor WebView teardown behavior so JS interop and IPC paths handle page reload and WebView shutdown safely, including follow-up race-condition fixes from review feedback.
WebViewJSRuntimeas disconnected duringPageContext.DisposeAsyncsoBeginInvokeJSfails withJSDisconnectedException, whileEndInvokeDotNet/SendByteArrayno-op after disposal.IpcSenderat the start ofWebViewManager.DisposeAsyncCoreand adds execution-time disposal checks in queued dispatcher delegates to prevent TOCTOU races for outbound messages and unhandled-exception dispatch.IpcReceiverdisposal handling to drop only disposed-scope user-invocation messages, while still allowing framework completion/event messages (EndInvokeJS, render/location completion paths).AttachToPageAsync(including post-await race window), and guardsTryDispatchAsyncagainst running work on disposed state.volatileto ensure cross-thread visibility.