Repository navigation
feat(inspector): serve multiple DevTools clients at once - #526
Draft
edusperoni wants to merge 1 commit into
Draft
edusperoni wants to merge 1 commit into
edusperoni wants to merge 1 commit into
Conversation
Each connection now gets its own V8InspectorSession on the main isolate and on every worker, and sees only replies to its own commands, as with Chrome and Node. InspectorSessions keeps the per-connection sessions for one isolate; connect, disconnect and messages share one queue, so a client's session exists before its first message and outlives its last. A departing client's session is destroyed, which disables its agents: a pause ends only if no other client still has the debugger enabled. A session dispatching when its client leaves (the pause loop can run under its own Runtime.evaluate) gets Debugger.disable at once and is destroyed when that dispatch returns. Per-client state: Target auto-attach and announced workers, IO streams, and tracing (one owner at a time). @nativescript/core's JS domains are global, so their events go to every client and a <Domain>.disable only reaches the handler once no other client has the domain enabled. The server no longer closes the previous client. It drops a message read just before a write error closed its connection, closes a client more than 256 MB behind on writes, and no longer logs connection resets and broken pipes. Also fix a trap in libdispatch when a message dispatched by the drain pauses (e.g. evaluating `debugger` in the console): the drain held messageLoopQueue_ while dispatching and the pause loop dispatch_sync'd onto it. The drain now only reads the pause-loop flag under that queue. This replaces the single-client sync sentinel, session rebinding and frontend generations.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
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.
What is the current behavior?
The inspector serves one client at a time (#516). A second connection closes the first, and the single V8 session is reset or rebound for the newcomer. You cannot, for example, keep Chrome DevTools open while a CDP script or a second DevTools window attaches.
What is the new behavior?
Any number of clients can connect at once, as with Chrome and Node. Each gets its own V8 session and sees only replies to its own commands.
Sessions
InspectorSessionskeeps oneV8InspectorSessionper connection on an isolate. Each session's channel writes only to its own connection. It is used for the main isolate and for every worker.Runtime.evaluate. In that case it getsDebugger.disableright away, so the pause is released if nobody else wants it, and it is destroyed when that dispatch returns.Workers
Target.setAutoAttachand the set of announced workers are tracked per client.attachedToTargetanddetachedFromTargetgo only to the clients that asked.Shared state
Debugger.paused, and any of them can resume.Debugger.pauseschedules the pause in the requesting client's session. A request still pending when a pause starts is dropped, so it can't pause again after the resume.Tracing.startuntil itsTracing.endor disconnect, and other clients get "Tracing is already started".Network.loadNetworkResourcestreams belong to the client that opened them.IO.read/IO.closefrom another client get "Invalid stream handle", and the streams are dropped when their client disconnects.<Domain>.disablereaches the handler only once no other client has that domain enabled.HeapProfiler.disable(or its disconnect) stops tracking for everyone. This is a V8 limitation that Chrome shares, documented on the class.Server
InspectorServerno longer closes the previous client.onClientConnectedprecedes its messages andonClientDisconnectedfollows them. A message read just before a write error closed the connection is dropped.Also fixed: a pause triggered by a message the drain dispatched (e.g. evaluating
debuggerin the console) re-entereddispatch_syncon the queue the drain held and trapped in libdispatch. The drain now checks the pause-loop flag under that queue but dispatches outside it. The pause loop schedules a drain when it exits.Known issues / follow-ups
Debugger.pauseon a worker never pauses it, whiledebugger;statements do. This also fails on main before this change, verified with the fix(inspector): release the frontend's resources when DevTools disconnects #516 build, so it is left for a separate fix.enableevent, so they keep emitting into the void. Unchanged from before.Testing
./run_tests.sh --keep-sim): 1835 specs, 0 failures.Debugger.paused, B resumes, and both seeDebugger.resumed.Runtime.evaluate(debugger;): alone, the app resumes. With B debugging, it stays paused, and A's evaluate reply never reaches B. In both cases a later client can pause and resume again. Before the drain fix, this path crashed in libdispatch.attachedToTarget, and both evaluate in the worker with the same id, each getting its own reply. The worker pauses ondebugger;and both see it. A leaves while paused, the worker stays paused for B, B resumes, and B's worker session keeps working.