Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Memory leak in menu #195580

Closed
SimonSiefke opened this issue Oct 13, 2023 · 5 comments · Fixed by #196302
Closed

Memory leak in menu #195580

SimonSiefke opened this issue Oct 13, 2023 · 5 comments · Fixed by #196302
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders
Milestone

Comments

@SimonSiefke
Copy link
Contributor

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.83.0
  • OS Version: Ubuntu 23.04

Steps to Reproduce:

  1. Open a VSCode Window
  2. In the menu at the top, click on "File" which opens the menu
  3. In the menu at the top, click on "File" again which closes the menu
  4. A keydown event listener has been added, but not been removed
{
  "eventListeners": [
    {
      "type": "keydown",
      "description": "N=>{new P.$qO(N).equals(2)&&N.preventDefault()}",
      "objectId": "8193754018518281278.4.217608",
      "stack": [
        "listener (.vscode-test/vscode-linux-x64-1.83.0/resources/app/out/vs/workbench/workbench.desktop.main.js:244:39878)"
      ],
      "sourceMaps": [
        "https://ticino.blob.core.windows.net/sourcemaps/e7e037083ff4455cf320e344325dacb480062c3c/core/vs/workbench/workbench.desktop.main.js.map"
      ],
      "originalStack": [
        "/src/vs/base/browser/ui/menu/menu.ts:122:58"
      ]
    }
  ]
}

It seems the issue is here:

addDisposableListener(menuElement, EventType.KEY_DOWN, (e) => {
const event = new StandardKeyboardEvent(e);
// Stop tab navigation of menus
if (event.equals(KeyCode.Tab)) {
e.preventDefault();
}
});

@vscodenpa
Copy link

Thanks for creating this issue! It looks like you may be using an old version of VS Code, the latest stable release is 1.83.1. Please try upgrading to the latest version and checking whether this issue remains.

Happy Coding!

@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Oct 23, 2023
@vscodenpa vscodenpa added this to the October 2023 milestone Oct 23, 2023
@rzhao271
Copy link
Contributor

Thanks @SimonSiefke !
I'm wondering how you got the event listener list?

@SimonSiefke
Copy link
Contributor Author

You're welcome! I have a project here: https://github.com/SimonSiefke/vscode-memory-leak-finder

Basic Overview

There are several end-to-end tests, which also measure event listeners to find memory leaks.

The title-bar-menu-toggle.js end-to-end test is defined here: https://github.com/SimonSiefke/vscode-memory-leak-finder/blob/c69c2f819a1db4003d6d83c21d1758d92df5cdc2/packages/e2e/src/title-bar-menu-toggle.js.

It can be executed like this:

git clone git@github.com:SimonSiefke/vscode-memory-leak-finder.git &&
cd vscode-memory-leak-finder &&
npm ci &&
node packages/cli/bin/test.js --cwd packages/e2e  --check-leaks --measure event-listeners  --only title-bar-menu-toggle &&
cat .vscode-memory-leak-finder-results/event-listeners/title-bar-menu-toggle.json

More Details

Before and after a test is executed, all event listeners are queried using Chrome Devtools Protocol Runtime.queryObjects({ prototypeId: "EventTarget.prototype" }) and DomDebugger.getEventListeners.

We get an array of event listeners before and after, for example

// before
[
  {
    "type": "focusin",
    "description": "()=>this.j()",
    "objectId": "524841679309534768.4.2930",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:148:37007)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ]
  }
]

and

// after
[
  {
    "type": "focusin",
    "description": "()=>this.j()",
    "objectId": "524841679309534768.4.2930",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:148:37007)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ]
  },
  {
    "type": "keydown",
    "description": "N=>{new P.$qO(N).equals(2)&&N.preventDefault()}",
    "objectId": "3680313440875909344.4.4572",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:39878)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "originalStack": ["/src/vs/base/browser/ui/menu/menu.ts:122:58"]
  }
]

The before and after arrays are compared to see which event listeners have been added. In the example above, there is one keydown listener more in the after array which is not in the before array.

The tests are structured in a way one would be expect that the number of event listeners before and after the test are equal. For example, when opening and closing the menu, one would expect the number of event listeners stays equal. This is the menu toggle test:

// title-bar-menu-toggle.js
export const run = async ({ TitleBar }) => {
  await TitleBar.showMenuFile()
  await TitleBar.hideMenuFile()
}

Every time the test was executed, event listeners increased by one keydown listener in /src/vs/base/browser/ui/menu/menu.ts:122:58, which indicates a memory leak and in this case is precisely the location of the memory leak.

