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

Text buffer memory leak #142933

Closed
cynecx opened this issue Feb 12, 2022 · 4 comments
Closed

Text buffer memory leak #142933

cynecx opened this issue Feb 12, 2022 · 4 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@cynecx
Copy link

cynecx commented Feb 12, 2022

Version: 1.65.0-insider
Commit: bb221a61d29deabd99ee9431736d04f2175cb596
Date: 2022-02-11T05:18:07.499Z
Electron: 16.0.8
Chromium: 96.0.4664.110
Node.js: 16.9.1
V8: 9.6.180.21-electron.0
OS: Linux x64 5.16.8-arch1-2

Same as in #107999.

Steps to Reproduce:

  1. Prepare a large text file (Citylots.json is ~190MB):
wget "https://raw.githubusercontent.com/zemirco/sf-city-lots-json/master/citylots.json"
cp citylots.json evenlarger.json
cat citylots.json >> evenlarger.json
cat citylots.json >> evenlarger.json
  1. Open evenlarger.json in vscode.
  2. Scroll around.
  3. Close the file.
  4. Observe the memory usage through "Process Explorer".

Screenshot from 2022-02-12 18-27-44

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

This is certainly a regression because 1-3 months ago, I haven't noticed this memory leak.

A memory snapshot proves that a reference to the text buffer is still around somewhere:

Screenshot from 2022-02-12 18-30-13

image

@rzhao271 rzhao271 assigned hediet and alexdima and unassigned rzhao271 Feb 14, 2022
@hediet hediet added this to the February 2022 milestone Feb 15, 2022
@hediet hediet added freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues and removed freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues labels Feb 15, 2022
@hediet
Copy link
Member

hediet commented Feb 15, 2022

I cannot reproduce. Also, the download link to the json file is invalid, you need to pass ?raw=true.

Note that garbage collection takes some time until unused text models are cleaned up.
I cannot explain why there are some text models kept alive even if all editors are closed, but even after opening hundreds of files, there are at most two such hanging (and disposed) text models. These text models no longer keep the text buffer alive.

@hediet hediet added the info-needed Issue requires more information from poster label Feb 15, 2022
@bpasero
Copy link
Member

bpasero commented Feb 15, 2022

I think a text model that is being disposed when you close an editor is immediately freed from memory if there are no more references. But one thing I remember is that when an extension resolves a text document for any reason, we only free up such a reference after a certain timeout because there is no API to release the use of a text model. So maybe retry with extensions disabled.

@alexdima
Copy link
Member

alexdima commented Feb 15, 2022

@cynecx I also tried, but failed to reproduce the leakage. However, while investigating I ran into #143111 and #143112 .

In the past, in order to be overly careful, when a TextModel gets disposed I've added that we swap its text buffer with an empty one. This helps to minimize the effect in those rare cases where a reference to a disposed TextModel lingers somewhere.

Looking at your screenshot, I see that there is a reference via v8's deoptimization data left to a disposed TextModel. While we cannot control how v8 functions, we can try to be even more protective. Again, from your screenshot, it looks that TextModel._bufferDisposable still ends up referencing the text buffer and we can also manually remove that reference as part of being overly careful.

@alexdima alexdima removed the info-needed Issue requires more information from poster label Feb 15, 2022
@cynecx
Copy link
Author

cynecx commented Feb 17, 2022

@alexdima 120318d seems to have fixed this. I can't seem to reproduce this on the latest insiders. Thanks!

@alexdima alexdima added bug Issue identified by VS Code Team member as probable bug z-author-verified labels Feb 17, 2022
@roblourens roblourens added the verified Verification succeeded label Feb 25, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants
@roblourens @bpasero @hediet @alexdima @cynecx @rzhao271 and others