From 964f64a6079bff29c743a10c2d883d01d07b724a Mon Sep 17 00:00:00 2001 From: Vladimir Piskarev Date: Mon, 27 Mar 2023 15:27:36 +0300 Subject: [PATCH] Fix memory leak in plugin tree views Fixes a bug in `TreeViewExtImpl.getChildren(parentId: string)`, where newly created top-level children were added to the old root node instead of the new one. This caused a memory leak by retaining an implicit reference to the old root node from the new one and failing to properly track and, hence, dispose the top-level children. The fix ensures that old nodes get disposed and become subject to garbage collection by reusing the single root node instead of creating a new one on full refresh. See https://github.com/eclipse-theia/theia/issues/10404#issuecomment-1481293316 for more information. Fixes #10404. --- packages/plugin-ext/src/plugin/tree/tree-views.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/plugin-ext/src/plugin/tree/tree-views.ts b/packages/plugin-ext/src/plugin/tree/tree-views.ts index c5a84674b9d21..81778ae89a180 100644 --- a/packages/plugin-ext/src/plugin/tree/tree-views.ts +++ b/packages/plugin-ext/src/plugin/tree/tree-views.ts @@ -361,7 +361,7 @@ class TreeViewExtImpl implements Disposable { } async getChildren(parentId: string): Promise { - const parentNode = this.nodes.get(parentId); + let parentNode = this.nodes.get(parentId); const parent = parentNode?.value; if (parentId && !parent) { console.error(`No tree item with id '${parentId}' found.`); @@ -370,9 +370,10 @@ class TreeViewExtImpl implements Disposable { this.clearChildren(parentNode); // place root in the cache - if (parentId === '') { + if (parentId === '' && !parentNode) { const rootNodeDisposables = new DisposableCollection(); - this.nodes.set(parentId, { id: '', disposables: rootNodeDisposables, dispose: () => { rootNodeDisposables.dispose(); } }); + parentNode = { id: '', disposables: rootNodeDisposables, dispose: () => { rootNodeDisposables.dispose(); } }; + this.nodes.set(parentId, parentNode); } // ask data provider for children for cached element const result = await this.options.treeDataProvider.getChildren(parent);