`NotebookModel` memory leak

[AlbertHilb]

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]

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

[AlbertHilb]

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]

Thanks for investigating!

[telamonian]

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]

@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]

IIRC, this line should disconnect this signal connection

[jasongrout]

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]

IIRC, this line should disconnect this signal connection

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

[AlbertHilb]

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]

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.