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

NotebookModel memory leak #8371

Open
AlbertHilb opened this issue May 5, 2020 · 10 comments
Open

NotebookModel memory leak #8371

AlbertHilb opened this issue May 5, 2020 · 10 comments

Comments

@AlbertHilb
Copy link
Contributor

NotebookModel is not deleted after a notebook tab is closed.

Reproduce

  1. Open jupyterlab.
  2. Open a notebook.
  3. Close it.
  4. Repeat steps 2 and 3 three times.
  5. Open Chrome DevTools.
  6. Take a Heap Snapshot.
  7. Three NotebookModels are present.

Expected behavior

I don't master jupyterlab internal design, but I think no NotebookModel should be in memory or, at most, only one.

Context

  • Browser and version: Chrome 81
  • JupyterLab version: 2.1.0
@jasongrout
Copy link
Contributor

Just checking: by closing this issue, are you saying that the notebook models actually are garbage collected after all?

@AlbertHilb
Copy link
Contributor Author

AlbertHilb commented May 5, 2020

No, unfortunateIy not.
I discovered also other objects related to notebooks are not deleted (Notebook, NotebookPanel, NotebookPanelLayout).
I'm investigating if there is a root memory leak, the others depends on, to modify the issue accordingly.

@jasongrout
Copy link
Contributor

Thanks for investigating!

@telamonian
Copy link
Member

Does the memory leak still occur if you have multiple tabs open when you close the notebook tab? I did notice a potential memory leak issue a while back where some Lumino cleanup code doesn't get called on the closed TabBar item if it's the last one in the TabBar. At the time, I didn't investigate it further to see if it was causing any more pervasive issues

@AlbertHilb AlbertHilb reopened this May 5, 2020
@AlbertHilb
Copy link
Contributor Author

AlbertHilb commented May 5, 2020

@telamonian Yes, it is still present.

NotebookModel has different retainers, one of which is Context.
Context, in turn, has many retainers. One of them seems to be this signal connection, since adding explicit disconnection of that signal inside Context.dispose reduces retainers by one.

Most of the retainers seem signaling related.

@jasongrout
Copy link
Contributor

IIRC, this line should disconnect this signal connection

@jasongrout
Copy link
Contributor

On disposal, IIRC, Signal.clearData(this) should be called to disconnect any signal handlers where this was given as the context, the second argument to .connect. If there is somewhere those disconnections aren't happening, we should fix that, so thanks for tracing things down. Any more specifics?

@AlbertHilb
Copy link
Contributor Author

IIRC, this line should disconnect this signal connection

I know, but, maybe, it doesn't. I will take a look tomorrow.

@AlbertHilb
Copy link
Contributor Author

I confirm, the signal mentioned above is one of the causes of the memory leak.

This is what happens.

When the signal connection is set, an IConnection object with thisArg property set to Context is added to the IConnection array associated to the ContentsManager in the receiversForSender WeakMap.

That thisArg property is not cleared by Signal.ClearData and this prevent Context from being garbage collected.

However, as I said, this is only one of ten Context retainers.

@AlbertHilb
Copy link
Contributor Author

Another cause of the Context memory leak is the following.

When a new Notebook is added to the NotebookTracker a new RenderedVDOM renderer factory is added to the notebook's rendermime registry (see here).

The createRenderer property of that factory is definited as

createRenderer: options => new RenderedVDOM(options, context)

So it contains a reference to Context that is not cleared after the notebook is closed.

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

No branches or pull requests

3 participants