Memory leak in plugin tree views

[xcariba]

Bug Description:

Theia always fetches whole tree in plugin tree views when change event is fired. (It calls tree refresh without argument and it causes complete refresh).
In tree-views.ts Theia calls clearChildren to dispose previous commands and remove children before new child elements are fetched.
When full refresh is called it begins with root node (with empty id) and old nodes are not properly disposed/removed. If tree is huge and is constantly updating it consumes several gb of memory and sometimes even crashes. I found out that old commands are not disposed and still kept in command registry (and maybe somewhere else). Same tree works fine in VSCode.

Steps to Reproduce:

1. Install vscode extension that has huge tree view and is constanlty updating.
2. Open it
3. Open chrome inspector (memory tab).
4. Start tree updates and wait for some time.
5. In chrome inspector memory consumption will increase (from 35mb to 600-900mb in my case)

Additional Information

• Operating System: Fedora
• Theia Version: 1.16, master.

[tsmaeder]

When full refresh is called it begins with root node (with empty id) and old nodes are not properly disposed/removed

@xcariba What makes you say that old nodes are not properly disposed? How would nodes escape being disposed when refreshing the root with id ''?

[xcariba]

@tsmaeder I have a custom (proprietary) extension that uses tree view and first implementation was constantly updating. In chrome developer tools I found out that when frequent updates occur memory consumption can go (from ~35mb up to ~1Gb). So I optimized my code so that it sends updates a lot less frequent. It still consumes a lot of memory after some time, but now it can work for a ~day before it reaches high numbers.
I made several memory dumps and it had a lot of old versions of tree view item objects and its parts (like commands). Maybe some references does not allow garbage collector to dispose of these objects.

[tsmaeder]

@xcariba do the "new" elements have the same id's as the "old" elements or do they always change?

[xcariba]

@tsmaeder they have same ids

[pisv]

Looking at the current implementation of the TreeViewExtImpl<T>.getChildren(parentId: string) method, every time this method is called with parentId === '', a new root node is created and placed into this.nodes map, replacing the old root node (if any) in the map:

theia/packages/plugin-ext/src/plugin/tree/tree-views.ts

Lines 373 to 376 in dacd392

	if (parentId === '') {
	const rootNodeDisposables = new DisposableCollection();
	this.nodes.set(parentId, { id: '', disposables: rootNodeDisposables, dispose: () => { rootNodeDisposables.dispose(); } });
	}

The newly created top-level children are pushed to parentNode.children:

theia/packages/plugin-ext/src/plugin/tree/tree-views.ts

Lines 402 to 406 in dacd392

	if (parentNode) {
	const children = parentNode.children || [];
	children.push(node);
	parentNode.children = children;
	}

However, parentNode is the old root rather than the new one:

theia/packages/plugin-ext/src/plugin/tree/tree-views.ts

Lines 363 to 364 in dacd392

	async getChildren(parentId: string): Promise<TreeViewItem[] | undefined> {
	const parentNode = this.nodes.get(parentId);

This causes two issues with regard to memory leaks:

1. The root node that is currently in this.nodes map has no children and therefore nothing is ever done in the clearChildren call for the root node:

theia/packages/plugin-ext/src/plugin/tree/tree-views.ts

Line 370 in dacd392

this.clearChildren(parentNode);

As a result of the child nodes not being disposed, the associated commands placed into the CommandsConverter.commandsMap will not be deleted and the map will grow up each time the tree view is refreshed:

theia/packages/plugin-ext/src/plugin/command-registry.ts

Lines 187 to 188 in dacd392

	this.commandsMap.set(id, command);
	disposables.push(new Disposable(() => this.commandsMap.delete(id)));

2. Each root node implicitly holds a reference to the 'previous' root node (where its children are incorrectly added) due to the parentNode variable being in a closure of the arrow function of the node's dispose property:

theia/packages/plugin-ext/src/plugin/tree/tree-views.ts

Line 375 in dacd392

this.nodes.set(parentId, { id: '', disposables: rootNodeDisposables, dispose: () => { rootNodeDisposables.dispose(); } });

As a result of this 'chaining' of the trees, the nodes accumulate on each refresh. This can cause a massive memory leak.

The following screenshots illustrate the issue when opening Theia on the vscode-extension-samples/helloworld-sample workspace and using the Node Dependencies view contributed by the vscode-extension-samples/tree-view-sample:

As can be seen, the nodes accumulate each time the view is refreshed.

---

It looks like it can be fixed by reusing the single root node instead of creating a new one each time the getChildren method is called with parentId === ''. I have tried the fix locally and it seems to work fine:

I can try to provide the patch, if needed.

[pisv]

Since this is a help wanted issue, I have submitted a PR proactively. Although the fix is not complicated, hopefully it can still help save a keystroke or two.