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 #3

Closed
mlucool opened this issue Mar 10, 2023 · 2 comments · Fixed by #4
Closed

Memory leak #3

mlucool opened this issue Mar 10, 2023 · 2 comments · Fixed by #4

Comments

@mlucool
Copy link
Member

mlucool commented Mar 10, 2023

With limit output set head/tail set to 50 characters, the following reproducers a memory leak likely in this plugin:

import sys
import time

for _ in range(1_000):
    sys.stderr.write('w'*1000)
    sys.stderr.flush()
    time.sleep(0.01)

If you watch process memory it shoots up rapidly. When you clear all outputs it still stays high. Looking at a post-clear all outputs heap snapshot you can see the old strings there:
image

It's not clear the cause, but maybe we need to do something special for dispose?

@krassowski
Copy link
Contributor

I see a memory leak associated with the use of context menu over the output node (which can be used to select "Clear Outputs" or "Clear All Outputs").

Here is the diff when using command palette:

Screenshot from 2023-03-12 16-59-26

And here is one when using context menu on the output:

Screenshot from 2023-03-12 17-01-34

This is because when context menu gets invoked, the event gets retained on this._contextMenuEvent in evtContextMenu to enable later access by contextMenuHitTest() if needed.

Screenshot from 2023-03-12 17-07-12

As for fixing this, we could empty the contents of the node on disposal. This way the detached node will still be there but it will not contain a reference to the long string or anything else, allowing those to be freed. Any solution that would dispose the problematic event reference would break contextMenuHitTest which is relied upon by extensions.

Can you confirm that you do not see a memory leak when using command palette to clear outputs?

@mlucool
Copy link
Member Author

mlucool commented Mar 13, 2023

I can no longer reproduce this without limit output. I must have cleared output differently,


Looking at this more, I think this is an upstream bug and maybe limit output makes it worse.

Given and no limit output plugin installed:

import sys
import time

for _ in range(10):
    sys.stderr.write('w'*10_000)
    sys.stderr.flush()
    time.sleep(0.01)

Then I run clear output from the command pallet, I would expect to see no wwwww... strings in the heap snapshot, but I do.

That being said, it seems the command palette does better than clear output for freeing memory when this plugin is enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants