Skip to content

t #333786

Description

@vs-code-engineering

Summary

The main-process BrowserViewMainService throws Browser view <id> not found when a renderer subscribes to a per-view "dynamic" event (onDynamicDidChangePermissions in the reported stack) over IPC after that view has already been destroyed. Event subscription is resolved asynchronously across the process boundary, so the view can disappear in the window between the renderer subscribing and the main process handling the listen request. The throw crosses the IPC boundary as an unhandled error and is reported to telemetry (~177 users/bucket across 5 sibling buckets that differ only by the random view GUID). This is a post-fix recurrence of the same class fixed for #318995 in 1.124.0, now surfacing through the newer permissions event path.

Fixes #333783
Recommended reviewer: @kycutler

Culprit Commit

Field Value
Commit 67aea9c1
Author @kycutler
PR #322639
Message Support browser permissions (#322639)
Why This commit added onDynamicDidChangePermissions(id) (and the renderer subscribes to it in BrowserViewModel's constructor), following the existing onDynamic* pattern that calls _getBrowserView(id) — which throws when the view is gone. The permissions event is the specific accessor in the reported stack; all sibling onDynamic* accessors share the same race.

Code Flow

sequenceDiagram
    participant Renderer as BrowserViewModel (renderer)
    participant IPC as IPC channel
    participant Main as BrowserViewMainService
    participant Map as browserViews (DisposableMap)

    Renderer->>IPC: listen onDynamicDidChangePermissions(id)
    Note over Map: ⚠️ Root cause:<br/>view destroyed before<br/>listen request handled
    IPC->>Main: onEventListen -> onDynamicDidChangePermissions(id)
    Main->>Map: _getBrowserView(id) -> get(id) === undefined
    Note over Main: 💥 throw new Error(<br/>`Browser view ${id} not found`)
    Main-->>IPC: error crosses process boundary
Loading

Affected Files

File Role Evidence
src/vs/platform/browserView/electron-main/browserViewMainService.ts crash site L122 (from stack): throw new Error(\Browser view ${id} not found`)`
src/vs/platform/browserView/electron-main/browserViewMainService.ts root cause L224-L226: onDynamicDidChangePermissions(id) returns this._getBrowserView(id).onDidChangePermissions, throwing when the view is already gone
src/vs/workbench/contrib/browserView/common/browserView.ts consumer L539-L540: this._register(this.browserViewService.onDynamicDidChangePermissions(this.id)(snapshot => ...)) subscribes over IPC

Repro Steps

Non-deterministic timing race across the renderer↔main IPC boundary:

  1. Open an integrated browser view so a BrowserViewModel is created and subscribes to onDynamicDidChangePermissions(id) (and the other onDynamic* events) over IPC.
  2. Destroy/close the browser view (e.g. close the editor) so browserViews.deleteAndDispose(id) removes it from the map in the main process.
  3. If the renderer's listen request for any onDynamic* event is handled by the main process after the view is removed, _getBrowserView(id) throws and the error is reported to telemetry.
  4. Likelihood increases when views are opened and closed rapidly, or during window teardown when many subscriptions and disposals interleave.

How the Fix Works

Chosen approach (browserViewMainService.ts): Added a small producer-side helper _getBrowserViewEvent<T>(id, getEvent) that resolves the requested event only when the view still exists and returns Event.None otherwise. All 20 onDynamic* event accessors now route through it instead of this._getBrowserView(id).<event>. This fixes the problem at the producer of the invalid state (the main-process event accessor), not at an unrelated crash site or by silencing telemetry.

A missing view at event-subscription time is an expected outcome of asynchronous IPC subscription, not a real error: the view is gone, so there are no further events to deliver and an empty event stream is the semantically correct result. This is the external/untrusted-boundary case in the lifecycle-race guidance — the consumer lives in a separate process and cannot synchronously coordinate its subscription with main-process disposal, so the producer must tolerate the race for these event getters. Non-event methods (getState, layout, loadURL, etc.) intentionally keep throwing via _getBrowserView, because calling those against a destroyed view is a genuine caller error rather than a benign subscription race.

Alternatives considered:

  • Wrapping the IPC onEventListen in try/catch — hides the error from telemetry for all channels and swallows genuine bugs, rather than fixing the specific event getters that are legitimately racy.
  • Guarding at the renderer consumer (BrowserViewModel) — the renderer cannot know the view was destroyed in the main process at subscription time, so it cannot avoid the race; the fix belongs at the producer.
  • Making _getBrowserView itself return undefined/Event.None for everything — would weaken the invariant for the many state-mutating methods where a missing view really is a bug.

Lifecycle pattern: external/untrusted boundary (cross-process IPC event subscription).
Producer site: src/vs/platform/browserView/electron-main/browserViewMainService.tsonDynamicDidChangePermissions() and sibling onDynamic* accessors.
Consumer-side fix justification: the consumer (BrowserViewModel) runs in the renderer process and subscribes over IPC; it has no synchronous view of main-process disposal, so it cannot prevent the race. The fix is applied at the producer (main-process event accessors), which is the correct side and keeps the throwing contract intact for genuine misuse of state-mutating methods.

Recommended Owner

@kycutler — author of the culprit commit (#322639, "Support browser permissions") and of the surrounding onDynamic* browser-view accessors; actively committing to microsoft/vscode within the last 90 days.

Generated by errors-fix · opus48 · 473.9 AIC · ⌖ 19.5 AIC · ⊞ 18.6K ·


Note

This was originally intended as a pull request, but PR creation failed. The changes have been pushed to the branch fix/browser-view-dynamic-event-race-c58ec365481fc82e.

Original error: ERR_API: [2026-09-01T15:37:12.277Z] create pull request in microsoft/vscode failed (attempt 1)

Original error: Validation Failed: {"resource":"PullRequest","code":"custom","field":"fork_collab","message":"fork_collab Fork collab can't be granted by someone without permission"} - https://docs.github.com/rest/pulls/pulls#create-a-pull-request
Retryable: false
Suggestion: This error cannot be resolved by retrying. Please check the error details and fix the underlying issue.

To create the pull request manually:

gh pr create --title "t" --base main --head vscodebot-pr:fix/browser-view-dynamic-event-race-c58ec365481fc82e --repo microsoft/vscode
Show patch preview (151 of 151 lines)
From dd3d3739906122745b16cb4ec56dce06b6b8ac2f Mon Sep 17 00:00:00 2001
X-GH-AW-Base-Commit: 4735247b4a22a921d588e224f347a2186f11ba2f
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Tue, 1 Sep 2026 15:27:41 +0000
Subject: [PATCH] fix: don't throw resolving browser view dynamic events after
 view destroyed (fixes #333783)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../electron-main/browserViewMainService.ts   | 60 ++++++++++++-------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/src/vs/platform/browserView/electron-main/browserViewMainService.ts b/src/vs/platform/browserView/electron-main/browserViewMainService.ts
index 7de2bb5cd3e..5b78efbf7ec 100644
--- a/src/vs/platform/browserView/electron-main/browserViewMainService.ts
+++ b/src/vs/platform/browserView/electron-main/browserViewMainService.ts
@@ -124,6 +124,26 @@ export class BrowserViewMainService extends Disposable implements IBrowserViewMa
 		return view;
 	}
 
+	/**
+	 * Resolve a dynamic event for a browser view, returning {@link Event.None}
+	 * when the view no longer exists.
+	 *
+	 * Event subscriptions are established asynchronously over IPC: a renderer
+	 * issues a `listen` request and the main process resolves the event getter
+	 * when the message arrives. The view can be destroyed in the window between
+	 * the renderer subscribing and the request being handled, so a missing view
+	 * here is an expected race rather than a bug. In that case there are no
+	 * further events to deliver, so an empty event is the correct result — the
+	 * getter must not throw across the process boundary.
+	 */
+	private _getBrowserViewEvent<T>(id: string, getEvent: (view: BrowserView) => Event<T>): Event<T> {
+		const view = this.browserViews.get(id);
+		if (!view) {
+			return Event.None;
+		}
+		return getEvent(view);
+	}
+
 	private _getViewInfo(view: BrowserView): IBrowserViewInfo {
 		return {
 			id: view.id,
@@ -146,83 +166,83 @@
... (truncated)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions