Skip to content

Commit f6c8fe5

Browse files
Hankshavjovet-lonti
andauthoredFeb 20, 2024··
Fix memory leak in DockerPanelRenderer and ToolbarAwareTabBar (#13327)
The DockerPanelRenderer was not disposing the core preference listener. The ToolbarAwareTabBar was not disposing the TabBarToolbar. Signed-off-by: Vivien Jovet <vivien.jovet@torocloud.com> Co-authored-by: Vivien Jovet <vivien.jovet@torocloud.com>
1 parent 25a84b2 commit f6c8fe5

File tree

4 files changed

+106
-5
lines changed

4 files changed

+106
-5
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// *****************************************************************************
2+
// Copyright (C) 2023 Toro Cloud Pty Ltd and others.
3+
//
4+
// This program and the accompanying materials are made available under the
5+
// terms of the Eclipse Public License v. 2.0 which is available at
6+
// http://www.eclipse.org/legal/epl-2.0.
7+
//
8+
// This Source Code may also be made available under the following Secondary
9+
// Licenses when the conditions for such availability set forth in the Eclipse
10+
// Public License v. 2.0 are satisfied: GNU General Public License, version 2
11+
// with the GNU Classpath Exception which is available at
12+
// https://www.gnu.org/software/classpath/license.html.
13+
//
14+
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
15+
16+
import { test } from '@playwright/test';
17+
import { TheiaApp } from '../theia-app';
18+
import { TheiaAppLoader } from '../theia-app-loader';
19+
import { TheiaExplorerView } from '../theia-explorer-view';
20+
import { TheiaTextEditor } from '../theia-text-editor';
21+
import { TheiaWelcomeView } from '../theia-welcome-view';
22+
import { TheiaWorkspace } from '../theia-workspace';
23+
24+
test.describe('Theia Application Shell', () => {
25+
test.describe.configure({
26+
timeout: 120000
27+
});
28+
29+
let app: TheiaApp;
30+
31+
test.beforeAll(async ({ playwright, browser }) => {
32+
const ws = new TheiaWorkspace(['src/tests/resources/sample-files1']);
33+
app = await TheiaAppLoader.load({ playwright, browser }, ws);
34+
35+
// The welcome view must be closed because the memory leak only occurs when there are
36+
// no tabs left open.
37+
const welcomeView = new TheiaWelcomeView(app);
38+
39+
if (await welcomeView.isTabVisible()) {
40+
await welcomeView.close();
41+
}
42+
});
43+
44+
test.afterAll(async () => {
45+
await app.page.close();
46+
});
47+
48+
/**
49+
* The aim of this test is to detect memory leaks when opening and closing editors many times.
50+
* Remove the skip and run the test, check the logs for any memory leak warnings.
51+
* It should take less than 2min to run, if it takes longer than that, just increase the timeout.
52+
*/
53+
test.skip('should open and close a text editor many times', async () => {
54+
for (let i = 0; i < 200; i++) {
55+
const explorer = await app.openView(TheiaExplorerView);
56+
57+
const fileStatNode = await explorer.getFileStatNodeByLabel('sample.txt');
58+
const contextMenu = await fileStatNode.openContextMenu();
59+
await contextMenu.clickMenuItem('Open');
60+
61+
const textEditor = new TheiaTextEditor('sample.txt', app);
62+
await textEditor.waitForVisible();
63+
64+
await textEditor.close();
65+
}
66+
});
67+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// *****************************************************************************
2+
// Copyright (C) 2023 Toro Cloud Pty Ltd and others.
3+
//
4+
// This program and the accompanying materials are made available under the
5+
// terms of the Eclipse Public License v. 2.0 which is available at
6+
// http://www.eclipse.org/legal/epl-2.0.
7+
//
8+
// This Source Code may also be made available under the following Secondary
9+
// Licenses when the conditions for such availability set forth in the Eclipse
10+
// Public License v. 2.0 are satisfied: GNU General Public License, version 2
11+
// with the GNU Classpath Exception which is available at
12+
// https://www.gnu.org/software/classpath/license.html.
13+
//
14+
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
15+
16+
import { TheiaApp } from './theia-app';
17+
import { TheiaView } from './theia-view';
18+
import { normalizeId } from './util';
19+
20+
const TheiaWelcomeViewData = {
21+
tabSelector: normalizeId('#shell-tab-getting.started.widget'),
22+
viewSelector: normalizeId('#getting.started.widget'),
23+
viewName: 'Welcome'
24+
};
25+
26+
export class TheiaWelcomeView extends TheiaView {
27+
28+
constructor(app: TheiaApp) {
29+
super(TheiaWelcomeViewData, app);
30+
}
31+
}

‎packages/core/src/browser/shell/application-shell.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,19 @@ export class DockPanelRenderer implements DockLayout.IRenderer {
132132
getDynamicTabOptions());
133133
this.tabBarClasses.forEach(c => tabBar.addClass(c));
134134
renderer.tabBar = tabBar;
135-
tabBar.disposed.connect(() => renderer.dispose());
136135
renderer.contextMenuPath = SHELL_TABBAR_CONTEXT_MENU;
137136
tabBar.currentChanged.connect(this.onCurrentTabChanged, this);
138-
this.corePreferences.onPreferenceChanged(change => {
137+
const prefChangeDisposable = this.corePreferences.onPreferenceChanged(change => {
139138
if (change.preferenceName === 'workbench.tab.shrinkToFit.enabled' ||
140139
change.preferenceName === 'workbench.tab.shrinkToFit.minimumSize' ||
141140
change.preferenceName === 'workbench.tab.shrinkToFit.defaultSize') {
142141
tabBar.dynamicTabOptions = getDynamicTabOptions();
143142
}
144143
});
144+
tabBar.disposed.connect(() => {
145+
prefChangeDisposable.dispose();
146+
renderer.dispose();
147+
});
145148
this.onDidCreateTabBarEmitter.fire(tabBar);
146149
return tabBar;
147150
}

‎packages/core/src/browser/shell/tab-bars.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ export class TabBarRenderer extends TabBar.Renderer {
170170
const hover = this.tabBar && (this.tabBar.orientation === 'horizontal' && this.corePreferences?.['window.tabbar.enhancedPreview'] === 'classic')
171171
? { title: title.caption }
172172
: {
173-
onmouseenter: this.handleMouseEnterEvent
174-
};
173+
onmouseenter: this.handleMouseEnterEvent
174+
};
175175

176176
return h.li(
177177
{
@@ -967,7 +967,7 @@ export class ToolbarAwareTabBar extends ScrollableTabBar {
967967

968968
protected override onBeforeDetach(msg: Message): void {
969969
if (this.toolbar && this.toolbar.isAttached) {
970-
Widget.detach(this.toolbar);
970+
this.toolbar.dispose();
971971
}
972972
super.onBeforeDetach(msg);
973973
}

0 commit comments

Comments
 (0)
Please sign in to comment.