From 94315b863646ca091565104a56e4ab31cbb65341 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 29 Nov 2018 08:55:50 +0100 Subject: [PATCH] lifecycle - introduce onShutdown event for disposing things after onWillShutdown --- src/vs/platform/lifecycle/common/lifecycle.ts | 28 +++++++++++------ .../electron-browser/lifecycleService.ts | 30 ++++++++++++------- .../api/electron-browser/mainThreadWebview.ts | 6 ++-- src/vs/workbench/electron-browser/main.ts | 2 +- src/vs/workbench/electron-browser/shell.ts | 17 +++++------ .../workbench/electron-browser/workbench.ts | 4 +-- .../electron-browser/task.contribution.ts | 2 +- .../parts/terminal/common/terminalService.ts | 4 +-- .../electron-browser/extensionHost.ts | 4 +-- .../textfile/common/textFileService.ts | 2 +- .../textfile/test/textFileService.test.ts | 14 ++++----- .../workbench/test/workbenchTestServices.ts | 17 +++++++---- 12 files changed, 75 insertions(+), 55 deletions(-) diff --git a/src/vs/platform/lifecycle/common/lifecycle.ts b/src/vs/platform/lifecycle/common/lifecycle.ts index 27a179e3cf1..41fe7ac9089 100644 --- a/src/vs/platform/lifecycle/common/lifecycle.ts +++ b/src/vs/platform/lifecycle/common/lifecycle.ts @@ -18,7 +18,7 @@ export const ILifecycleService = createDecorator('lifecycleSe * Note: It is absolutely important to avoid long running promises if possible. Please try hard * to return a boolean directly. Returning a promise has quite an impact on the shutdown sequence! */ -export interface WillShutdownEvent { +export interface BeforeShutdownEvent { /** * Allows to veto the shutdown. The veto can be a long running operation but it @@ -27,7 +27,7 @@ export interface WillShutdownEvent { veto(value: boolean | Thenable): void; /** - * The reason why Code will be shutting down. + * The reason why the application will be shutting down. */ reason: ShutdownReason; } @@ -40,7 +40,7 @@ export interface WillShutdownEvent { * Note: It is absolutely important to avoid long running promises if possible. Please try hard * to return a boolean directly. Returning a promise has quite an impact on the shutdown sequence! */ -export interface ShutdownEvent { +export interface WillShutdownEvent { /** * Allows to join the shutdown. The promise can be a long running operation but it @@ -49,7 +49,7 @@ export interface ShutdownEvent { join(promise: Thenable): void; /** - * The reason why Code is shutting down. + * The reason why the application is shutting down. */ reason: ShutdownReason; } @@ -137,17 +137,26 @@ export interface ILifecycleService { /** * Fired before shutdown happens. Allows listeners to veto against the - * shutdown. + * shutdown to prevent it from happening. + * + * The event carries a shutdown reason that indicates how the shutdown was triggered. */ - readonly onWillShutdown: Event; + readonly onBeforeShutdown: Event; /** - * Fired when no client is preventing the shutdown from happening. Can be used to dispose heavy resources - * like running processes. Can also be used to save UI state to storage. + * Fired when no client is preventing the shutdown from happening (from onBeforeShutdown). + * Can be used to save UI state even if that is long running through the WillShutdownEvent#join() + * method. * * The event carries a shutdown reason that indicates how the shutdown was triggered. */ - readonly onShutdown: Event; + readonly onWillShutdown: Event; + + /** + * Fired when the shutdown is about to happen after long running shutdown operations + * have finished (from onWillShutdown). This is the right place to dispose resources. + */ + readonly onShutdown: Event; /** * Returns a promise that resolves when a certain lifecycle phase @@ -158,6 +167,7 @@ export interface ILifecycleService { export const NullLifecycleService: ILifecycleService = { _serviceBrand: null, + onBeforeShutdown: Event.None, onWillShutdown: Event.None, onShutdown: Event.None, phase: LifecyclePhase.Restored, diff --git a/src/vs/platform/lifecycle/electron-browser/lifecycleService.ts b/src/vs/platform/lifecycle/electron-browser/lifecycleService.ts index fb0b87eef59..a21b5c3ba72 100644 --- a/src/vs/platform/lifecycle/electron-browser/lifecycleService.ts +++ b/src/vs/platform/lifecycle/electron-browser/lifecycleService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { toErrorMessage } from 'vs/base/common/errorMessage'; -import { ILifecycleService, WillShutdownEvent, ShutdownReason, StartupKind, LifecyclePhase, handleVetos, LifecyclePhaseToString, ShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; +import { ILifecycleService, BeforeShutdownEvent, ShutdownReason, StartupKind, LifecyclePhase, handleVetos, LifecyclePhaseToString, WillShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; import { ipcRenderer as ipc } from 'electron'; import { Event, Emitter } from 'vs/base/common/event'; @@ -22,11 +22,14 @@ export class LifecycleService extends Disposable implements ILifecycleService { _serviceBrand: any; + private readonly _onBeforeShutdown = this._register(new Emitter()); + get onBeforeShutdown(): Event { return this._onBeforeShutdown.event; } + private readonly _onWillShutdown = this._register(new Emitter()); get onWillShutdown(): Event { return this._onWillShutdown.event; } - private readonly _onShutdown = this._register(new Emitter()); - get onShutdown(): Event { return this._onShutdown.event; } + private readonly _onShutdown = this._register(new Emitter()); + get onShutdown(): Event { return this._onShutdown.event; } private readonly _startupKind: StartupKind; get startupKind(): StartupKind { return this._startupKind; } @@ -77,8 +80,8 @@ export class LifecycleService extends Disposable implements ILifecycleService { // store shutdown reason to retrieve next startup this.storageService.store(LifecycleService.LAST_SHUTDOWN_REASON_KEY, JSON.stringify(reply.reason), StorageScope.WORKSPACE); - // trigger onWillShutdown events and veto collecting - this.handleWillShutdown(reply.reason).then(veto => { + // trigger onBeforeShutdown events and veto collecting + this.handleBeforeShutdown(reply.reason).then(veto => { if (veto) { this.logService.trace('lifecycle: onBeforeUnload prevented via veto'); this.storageService.remove(LifecycleService.LAST_SHUTDOWN_REASON_KEY, StorageScope.WORKSPACE); @@ -94,17 +97,22 @@ export class LifecycleService extends Disposable implements ILifecycleService { ipc.on('vscode:onWillUnload', (event, reply: { replyChannel: string, reason: ShutdownReason }) => { this.logService.trace(`lifecycle: onWillUnload (reason: ${reply.reason})`); - // trigger onShutdown events and joining - return this.handleShutdown(reply.reason).then(() => { + // trigger onWillShutdown events and joining + return this.handleWillShutdown(reply.reason).then(() => { + + // trigger onShutdown event now that we know we will quit + this._onShutdown.fire(); + + // acknowledge to main side ipc.send(reply.replyChannel, windowId); }); }); } - private handleWillShutdown(reason: ShutdownReason): Promise { + private handleBeforeShutdown(reason: ShutdownReason): Promise { const vetos: (boolean | Thenable)[] = []; - this._onWillShutdown.fire({ + this._onBeforeShutdown.fire({ veto(value) { vetos.push(value); }, @@ -117,10 +125,10 @@ export class LifecycleService extends Disposable implements ILifecycleService { }); } - private handleShutdown(reason: ShutdownReason): Thenable { + private handleWillShutdown(reason: ShutdownReason): Thenable { const joiners: Thenable[] = []; - this._onShutdown.fire({ + this._onWillShutdown.fire({ join(promise) { if (promise) { joiners.push(promise); diff --git a/src/vs/workbench/api/electron-browser/mainThreadWebview.ts b/src/vs/workbench/api/electron-browser/mainThreadWebview.ts index 74a4a653241..a9f51628d7c 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadWebview.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadWebview.ts @@ -53,8 +53,8 @@ export class MainThreadWebviews implements MainThreadWebviewsShape, WebviewReviv this._toDispose.push(_webviewService.registerReviver(MainThreadWebviews.viewType, this)); - lifecycleService.onWillShutdown(e => { - e.veto(this._onWillShutdown()); + lifecycleService.onBeforeShutdown(e => { + e.veto(this._onBeforeShutdown()); }, this, this._toDispose); } @@ -183,7 +183,7 @@ export class MainThreadWebviews implements MainThreadWebviewsShape, WebviewReviv return this._revivers.has(webview.state.viewType) || !!webview.reviver; } - private _onWillShutdown(): boolean { + private _onBeforeShutdown(): boolean { this._webviews.forEach((view) => { if (this.canRevive(view)) { view.state.state = view.webviewState; diff --git a/src/vs/workbench/electron-browser/main.ts b/src/vs/workbench/electron-browser/main.ts index 0387f0da2be..248eaaebb07 100644 --- a/src/vs/workbench/electron-browser/main.ts +++ b/src/vs/workbench/electron-browser/main.ts @@ -137,7 +137,7 @@ function openWorkbench(configuration: IWindowConfiguration): Promise { }, mainServices, mainProcessClient, configuration); // Gracefully Shutdown Storage - shell.onShutdown(event => { + shell.onWillShutdown(event => { event.join(storageService.close()); }); diff --git a/src/vs/workbench/electron-browser/shell.ts b/src/vs/workbench/electron-browser/shell.ts index 1f2ac72109a..78796b5c45f 100644 --- a/src/vs/workbench/electron-browser/shell.ts +++ b/src/vs/workbench/electron-browser/shell.ts @@ -42,7 +42,7 @@ import { ExtensionService } from 'vs/workbench/services/extensions/electron-brow import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { InstantiationService } from 'vs/platform/instantiation/node/instantiationService'; -import { ILifecycleService, LifecyclePhase, ShutdownReason, ShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; +import { ILifecycleService, LifecyclePhase, WillShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; import { IMarkerService } from 'vs/platform/markers/common/markers'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { ISearchService, ISearchHistoryService } from 'vs/platform/search/common/search'; @@ -122,8 +122,8 @@ export interface ICoreServices { */ export class WorkbenchShell extends Disposable { - private readonly _onShutdown = this._register(new Emitter()); - get onShutdown(): Event { return this._onShutdown.event; } + private readonly _onWillShutdown = this._register(new Emitter()); + get onWillShutdown(): Event { return this._onWillShutdown.event; } private storageService: DelegatingStorageService; private environmentService: IEnvironmentService; @@ -428,11 +428,8 @@ export class WorkbenchShell extends Disposable { serviceCollection.set(IDialogService, instantiationService.createInstance(DialogService)); const lifecycleService = instantiationService.createInstance(LifecycleService); - this._register(lifecycleService.onShutdown(event => { - this._onShutdown.fire(event); - - this.dispose(event.reason); - })); + this._register(lifecycleService.onWillShutdown(event => this._onWillShutdown.fire(event))); + this._register(lifecycleService.onShutdown(() => this.dispose())); serviceCollection.set(ILifecycleService, lifecycleService); this.lifecycleService = lifecycleService; @@ -585,12 +582,12 @@ export class WorkbenchShell extends Disposable { this.workbench.layout(); } - dispose(reason = ShutdownReason.QUIT): void { + dispose(): void { super.dispose(); // Dispose Workbench if (this.workbench) { - this.workbench.dispose(reason); + this.workbench.dispose(); } this.mainProcessClient.dispose(); diff --git a/src/vs/workbench/electron-browser/workbench.ts b/src/vs/workbench/electron-browser/workbench.ts index 75a43f0999d..0aaba8ded7b 100644 --- a/src/vs/workbench/electron-browser/workbench.ts +++ b/src/vs/workbench/electron-browser/workbench.ts @@ -477,7 +477,7 @@ export class Workbench extends Disposable implements IPartService { private registerListeners(): void { // Lifecycle - this._register(this.lifecycleService.onWillShutdown(e => this.saveState(e.reason))); + this._register(this.lifecycleService.onBeforeShutdown(e => this.saveState(e.reason))); // Listen to visible editor changes this._register(this.editorService.onDidVisibleEditorsChange(() => this.onDidVisibleEditorsChange())); @@ -1142,7 +1142,7 @@ export class Workbench extends Disposable implements IPartService { } } - dispose(reason = ShutdownReason.QUIT): void { + dispose(): void { super.dispose(); this.workbenchShutdown = true; diff --git a/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts index e2b77e20e40..3a7a8d9b9f0 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts @@ -529,7 +529,7 @@ class TaskService extends Disposable implements ITaskService { this.updateWorkspaceTasks(); })); this._taskRunningState = TASK_RUNNING_STATE.bindTo(contextKeyService); - this._register(lifecycleService.onWillShutdown(event => event.veto(this.beforeShutdown()))); + this._register(lifecycleService.onBeforeShutdown(event => event.veto(this.beforeShutdown()))); this._register(storageService.onWillSaveState(() => this.saveState())); this._onDidStateChange = this._register(new Emitter()); this.registerCommands(); diff --git a/src/vs/workbench/parts/terminal/common/terminalService.ts b/src/vs/workbench/parts/terminal/common/terminalService.ts index 88d47af4759..729c7d30b56 100644 --- a/src/vs/workbench/parts/terminal/common/terminalService.ts +++ b/src/vs/workbench/parts/terminal/common/terminalService.ts @@ -63,7 +63,7 @@ export abstract class TerminalService implements ITerminalService { this._activeTabIndex = 0; this._isShuttingDown = false; this._findState = new FindReplaceState(); - lifecycleService.onWillShutdown(event => event.veto(this._onWillShutdown())); + lifecycleService.onBeforeShutdown(event => event.veto(this._onBeforeShutdown())); lifecycleService.onShutdown(() => this._onShutdown()); this._terminalFocusContextKey = KEYBINDING_CONTEXT_TERMINAL_FOCUS.bindTo(this._contextKeyService); this._findWidgetVisible = KEYBINDING_CONTEXT_TERMINAL_FIND_WIDGET_VISIBLE.bindTo(this._contextKeyService); @@ -92,7 +92,7 @@ export abstract class TerminalService implements ITerminalService { public abstract setContainers(panelContainer: HTMLElement, terminalContainer: HTMLElement): void; public abstract requestExtHostProcess(proxy: ITerminalProcessExtHostProxy, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI, cols: number, rows: number): void; - private _onWillShutdown(): boolean | PromiseLike { + private _onBeforeShutdown(): boolean | PromiseLike { if (this.terminalInstances.length === 0) { // No terminal instances, don't veto return false; diff --git a/src/vs/workbench/services/extensions/electron-browser/extensionHost.ts b/src/vs/workbench/services/extensions/electron-browser/extensionHost.ts index 28a41e9029c..1b955e1bd16 100644 --- a/src/vs/workbench/services/extensions/electron-browser/extensionHost.ts +++ b/src/vs/workbench/services/extensions/electron-browser/extensionHost.ts @@ -124,7 +124,7 @@ export class ExtensionHostProcessWorker implements IExtensionHostStarter { this._toDispose = []; this._toDispose.push(this._onCrashed); - this._toDispose.push(this._lifecycleService.onWillShutdown((e) => this._onWillShutdown(e))); + this._toDispose.push(this._lifecycleService.onWillShutdown(e => this._onWillShutdown(e))); this._toDispose.push(this._lifecycleService.onShutdown(reason => this.terminate())); this._toDispose.push(this._broadcastService.onBroadcast(b => this._onBroadcast(b))); @@ -563,7 +563,7 @@ export class ExtensionHostProcessWorker implements IExtensionHostStarter { } }); - event.veto(timeout(100 /* wait a bit for IPC to get delivered */).then(() => false)); + event.join(timeout(100 /* wait a bit for IPC to get delivered */)); } } } diff --git a/src/vs/workbench/services/textfile/common/textFileService.ts b/src/vs/workbench/services/textfile/common/textFileService.ts index 766d4e482b3..e34dac4fc56 100644 --- a/src/vs/workbench/services/textfile/common/textFileService.ts +++ b/src/vs/workbench/services/textfile/common/textFileService.ts @@ -106,7 +106,7 @@ export abstract class TextFileService extends Disposable implements ITextFileSer private registerListeners(): void { // Lifecycle - this.lifecycleService.onWillShutdown(event => event.veto(this.beforeShutdown(event.reason))); + this.lifecycleService.onBeforeShutdown(event => event.veto(this.beforeShutdown(event.reason))); this.lifecycleService.onShutdown(this.dispose, this); // Files configuration changes diff --git a/src/vs/workbench/services/textfile/test/textFileService.test.ts b/src/vs/workbench/services/textfile/test/textFileService.test.ts index fdf826a714c..f50f71ca6a4 100644 --- a/src/vs/workbench/services/textfile/test/textFileService.test.ts +++ b/src/vs/workbench/services/textfile/test/textFileService.test.ts @@ -7,7 +7,7 @@ import * as sinon from 'sinon'; import * as platform from 'vs/base/common/platform'; import { URI } from 'vs/base/common/uri'; import { TPromise } from 'vs/base/common/winjs.base'; -import { ILifecycleService, WillShutdownEvent, ShutdownReason } from 'vs/platform/lifecycle/common/lifecycle'; +import { ILifecycleService, BeforeShutdownEvent, ShutdownReason } from 'vs/platform/lifecycle/common/lifecycle'; import { workbenchInstantiationService, TestLifecycleService, TestTextFileService, TestWindowsService, TestContextService, TestFileService } from 'vs/workbench/test/workbenchTestServices'; import { toResource } from 'vs/base/test/common/utils'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -37,7 +37,7 @@ class ServiceAccessor { } } -class ShutdownEventImpl implements WillShutdownEvent { +class BeforeShutdownEventImpl implements BeforeShutdownEvent { public value: boolean | TPromise; public reason = ShutdownReason.CLOSE; @@ -69,7 +69,7 @@ suite('Files - TextFileService', () => { model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8'); (accessor.textFileService.models).add(model.getResource(), model); - const event = new ShutdownEventImpl(); + const event = new BeforeShutdownEventImpl(); accessor.lifecycleService.fireWillShutdown(event); const veto = event.value; @@ -94,7 +94,7 @@ suite('Files - TextFileService', () => { assert.equal(service.getDirty().length, 1); - const event = new ShutdownEventImpl(); + const event = new BeforeShutdownEventImpl(); accessor.lifecycleService.fireWillShutdown(event); assert.ok(event.value); @@ -114,7 +114,7 @@ suite('Files - TextFileService', () => { assert.equal(service.getDirty().length, 1); - const event = new ShutdownEventImpl(); + const event = new BeforeShutdownEventImpl(); accessor.lifecycleService.fireWillShutdown(event); const veto = event.value; @@ -145,7 +145,7 @@ suite('Files - TextFileService', () => { assert.equal(service.getDirty().length, 1); - const event = new ShutdownEventImpl(); + const event = new BeforeShutdownEventImpl(); accessor.lifecycleService.fireWillShutdown(event); return (>event.value).then(veto => { @@ -450,7 +450,7 @@ suite('Files - TextFileService', () => { assert.equal(service.getDirty().length, 1); - const event = new ShutdownEventImpl(); + const event = new BeforeShutdownEventImpl(); event.reason = shutdownReason; accessor.lifecycleService.fireWillShutdown(event); diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index 4adc3d609c8..9fd2c82eb13 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -24,7 +24,7 @@ import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { IEditorOptions, IResourceInput } from 'vs/platform/editor/common/editor'; import { IUntitledEditorService, UntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { IWorkspaceContextService, IWorkspace as IWorkbenchWorkspace, WorkbenchState, IWorkspaceFolder, IWorkspaceFoldersChangeEvent, Workspace } from 'vs/platform/workspace/common/workspace'; -import { ILifecycleService, WillShutdownEvent, ShutdownReason, StartupKind, LifecyclePhase, ShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; +import { ILifecycleService, BeforeShutdownEvent, ShutdownReason, StartupKind, LifecyclePhase, WillShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { TextFileService } from 'vs/workbench/services/textfile/common/textFileService'; import { FileOperationEvent, IFileService, IResolveContentOptions, FileOperationError, IFileStat, IResolveFileResult, FileChangesEvent, IResolveFileOptions, IContent, IUpdateContentOptions, IStreamContent, ICreateFileOptions, ITextSnapshot, IResourceEncodings } from 'vs/platform/files/common/files'; @@ -1144,29 +1144,34 @@ export class TestLifecycleService implements ILifecycleService { public phase: LifecyclePhase; public startupKind: StartupKind; + private _onBeforeShutdown = new Emitter(); private _onWillShutdown = new Emitter(); - private _onShutdown = new Emitter(); + private _onShutdown = new Emitter(); when(): TPromise { return TPromise.as(void 0); } public fireShutdown(reason = ShutdownReason.QUIT): void { - this._onShutdown.fire({ + this._onWillShutdown.fire({ join: () => { }, reason }); } - public fireWillShutdown(event: WillShutdownEvent): void { - this._onWillShutdown.fire(event); + public fireWillShutdown(event: BeforeShutdownEvent): void { + this._onBeforeShutdown.fire(event); + } + + public get onBeforeShutdown(): Event { + return this._onBeforeShutdown.event; } public get onWillShutdown(): Event { return this._onWillShutdown.event; } - public get onShutdown(): Event { + public get onShutdown(): Event { return this._onShutdown.event; } } -- GitLab