Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Possible event listener leak in webviews, when starting without a trace server #307

Open
marcdumais-work opened this issue Mar 6, 2025 · 2 comments
Labels
bug Something isn't working

Comments

@marcdumais-work
Copy link
Contributor

marcdumais-work commented Mar 6, 2025

Bug Description:

When testing something else, I noticed the following warning in the debug console:

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 EXPERIMENT_SELECTED listeners added to [SignalManager]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit
(Use exe --trace-warnings ... to show where the warning was created)

I used the debugger to confirm - it looks like when the extension is started without a trace server, the webviews are created and at least 4 of them registers a listener for event EXPERIMENT_SELECTED (some views register other event listeners too, but that event is the first to hit the "max 10" limit).

Then when the trace server is started, it looks like the same 4 webviews are probably re-created or at least go through their init again, registering new listeners.

From there it's just a matter of opening a few traces - each "trace panel" registers one EXPERIMENT_SELECTED event listener and we go over the 10 listeners limit for event EXPERIMENT_SELECTED, and the warning is printed.

Steps to Reproduce:

  1. Add the following breakpoints and watches:
  • vscode-trace-extension/node_modules/traceviewer-base/src/signals/signal-manager.ts line 78 (signalManager#on()) and line 101 (#off())
    • add conditional breakpoint: event == EXPERIMENT_SELECTED
  • add a watch: "this._events.EXPERIMENT_SELECTED" - this will permit seeing quickly how many listeners are currently registered for that event.
  • vscode-trace-extension/vscode-trace-extension/src/trace-explorer/abstract-trace-explorer-provider.ts line 72 (AbstractTraceExplorerProvider#resolveWebviewView())
    • add breakpoint
  1. Start a debug session without a trace server running
  2. The debugger should stop 4 times in AbstractTraceExplorerProvider@init() - you can look in object wenviewView -> #r to see which webview is being initialized. Step-in the specific webview implementation's "init()". - in init() you should see a listener being registered for event "EXPERIMENT_SELECTED", e.g. something like signalManager().on('EXPERIMENT_SELECTED', this._onExperimentSelected);.
  • Continue execution and the breakpoint in signalManager should hit.
  1. Continue debugging - you should see 4 event listeners being created from the 4 webviews

Image

  1. Start the trace server and refresh the trace side panel using the button - the 4 same webviews should re-register the same event listeners, for a total of 8

Image

  1. Open around 3 traces - for each one's trace panel, a new event listener should be registered. Watch for the warning about a potential leak in the debug console - you can follow the count of registered listener looking at the previously created "watch" while stopped in SignalManager.:

Image

Image


Additional Information:

Here is where the webviews entrypoint seems to be - AbstractTraceExplorerProvider class. the init of the specific webview is called, an in there various event listeners are registered (webview-specific):

AbstractTraceExplorerProvider:
Image

Webview "'traceExplorer.openedTracesView'":

Image

Webview "traceExplorer.availableViews":

Image

Webview "traceExplorer.timeRangeDataView":

Image

Webview "traceExplorer.itemPropertiesView":

Image

Additional Information

  • Operating System: Ubuntu 22.04
  • VSCode Version: VSCodium v1.97.2
@marcdumais-work marcdumais-work added the bug Something isn't working label Mar 6, 2025
@marcdumais-work
Copy link
Contributor Author

From what I can see, even without the leak (which adds 4 listeners), we would get the warning with ~7 traces open, which seems a not-unreasonable use-case. Maybe we should consider increasing the limit as per the warning.

@marcdumais-work marcdumais-work changed the title Pssible event listener leak in webviews, when starting without a trace server Possible event listener leak in webviews, when starting without a trace server Mar 6, 2025
@marcdumais-work
Copy link
Contributor Author

marcdumais-work commented Mar 6, 2025

I think the issue may be related to this line, towards the end of the extension's activate(), where momentarily we simulate the trace server being up (when sometimes it's not yet) - when that happens, the various werbviews are created and register listeners:

Image

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant