fix: cap concurrency in detectOpenDevToolsWindows to avoid CDP fan-out on many-tab profiles#2274
Open
mvanhorn wants to merge 1 commit into
Open
Conversation
detectOpenDevToolsWindows ran an unbounded Promise.all over every open page, issuing one hasDevTools()/openDevTools() CDP round-trip per tab simultaneously. On profiles with a large number of tabs this fans out hundreds to thousands of concurrent CDP calls on the first tool call, which can hang or crash the browser. Bound the fan-out with a small fixed-size worker pool (mapWithConcurrency, limit 10) so at most a constant number of CDP round-trips are in flight at once. Behavior is unchanged for profiles at or below the cap, and the existing per-page try/catch fallback for pre-144 Chrome/Electron is preserved. Refs ChromeDevTools#1921
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Caps the concurrency of the DevTools-window detection pass so it no longer issues one simultaneous CDP call per open tab.
Why
detectOpenDevToolsWindows()insrc/McpContext.tsfanned out an unboundedPromise.all(pages.map(...))over every open page, so each tab issued its ownhasDevTools()/openDevTools()CDP round-trip at the same time. This method runs on init and again on every response that carriesincludePages, so a single tool call could put one concurrent CDP call in flight per tab.On a profile with a very large number of tabs this becomes hundreds to thousands of simultaneous CDP calls on the first tool call, which is one of the paths that can make the browser hang or crash (Refs #1921).
Change
Bound the fan-out with a small fixed-size worker pool. A new
mapWithConcurrency(items, limit, worker)helper runs at mostlimitworkers at once (default cap10,DEVTOOLS_DETECTION_CONCURRENCY), each pulling the next page from a shared index.detectOpenDevToolsWindows()now walks the pages through this helper with the exact same per-page body it had before.try/catchfallback for pre-144.0.7559.59Chrome/Electron (where the DevTools command throws) is preserved, and one page failing does not abort detection for the rest.This is deliberately narrow: it only caps the concurrency of the existing detection pass and does not change what gets detected or attached. It is one piece of the broader many-tab hardening tracked in #1921, not a full fix for it.
Testing
Added unit tests for
mapWithConcurrencyintests/McpContext.test.tscovering:npm run check-format,npm run build, andnpm testall pass locally.