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.

Sign up for GitHub

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

Jump to bottom

Potential leak of notebook outputs #131145

Closed
Tracked by #130950
mjbvz opened this issue Aug 19, 2021 · 2 comments
Closed
Tracked by #130950

Potential leak of notebook outputs #131145

mjbvz opened this issue Aug 19, 2021 · 2 comments
Assignees
@mjbvz
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Milestone
August 2021

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Aug 19, 2021

Debugging notebook memory usage, I noticed that this code is rendering markdown but not dealing with the resulting disposables:

vscode/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts

Line 40 in 955a1ac

disposeables: new DisposableStore()

This seems to cause a notebook output data buffer to be retained even after the notebook is closed
@mjbvz mjbvz added this to the August 2021 milestone Aug 19, 2021
@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label Aug 19, 2021
@mjbvz mjbvz mentioned this issue Aug 19, 2021
Closed
2 tasks
@mjbvz mjbvz closed this as completed in baa68c6 Aug 19, 2021
@mjbvz
Copy link
Contributor Author

mjbvz commented Aug 19, 2021

One more potential case (although I don't see it causing leaks):

vscode/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts

Line 808 in b88a5c3

disposables: new DisposableStore()

@mjbvz mjbvz reopened this Aug 19, 2021
AiverReaver pushed a commit to AiverReaver/vscode that referenced this issue Aug 19, 2021
@mjbvz @AiverReaver
Make sure we dispose of rendered markdown for notebooks
6aba094 
Fixes microsoft#131145
@mjbvz mjbvz closed this as completed in a64f66f Aug 19, 2021
@rzhao271
Copy link
Contributor

@mjbvz what are some verification steps for this issue?

@rzhao271 rzhao271 added the verification-steps-needed Steps to verify are needed for verification label Aug 24, 2021
@mjbvz mjbvz added debt Code quality issues and removed verification-steps-needed Steps to verify are needed for verification bug Issue identified by VS Code Team member as probable bug labels Aug 24, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mjbvz mjbvz

Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Milestone
August 2021
Development

No branches or pull requests
4 participants
@rebornix @rzhao271 @mjbvz and others