From 028ee501711b3d0c386dd1390423eff40eb4a266 Mon Sep 17 00:00:00 2001 From: Vivien Jovet Date: Mon, 29 Jan 2024 15:51:29 +0100 Subject: [PATCH] Fix memory leak in DockerPanelRenderer and ToolbarAwareTabBar The DockerPanelRenderer was not disposing the core preference listener. The ToolbarAwareTabBar was not disposing the TabBarToolbar. Signed-off-by: Vivien Jovet --- .../src/tests/theia-application-shell.test.ts | 67 +++++++++++++++++++ examples/playwright/src/theia-welcome-view.ts | 31 +++++++++ .../src/browser/shell/application-shell.ts | 7 +- packages/core/src/browser/shell/tab-bars.ts | 6 +- 4 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 examples/playwright/src/tests/theia-application-shell.test.ts create mode 100644 examples/playwright/src/theia-welcome-view.ts diff --git a/examples/playwright/src/tests/theia-application-shell.test.ts b/examples/playwright/src/tests/theia-application-shell.test.ts new file mode 100644 index 0000000000000..779fc5358c18f --- /dev/null +++ b/examples/playwright/src/tests/theia-application-shell.test.ts @@ -0,0 +1,67 @@ +// ***************************************************************************** +// Copyright (C) 2023 Toro Cloud Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 + +import { test } from '@playwright/test'; +import { TheiaApp } from '../theia-app'; +import { TheiaAppLoader } from '../theia-app-loader'; +import { TheiaExplorerView } from '../theia-explorer-view'; +import { TheiaTextEditor } from '../theia-text-editor'; +import { TheiaWelcomeView } from '../theia-welcome-view'; +import { TheiaWorkspace } from '../theia-workspace'; + +test.describe('Theia Application Shell', () => { + test.describe.configure({ + timeout: 120000 + }); + + let app: TheiaApp; + + test.beforeAll(async ({ playwright, browser }) => { + const ws = new TheiaWorkspace(['src/tests/resources/sample-files1']); + app = await TheiaAppLoader.load({ playwright, browser }, ws); + + // The welcome view must be closed because the memory leak only occurs when there are + // no tabs left open. + const welcomeView = new TheiaWelcomeView(app); + + if (await welcomeView.isTabVisible()) { + await welcomeView.close(); + } + }); + + test.afterAll(async () => { + await app.page.close(); + }); + + /** + * The aim of this test is to detect memory leaks when opening and closing editors many times. + * Remove the skip and run the test, check the logs for any memory leak warnings. + * It should take less than 2min to run, if it takes longer than that, just increase the timeout. + */ + test.skip('should open and close a text editor many times', async () => { + for (let i = 0; i < 200; i++) { + const explorer = await app.openView(TheiaExplorerView); + + const fileStatNode = await explorer.getFileStatNodeByLabel('sample.txt'); + const contextMenu = await fileStatNode.openContextMenu(); + await contextMenu.clickMenuItem('Open'); + + const textEditor = new TheiaTextEditor('sample.txt', app); + await textEditor.waitForVisible(); + + await textEditor.close(); + } + }); +}); diff --git a/examples/playwright/src/theia-welcome-view.ts b/examples/playwright/src/theia-welcome-view.ts new file mode 100644 index 0000000000000..ce68a7f02f2df --- /dev/null +++ b/examples/playwright/src/theia-welcome-view.ts @@ -0,0 +1,31 @@ +// ***************************************************************************** +// Copyright (C) 2023 Toro Cloud Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 + +import { TheiaApp } from './theia-app'; +import { TheiaView } from './theia-view'; +import { normalizeId } from './util'; + +const TheiaWelcomeViewData = { + tabSelector: normalizeId('#shell-tab-getting.started.widget'), + viewSelector: normalizeId('#getting.started.widget'), + viewName: 'Welcome' +}; + +export class TheiaWelcomeView extends TheiaView { + + constructor(app: TheiaApp) { + super(TheiaWelcomeViewData, app); + } +} diff --git a/packages/core/src/browser/shell/application-shell.ts b/packages/core/src/browser/shell/application-shell.ts index 3e86fb211fe78..8331ad86fec09 100644 --- a/packages/core/src/browser/shell/application-shell.ts +++ b/packages/core/src/browser/shell/application-shell.ts @@ -132,16 +132,19 @@ export class DockPanelRenderer implements DockLayout.IRenderer { getDynamicTabOptions()); this.tabBarClasses.forEach(c => tabBar.addClass(c)); renderer.tabBar = tabBar; - tabBar.disposed.connect(() => renderer.dispose()); renderer.contextMenuPath = SHELL_TABBAR_CONTEXT_MENU; tabBar.currentChanged.connect(this.onCurrentTabChanged, this); - this.corePreferences.onPreferenceChanged(change => { + const prefChangeDisposable = this.corePreferences.onPreferenceChanged(change => { if (change.preferenceName === 'workbench.tab.shrinkToFit.enabled' || change.preferenceName === 'workbench.tab.shrinkToFit.minimumSize' || change.preferenceName === 'workbench.tab.shrinkToFit.defaultSize') { tabBar.dynamicTabOptions = getDynamicTabOptions(); } }); + tabBar.disposed.connect(() => { + prefChangeDisposable.dispose(); + renderer.dispose(); + }); this.onDidCreateTabBarEmitter.fire(tabBar); return tabBar; } diff --git a/packages/core/src/browser/shell/tab-bars.ts b/packages/core/src/browser/shell/tab-bars.ts index c43f42c73bbcf..794f59d917c73 100644 --- a/packages/core/src/browser/shell/tab-bars.ts +++ b/packages/core/src/browser/shell/tab-bars.ts @@ -170,8 +170,8 @@ export class TabBarRenderer extends TabBar.Renderer { const hover = this.tabBar && (this.tabBar.orientation === 'horizontal' && this.corePreferences?.['window.tabbar.enhancedPreview'] === 'classic') ? { title: title.caption } : { - onmouseenter: this.handleMouseEnterEvent - }; + onmouseenter: this.handleMouseEnterEvent + }; return h.li( { @@ -967,7 +967,7 @@ export class ToolbarAwareTabBar extends ScrollableTabBar { protected override onBeforeDetach(msg: Message): void { if (this.toolbar && this.toolbar.isAttached) { - Widget.detach(this.toolbar); + this.toolbar.dispose(); } super.onBeforeDetach(msg); }