fix: don't let a listener exception wedge EventEmitter - #2402
Closed
Joaquín Ruales (jruales) wants to merge 1 commit into
Closed
fix: don't let a listener exception wedge EventEmitter#2402Joaquín Ruales (jruales) wants to merge 1 commit into
Joaquín Ruales (jruales) wants to merge 1 commit into
Conversation
`EventEmitter.fire` left `_deliveryQueue` populated when a listener threw, so every later `fire` took the re-entrancy path and returned without dispatching. One exception permanently stopped all delivery on that emitter, which silently killed the CDP message pump for the connection and left the debuggee waiting for `Runtime.runIfWaitingForDebugger`. Restore the queue in a `finally` block, and treat messages for unknown sessions the same way messages for disposed sessions are already treated (warn and ignore) rather than throwing. Electron emits `Inspector.workerScriptLoaded` for sessions we don't track, which made this reachable in normal operation. Fixes microsoft#2401 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Fixes #2401
Problem
EventEmitter.fireleaves_deliveryQueuepopulated when a listener throws:Because the queue is never cleared, every later
firecomputesdispatch === false, enqueues, and returns without dispatching. One listener exception permanently stops all delivery on that emitter.Connectionreads its transport throughthis._transport.onMessage(...), so this silently kills the CDP message pump for the whole connection. The debuggee is then never sentRuntime.runIfWaitingForDebuggerand stays paused at startup — the window paints but accepts no input.The exception comes from
Connection._onMessage, which ignores messages for disposed sessions but throws for unknown ones.Inspector.workerScriptLoaded(a Blink event fired when a web worker finishes evaluating) is never handled by js-debug and can arrive before the worker's session is registered, sincetargetCreatedis queued throughenqueueLifecycleFn:Changes
src/common/events.ts— restore_deliveryQueuein afinallyblock so one bad listener can't wedge the emitter. This is the load-bearing fix and is independent of the CDP specifics.src/cdp/connection.ts— treat unknown sessions the way disposed sessions are already treated (warn and ignore) instead of throwing.src/common/events.test.ts— regression test.Validation
events.tschange (expected [ 'before' ] to deeply equal [ 'before', 'after' ]) and passes with it.npm run test:unit— 268 passing.tsc --noEmitandgulp lintclean.--extensionDevelopmentPath; same machine, repository (microsoft/vscode, Electron 42.8.1), isolated profile and launch compound, only this diff varying.Happy to split the
connection.tschange into a separate PR if you'd prefer to evaluate the two independently.