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

[365] potential listener LEAK detected #164704

Closed
jrieken opened this issue Oct 26, 2022 · 4 comments
Closed

[365] potential listener LEAK detected #164704

jrieken opened this issue Oct 26, 2022 · 4 comments
Assignees
Labels
freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues insiders-released Patch has been released in VS Code Insiders
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Oct 26, 2022

  • have an eager comments controller extension (see below)
  • open a notebook
  • create a new cell
  • leak warnings are printed
event.js:467 [365] potential listener LEAK detected, having 3431 listeners already. MOST frequent listener (3257):
event.js:468 Error
    at Stacktrace.create (event.js:478:35)
    at MainThreadCommentThread._event [as onDidChangeLabel] (event.js:576:44)
    at CommentThreadWidget._bindCommentThreadListeners (commentThreadWidget.js:150:69)
    at CommentThreadWidget.updateCommentThread (commentThreadWidget.js:98:18)
    at cellComments.js:79:55
    at process.processTicksAndRejections (node:internal/process/task_queues:96:5)
const cctrl = vscode.comments.createCommentController('foo', "HELLO")

	vscode.window.onDidChangeActiveTextEditor(e => {
		if (!e) {
			return;
		}
		const thread = cctrl.createCommentThread(e.document.uri, new vscode.Range(0, 0, 1, 1), [{
			author: { name: 'FOOO' },
			body: `Hello ${e.document.uri.path}`,
			mode: vscode.CommentMode.Editing
		}])

	})
@jrieken jrieken added the freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues label Oct 26, 2022
@jrieken
Copy link
Member Author

jrieken commented Oct 26, 2022

This seems to be rooted at updateCommentThread

It looks like a new _commentThreadDisposables is created without always disposing the old one. The if (this._commentThread !== commentThread) { looks highly suspicious.

Screenshot 2022-10-26 at 14 09 41

@jrieken jrieken added this to the October 2022 milestone Oct 26, 2022
@jrieken
Copy link
Member Author

jrieken commented Oct 26, 2022

Suggesting to tackle this for October because after a short while VS Code was crawling due to excessive listening

@rebornix
Copy link
Member

@jrieken thanks for good reproduce. The comment thread equality check is wrong, it's an oversight during refactoring iirc. I pushed a fix for this memory leak. Though with it fixed, I found there are issues with "Editing" state of comment thread for cells (focus issue), will look into this next week.

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Oct 26, 2022
@jrieken
Copy link
Member Author

jrieken commented Oct 27, 2022

Thanks @rebornix

@VSCodeTriageBot VSCodeTriageBot 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 27, 2022
formigoni pushed a commit to formigoni/vscode that referenced this issue Oct 27, 2022
* Fix microsoft#164704. comment thread memory leak.

* Dispose first.

* spell check
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Development

No branches or pull requests

4 participants