Make sure event subscriptions are stored and disposed (#214234) (#214312

)

* Make sure event subscriptions are stored and disposed (#214234)

* fix tests

src/vs/code/electron-main/app.ts

@@ -366,7 +366,7 @@ export class CodeApplication extends Disposable {
 		process.on('unhandledRejection', (reason: unknown) => onUnexpectedError(reason));
 
 		// Dispose on shutdown
-		this.lifecycleMainService.onWillShutdown(() => this.dispose());
+		Event.once(this.lifecycleMainService.onWillShutdown)(() => this.dispose());
 
 		// Contextmenu via IPC support
 		registerContextMenuListener();
@@ -598,7 +598,7 @@ export class CodeApplication extends Disposable {
 
 		// Main process server (electron IPC based)
 		const mainProcessElectronServer = new ElectronIPCServer();
-		this.lifecycleMainService.onWillShutdown(e => {
+		Event.once(this.lifecycleMainService.onWillShutdown)(e => {
 			if (e.reason === ShutdownReason.KILL) {
 				// When we go down abnormally, make sure to free up
 				// any IPC we accept from other windows to reduce

src/vs/platform/menubar/electron-main/menubar.ts

@@ -25,6 +25,7 @@ import { IUpdateService, StateType } from 'vs/platform/update/common/update';
 import { INativeRunActionInWindowRequest, INativeRunKeybindingInWindowRequest, IWindowOpenable, hasNativeTitlebar } from 'vs/platform/window/common/window';
 import { IWindowsCountChangedEvent, IWindowsMainService, OpenContext } from 'vs/platform/windows/electron-main/windows';
 import { IWorkspacesHistoryMainService } from 'vs/platform/workspaces/electron-main/workspacesHistoryMainService';
+import { Disposable } from 'vs/base/common/lifecycle';
 
 const telemetryFrom = 'menu';
 
@@ -42,7 +43,7 @@ interface IMenuItemWithKeybinding {
 	userSettingsLabel?: string;
 }
 
-export class Menubar {
+export class Menubar extends Disposable {
 
 	private static readonly lastKnownMenubarStorageKey = 'lastKnownMenubarData';
 
@@ -78,6 +79,8 @@ export class Menubar {
 		@IProductService private readonly productService: IProductService,
 		@IAuxiliaryWindowsMainService private readonly auxiliaryWindowsMainService: IAuxiliaryWindowsMainService
 	) {
+		super();
+
 		this.menuUpdater = new RunOnceScheduler(() => this.doUpdateMenu(), 0);
 
 		this.menuGC = new RunOnceScheduler(() => { this.oldMenus = []; }, 10000);
@@ -169,12 +172,12 @@ export class Menubar {
 	private registerListeners(): void {
 
 		// Keep flag when app quits
-		this.lifecycleMainService.onWillShutdown(() => this.willShutdown = true);
+		this._register(this.lifecycleMainService.onWillShutdown(() => this.willShutdown = true));
 
 		// Listen to some events from window service to update menu
-		this.windowsMainService.onDidChangeWindowsCount(e => this.onDidChangeWindowsCount(e));
-		this.nativeHostMainService.onDidBlurMainWindow(() => this.onDidChangeWindowFocus());
-		this.nativeHostMainService.onDidFocusMainWindow(() => this.onDidChangeWindowFocus());
+		this._register(this.windowsMainService.onDidChangeWindowsCount(e => this.onDidChangeWindowsCount(e)));
+		this._register(this.nativeHostMainService.onDidBlurMainWindow(() => this.onDidChangeWindowFocus()));
+		this._register(this.nativeHostMainService.onDidFocusMainWindow(() => this.onDidChangeWindowFocus()));
 	}
 
 	private get currentEnableMenuBarMnemonics(): boolean {

src/vs/platform/menubar/electron-main/menubarMainService.ts

@@ -8,31 +8,32 @@ import { ILifecycleMainService, LifecycleMainPhase } from 'vs/platform/lifecycle
 import { ILogService } from 'vs/platform/log/common/log';
 import { ICommonMenubarService, IMenubarData } from 'vs/platform/menubar/common/menubar';
 import { Menubar } from 'vs/platform/menubar/electron-main/menubar';
+import { Disposable } from 'vs/base/common/lifecycle';
 
 export const IMenubarMainService = createDecorator<IMenubarMainService>('menubarMainService');
 
 export interface IMenubarMainService extends ICommonMenubarService {
 	readonly _serviceBrand: undefined;
 }
 
-export class MenubarMainService implements IMenubarMainService {
+export class MenubarMainService extends Disposable implements IMenubarMainService {
 
 	declare readonly _serviceBrand: undefined;
 
-	private menubar: Promise<Menubar>;
+	private readonly menubar = this.installMenuBarAfterWindowOpen();
 
 	constructor(
 		@IInstantiationService private readonly instantiationService: IInstantiationService,
 		@ILifecycleMainService private readonly lifecycleMainService: ILifecycleMainService,
 		@ILogService private readonly logService: ILogService
 	) {
-		this.menubar = this.installMenuBarAfterWindowOpen();
+		super();
 	}
 
 	private async installMenuBarAfterWindowOpen(): Promise<Menubar> {
 		await this.lifecycleMainService.when(LifecycleMainPhase.AfterWindowOpen);
 
-		return this.instantiationService.createInstance(Menubar);
+		return this._register(this.instantiationService.createInstance(Menubar));
 	}
 
 	async updateMenubar(windowId: number, menus: IMenubarData): Promise<void> {

src/vs/platform/windows/electron-main/windowsStateHandler.ts

@@ -85,19 +85,19 @@ export class WindowsStateHandler extends Disposable {
 		});
 
 		// Handle various lifecycle events around windows
-		this.lifecycleMainService.onBeforeCloseWindow(window => this.onBeforeCloseWindow(window));
-		this.lifecycleMainService.onBeforeShutdown(() => this.onBeforeShutdown());
-		this.windowsMainService.onDidChangeWindowsCount(e => {
+		this._register(this.lifecycleMainService.onBeforeCloseWindow(window => this.onBeforeCloseWindow(window)));
+		this._register(this.lifecycleMainService.onBeforeShutdown(() => this.onBeforeShutdown()));
+		this._register(this.windowsMainService.onDidChangeWindowsCount(e => {
 			if (e.newCount - e.oldCount > 0) {
 				// clear last closed window state when a new window opens. this helps on macOS where
 				// otherwise closing the last window, opening a new window and then quitting would
 				// use the state of the previously closed window when restarting.
 				this.lastClosedState = undefined;
 			}
-		});
+		}));
 
 		// try to save state before destroy because close will not fire
-		this.windowsMainService.onDidDestroyWindow(window => this.onBeforeCloseWindow(window));
+		this._register(this.windowsMainService.onDidDestroyWindow(window => this.onBeforeCloseWindow(window)));
 	}
 
 	// Note that onBeforeShutdown() and onBeforeCloseWindow() are fired in different order depending on the OS:

src/vs/workbench/browser/layout.ts

@@ -608,7 +608,7 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
 			this.stateModel.setRuntimeValue(LayoutStateKeys.EDITOR_HIDDEN, false);
 		}
 
-		this.stateModel.onDidChangeState(change => {
+		this._register(this.stateModel.onDidChangeState(change => {
 			if (change.key === LayoutStateKeys.ACTIVITYBAR_HIDDEN) {
 				this.setActivityBarHidden(change.value as boolean);
 			}
@@ -630,7 +630,7 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
 			}
 
 			this.doUpdateLayoutConfiguration();
-		});
+		}));
 
 		// Layout Initialization State
 		const initialEditorsState = this.getInitialEditorsState();

src/vs/workbench/common/contributions.ts

@@ -250,7 +250,8 @@ export class WorkbenchContributionsRegistry extends Disposable implements IWorkb
 		const environmentService = this.environmentService = accessor.get(IEnvironmentService);
 		const editorPaneService = this.editorPaneService = accessor.get(IEditorPaneService);
 
-		this._register(lifecycleService.onWillShutdown(() => {
+		// Dispose contributions on shutdown
+		this._register(lifecycleService.onDidShutdown(() => {
 			this.instanceDisposables.clear();
 		}));
 

src/vs/workbench/contrib/output/browser/outputLinkProvider.ts

@@ -11,11 +11,11 @@ import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace
 import { OUTPUT_MODE_ID, LOG_MODE_ID } from 'vs/workbench/services/output/common/output';
 import { MonacoWebWorker, createWebWorker } from 'vs/editor/browser/services/webWorker';
 import { ICreateData, OutputLinkComputer } from 'vs/workbench/contrib/output/common/outputLinkComputer';
-import { IDisposable, dispose } from 'vs/base/common/lifecycle';
+import { IDisposable, dispose, Disposable } from 'vs/base/common/lifecycle';
 import { ILanguageConfigurationService } from 'vs/editor/common/languages/languageConfigurationRegistry';
 import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures';
 
-export class OutputLinkProvider {
+export class OutputLinkProvider extends Disposable {
 
 	private static readonly DISPOSE_WORKER_TIME = 3 * 60 * 1000; // dispose worker after 3 minutes of inactivity
 
@@ -29,14 +29,16 @@ export class OutputLinkProvider {
 		@ILanguageConfigurationService private readonly languageConfigurationService: ILanguageConfigurationService,
 		@ILanguageFeaturesService private readonly languageFeaturesService: ILanguageFeaturesService,
 	) {
+		super();
+
 		this.disposeWorkerScheduler = new RunOnceScheduler(() => this.disposeWorker(), OutputLinkProvider.DISPOSE_WORKER_TIME);
 
 		this.registerListeners();
 		this.updateLinkProviderWorker();
 	}
 
 	private registerListeners(): void {
-		this.contextService.onDidChangeWorkspaceFolders(() => this.updateLinkProviderWorker());
+		this._register(this.contextService.onDidChangeWorkspaceFolders(() => this.updateLinkProviderWorker()));
 	}
 
 	private updateLinkProviderWorker(): void {

src/vs/workbench/contrib/output/browser/outputServices.ts

@@ -103,8 +103,8 @@ export class OutputService extends Disposable implements IOutputService, ITextMo
 		this.activeOutputChannelLevelIsDefaultContext = CONTEXT_ACTIVE_OUTPUT_LEVEL_IS_DEFAULT.bindTo(contextKeyService);
 
 		// Register as text model content provider for output
-		textModelResolverService.registerTextModelContentProvider(OUTPUT_SCHEME, this);
-		instantiationService.createInstance(OutputLinkProvider);
+		this._register(textModelResolverService.registerTextModelContentProvider(OUTPUT_SCHEME, this));
+		this._register(instantiationService.createInstance(OutputLinkProvider));
 
 		// Create output channels for already registered channels
 		const registry = Registry.as<IOutputChannelRegistry>(Extensions.OutputChannels);

src/vs/workbench/services/dialogs/test/electron-sandbox/fileDialogService.test.ts

@@ -101,7 +101,7 @@ suite('FileDialogService', function () {
 
 		const dialogService = instantiationService.createInstance(TestFileDialogService, new TestSimpleFileDialog());
 		instantiationService.set(IFileDialogService, dialogService);
-		const workspaceService: IWorkspaceEditingService = instantiationService.createInstance(BrowserWorkspaceEditingService);
+		const workspaceService: IWorkspaceEditingService = disposables.add(instantiationService.createInstance(BrowserWorkspaceEditingService));
 		assert.strictEqual((await workspaceService.pickNewWorkspacePath())?.path.startsWith(testFile.path), true);
 		assert.strictEqual(await dialogService.pickWorkspaceAndOpen({}), undefined);
 	});
@@ -126,7 +126,7 @@ suite('FileDialogService', function () {
 		} as IPathService);
 		const dialogService = instantiationService.createInstance(TestFileDialogService, new TestSimpleFileDialog());
 		instantiationService.set(IFileDialogService, dialogService);
-		const workspaceService: IWorkspaceEditingService = instantiationService.createInstance(BrowserWorkspaceEditingService);
+		const workspaceService: IWorkspaceEditingService = disposables.add(instantiationService.createInstance(BrowserWorkspaceEditingService));
 		assert.strictEqual((await workspaceService.pickNewWorkspacePath())?.path.startsWith(testFile.path), true);
 		assert.strictEqual(await dialogService.pickWorkspaceAndOpen({}), undefined);
 	});
@@ -158,7 +158,7 @@ suite('FileDialogService', function () {
 		} as IPathService);
 		const dialogService = instantiationService.createInstance(TestFileDialogService, new TestSimpleFileDialog());
 		instantiationService.set(IFileDialogService, dialogService);
-		const workspaceService: IWorkspaceEditingService = instantiationService.createInstance(BrowserWorkspaceEditingService);
+		const workspaceService: IWorkspaceEditingService = disposables.add(instantiationService.createInstance(BrowserWorkspaceEditingService));
 		assert.strictEqual((await workspaceService.pickNewWorkspacePath())?.path.startsWith(testFile.path), true);
 		assert.strictEqual(await dialogService.pickWorkspaceAndOpen({}), undefined);
 	});

src/vs/workbench/services/workspaces/browser/abstractWorkspaceEditingService.ts

@@ -29,8 +29,9 @@ import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/w
 import { IWorkbenchConfigurationService } from 'vs/workbench/services/configuration/common/configuration';
 import { IUserDataProfilesService } from 'vs/platform/userDataProfile/common/userDataProfile';
 import { IUserDataProfileService } from 'vs/workbench/services/userDataProfile/common/userDataProfile';
+import { Disposable } from 'vs/base/common/lifecycle';
 
-export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditingService {
+export abstract class AbstractWorkspaceEditingService extends Disposable implements IWorkspaceEditingService {
 
 	declare readonly _serviceBrand: undefined;
 
@@ -51,7 +52,9 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 		@IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService,
 		@IUserDataProfilesService private readonly userDataProfilesService: IUserDataProfilesService,
 		@IUserDataProfileService private readonly userDataProfileService: IUserDataProfileService,
-	) { }
+	) {
+		super();
+	}
 
 	async pickNewWorkspacePath(): Promise<URI | undefined> {
 		const availableFileSystems = [Schemas.file];

src/vs/workbench/services/workspaces/electron-sandbox/workspaceEditingService.ts

@@ -67,10 +67,10 @@ export class NativeWorkspaceEditingService extends AbstractWorkspaceEditingServi
 	}
 
 	private registerListeners(): void {
-		this.lifecycleService.onBeforeShutdown(e => {
+		this._register(this.lifecycleService.onBeforeShutdown(e => {
 			const saveOperation = this.saveUntitledBeforeShutdown(e.reason);
 			e.veto(saveOperation, 'veto.untitledWorkspace');
-		});
+		}));
 	}
 
 	private async saveUntitledBeforeShutdown(reason: ShutdownReason): Promise<boolean> {