Skip to content

Commit

Permalink
Fix memory leak in DockerPanelRenderer and ToolbarAwareTabBar (#13327)
Browse files Browse the repository at this point in the history
The DockerPanelRenderer was not disposing the core preference listener.
The ToolbarAwareTabBar was not disposing the TabBarToolbar.

Signed-off-by: Vivien Jovet <[email protected]>
Co-authored-by: Vivien Jovet <[email protected]>
  • Loading branch information
Hanksha and vjovet-toro-cloud authored Feb 20, 2024
1 parent 25a84b2 commit f6c8fe5
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 5 deletions.
67 changes: 67 additions & 0 deletions examples/playwright/src/tests/theia-application-shell.test.ts
Original file line number Diff line number Diff line change
@@ -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();
}
});
});
31 changes: 31 additions & 0 deletions examples/playwright/src/theia-welcome-view.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}
7 changes: 5 additions & 2 deletions packages/core/src/browser/shell/application-shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/browser/shell/tab-bars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit f6c8fe5

Please sign in to comment.