In other cases, the output for memory leaks might not be quite as clear, but maybe still helpful. This is the output for the notebook-open test (opening and closing a notebook):

[
  {
    "type": "contextmenu",
    "description": "n=>{t.$_O.stop(n,!0)}",
    "objectId": "2723967474668247540.4.13637",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:18357)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "count": 1,
    "originalStack": ["/src/vs/base/browser/ui/actionbar/actionbar.ts:370:117"]
  },
  {
    "type": "-monaco-gesturetap",
    "description": "r=>this.onClick(r,!0)",
    "objectId": "2723967474668247540.4.13695",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:10198)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "count": 1,
    "originalStack": ["/src/vs/base/browser/ui/actionbar/actionViewItems.ts:121:68"]
  },
  {
    "type": "mousedown",
    "description": "r=>{c||I.$_O.stop(r,!0),this._action.enabled&&r.button===0&&o.classList.add(\"active\")}",
    "objectId": "2723967474668247540.4.13697",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:10258)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "count": 1,
    "originalStack": ["/src/vs/base/browser/ui/actionbar/actionViewItems.ts:123:70"]
  },
  {
    "type": "click",
    "description": "r=>{I.$_O.stop(r,!0),this.m&&this.m.isMenu||this.onClick(r)}",
    "objectId": "2723967474668247540.4.13699",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:10475)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "count": 1,
    "originalStack": ["/src/vs/base/browser/ui/actionbar/actionViewItems.ts:145:65"]
  },
  {
    "type": "dblclick",
    "description": "r=>{I.$_O.stop(r,!0)}",
    "objectId": "2723967474668247540.4.13701",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:10572)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "count": 1,
    "originalStack": ["/src/vs/base/browser/ui/actionbar/actionViewItems.ts:154:68"]
  },
  {
    "type": "mouseout",
    "description": "n=>{I.$_O.stop(n),o.classList.remove(\"active\")}",
    "objectId": "2723967474668247540.4.13705",
    "stack": [
      "listener (file:///home/simon/.cache/repos/vscode-memory-leak-finder/.vscode-test/vscode-linux-x64-1.83.1/resources/app/out/vs/workbench/workbench.desktop.main.js:244:10662)"
    ],
    "sourceMaps": [
      "https://ticino.blob.core.windows.net/sourcemaps/f1b07bd25dfad64b0167beb15359ae573aecd2cc/core/vs/workbench/workbench.desktop.main.js.map"
    ],
    "count": 2,
    "originalStack": ["/src/vs/base/browser/ui/actionbar/actionViewItems.ts:159:56"]
  }
]

It seems there is memory leak when opening and closing a notebook. But just looking at the output, I can't say much more. I'm not even sure where exactly the memory leak is and would need to look more closely at the actionbar.ts and actionViewItems.ts code.

I hope this helps! :)

@bpasero
Copy link
Member

bpasero commented Oct 24, 2023

This is really cool, feel free to file more issues as you find them!

How do you run VS Code in this model, are you starting it via playwright? Is it the web version or the electron version?

@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Oct 24, 2023
@SimonSiefke
Copy link
Contributor Author

How it runs:

  1. By default the VSCode electron application is downloaded using https://github.com/microsoft/vscode-test or if it exists the VSCODE_PATH environment variable is used
  2. VSCode is launched using spawn(vscodePath, ['--inspect-brk=0', '--remote-debugging-port=0'])
  3. An electron application in debug mode always prints a websocket url to stderr, for example
/snap/code/current/usr/share/code/code --inspect-brk=0
Debugger listening on ws://127.0.0.1:35511/32d7e970-b8ee-46f1-b63a-c3ecf9dfb296
For help, see: https://nodejs.org/en/docs/inspector
  1. WebSocket connections are created using new WebSocket("ws://127.0.0.1:35511/32d7e970-b8ee-46f1-b63a-c3ecf9dfb296")
  2. Through the Websocket connections, chrome devtools protocol commands are being sent and events received
  3. Once a VSCode window is launched, a script is injected into the page which makes the testing easier, e.g. globalThis.test.type('abc') finds the active input element, updates the input value and triggers input and change events.
  4. The tests run in nodejs, but make use of the injected test code by sending devtools protocol websocket messages, for example
// call injected code "type" function
await Runtime.callFunctionOn({ functionDeclaration: 'value => globalThis.test.type(value)', arguments: ['abc'] })

The tests were initially using playwright, though eventually it seemed useful to have more customization, for example reusing the same VSCode instance for all tests so that the tests execute faster.

All in all, it's very similar to the way playwright runs end-to-end tests. :)

@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants