From 98473e0e88cecd52ba44551d512efe445b6af11e Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 9 Dec 2015 13:21:29 +0100 Subject: [PATCH] Call extension deactivate(), dispose subscriptions on extension host shutdown --- .../browser/standalone/simpleServices.ts | 4 +++ .../common/worker/editorWorkerServer.ts | 4 +++ .../editor/test/common/servicesTestUtils.ts | 4 +++ .../plugins/common/abstractPluginService.ts | 5 +-- .../plugins/common/nativePluginService.ts | 27 ++++++++++++++++ src/vs/platform/plugins/common/plugins.ts | 15 +++++++++ src/vs/workbench/node/pluginHostMain.ts | 31 ++++++++++++++++++- src/vs/workbench/node/pluginHostProcess.ts | 22 +++++++++++-- .../thread/electron-browser/threadService.ts | 4 ++- 9 files changed, 109 insertions(+), 7 deletions(-) diff --git a/src/vs/editor/browser/standalone/simpleServices.ts b/src/vs/editor/browser/standalone/simpleServices.ts index 033df7f943b..93c7202721f 100644 --- a/src/vs/editor/browser/standalone/simpleServices.ts +++ b/src/vs/editor/browser/standalone/simpleServices.ts @@ -272,4 +272,8 @@ export class SimplePluginService extends AbstractPluginService { console.log(msg); } } + + public deactivate(pluginId:string): void { + // nothing to do + } } diff --git a/src/vs/editor/common/worker/editorWorkerServer.ts b/src/vs/editor/common/worker/editorWorkerServer.ts index 0bcd42cb2ab..e054afee0df 100644 --- a/src/vs/editor/common/worker/editorWorkerServer.ts +++ b/src/vs/editor/common/worker/editorWorkerServer.ts @@ -73,6 +73,10 @@ class WorkerPluginService extends AbstractPluginService { } } + public deactivate(pluginId:string): void { + // nothing to do + } + } export class EditorWorkerServer { diff --git a/src/vs/editor/test/common/servicesTestUtils.ts b/src/vs/editor/test/common/servicesTestUtils.ts index 935277a6238..eea049ef7e9 100644 --- a/src/vs/editor/test/common/servicesTestUtils.ts +++ b/src/vs/editor/test/common/servicesTestUtils.ts @@ -123,6 +123,10 @@ class MockPluginService extends AbstractPluginService { console.log(msg); } } + + public deactivate(pluginId:string): void { + // nothing to do + } } class MockModelService extends ModelServiceImpl {} diff --git a/src/vs/platform/plugins/common/abstractPluginService.ts b/src/vs/platform/plugins/common/abstractPluginService.ts index 70f04a078b7..a5d85100765 100644 --- a/src/vs/platform/plugins/common/abstractPluginService.ts +++ b/src/vs/platform/plugins/common/abstractPluginService.ts @@ -21,8 +21,8 @@ export interface IPluginExports { } export interface IPluginModule { - activate(subscriptions: IDisposable[]): WinJS.TPromise; - deactivate(callback:(err:any, success:boolean)=>void): void; + activate(ctx: IPluginContext): WinJS.TPromise; + deactivate(): void; } export interface IPluginContext { @@ -83,6 +83,7 @@ export abstract class AbstractPluginService implements IPluginService { this.activatedPlugins = {}; } + public abstract deactivate(pluginId:string): void; protected abstract _showMessage(severity:Severity, message:string): void; protected showMessage(severity:Severity, source:string, message:string): void { diff --git a/src/vs/platform/plugins/common/nativePluginService.ts b/src/vs/platform/plugins/common/nativePluginService.ts index 3b4847b3620..b7b07ae4a48 100644 --- a/src/vs/platform/plugins/common/nativePluginService.ts +++ b/src/vs/platform/plugins/common/nativePluginService.ts @@ -15,6 +15,7 @@ import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry'; import {PluginHostStorage} from 'vs/platform/storage/common/remotable.storage'; import * as paths from 'vs/base/common/paths'; import {IWorkspaceContextService, IConfiguration} from 'vs/platform/workspace/common/workspace'; +import {disposeAll} from 'vs/base/common/lifecycle'; class PluginMemento implements IPluginMemento { @@ -168,6 +169,10 @@ export class MainProcessPluginService extends AbstractPluginService { return this._pluginsStatus; } + public deactivate(pluginId:string): void { + this._proxy.deactivate(pluginId); + } + // -- overwriting AbstractPluginService protected _actualActivatePlugin(pluginDescription: IPluginDescription): WinJS.TPromise { @@ -232,6 +237,28 @@ export class PluginHostPluginService extends AbstractPluginService { } } + public deactivate(pluginId:string): void { + let plugin = this.activatedPlugins[pluginId]; + if (!plugin) { + return; + } + + // call deactivate if available + try { + if (typeof plugin.module.deactivate === 'function') { + plugin.module.deactivate(); + } + } catch(err) { + // TODO: Do something with err if this is not the shutdown case + } + + // clean up subscriptions + try { + disposeAll(plugin.subscriptions) + } catch(err) { + // TODO: Do something with err if this is not the shutdown case + } + } // -- overwriting AbstractPluginService diff --git a/src/vs/platform/plugins/common/plugins.ts b/src/vs/platform/plugins/common/plugins.ts index 42335e24bd5..d4ae4848dfb 100644 --- a/src/vs/platform/plugins/common/plugins.ts +++ b/src/vs/platform/plugins/common/plugins.ts @@ -46,15 +46,30 @@ export interface IPluginStatus { export interface IPluginService { serviceId: ServiceIdentifier; + activateByEvent(activationEvent:string): TPromise; activateAndGet(pluginId:string): TPromise; activateAndGet(pluginId:string): TPromise; isActivated(pluginId:string): boolean; + + /** + * This method should be called only on shutdown! + * More work is needed for this to be called any time! + */ + deactivate(pluginId:string): void; + + /** + * To be used only by the platform once on startup. + */ registrationDone(errors?:IMessage[]): void; registerOneTimeActivationEventListener(activationEvent: string, listener: IActivationEventListener): void; get(pluginId:string): any; + + /** + * Block on this signal any interactions with extensions. + */ onReady(): TPromise; getPluginsStatus(): { [id: string]: IPluginStatus }; } diff --git a/src/vs/workbench/node/pluginHostMain.ts b/src/vs/workbench/node/pluginHostMain.ts index 76a74bb1a42..d8f2b947243 100644 --- a/src/vs/workbench/node/pluginHostMain.ts +++ b/src/vs/workbench/node/pluginHostMain.ts @@ -104,16 +104,45 @@ interface ITestRunner { export class PluginHostMain { + private _isTerminating: boolean; + constructor( @IWorkspaceContextService private contextService: IWorkspaceContextService, @IPluginService private pluginService: IPluginService, @IInstantiationService instantiationService: IInstantiationService - ) {} + ) { + this._isTerminating = false; + } public start(): TPromise { return this.readPlugins(); } + public terminate(): void { + if (this._isTerminating) { + // we are already shutting down... + return; + } + this._isTerminating = true; + + try { + let allExtensions = PluginsRegistry.getAllPluginDescriptions(); + let allExtensionsIds = allExtensions.map(ext => ext.id); + let activatedExtensions = allExtensionsIds.filter(id => this.pluginService.isActivated(id)); + + activatedExtensions.forEach((extensionId) => { + this.pluginService.deactivate(extensionId); + }); + } catch(err) { + // TODO: write to log once we have one + } + + // Give extensions 1 second to wrap up any async dispose, then exit + setTimeout(() => { + exit() + }, 1000); + } + private readPlugins(): TPromise { let collector = new PluginsMessageCollector(); let env = this.contextService.getConfiguration().env; diff --git a/src/vs/workbench/node/pluginHostProcess.ts b/src/vs/workbench/node/pluginHostProcess.ts index 03b9c9bd746..ee57a5c42e5 100644 --- a/src/vs/workbench/node/pluginHostProcess.ts +++ b/src/vs/workbench/node/pluginHostProcess.ts @@ -16,8 +16,14 @@ interface IRendererConnection { initData: IInitData; } +// This calls exit directly in case the initialization is not finished and we need to exit +// Otherwise, if initialization completed we go to pluginHostMain.terminate() +var onTerminate = function() { + exit(); +}; + function connectToRenderer(): TPromise { - return new TPromise((c, e) => { + return new TPromise((c, e) => { const stats: number[] = []; // Listen init data message @@ -28,7 +34,13 @@ function connectToRenderer(): TPromise { }); // Listen to all other messages - process.on('message', msg => remoteCom.handle(msg)); + process.on('message', (msg) => { + if (msg.type === '__$terminate') { + onTerminate(); + return; + } + remoteCom.handle(msg); + }); // Print a console message when rejection isn't handled. For details // see https://nodejs.org/api/process.html#process_event_unhandledrejection @@ -50,7 +62,7 @@ function connectToRenderer(): TPromise { try { process.kill(msg.parentPid, 0); // throws an exception if the main process doesn't exist anymore. } catch (e) { - exit(); + onTerminate(); } }, 5000); @@ -86,6 +98,10 @@ TPromise.join([connectToRenderer(), connectToSharedProcess()]) const instantiationService = createServices(renderer.remoteCom, renderer.initData, sharedProcessClient); const pluginHostMain = instantiationService.createInstance(PluginHostMain); + onTerminate = () => { + pluginHostMain.terminate(); + }; + pluginHostMain.start() .done(null, err => console.error(err)); }); \ No newline at end of file diff --git a/src/vs/workbench/services/thread/electron-browser/threadService.ts b/src/vs/workbench/services/thread/electron-browser/threadService.ts index baa8313712b..d585a6e8acf 100644 --- a/src/vs/workbench/services/thread/electron-browser/threadService.ts +++ b/src/vs/workbench/services/thread/electron-browser/threadService.ts @@ -305,7 +305,9 @@ class PluginHostProcessManager { this.terminating = true; if (this.pluginHostProcessHandle) { - this.pluginHostProcessHandle.kill(); + this.pluginHostProcessHandle.send({ + type: '__$terminate' + }); } } } \ No newline at end of file -- GitLab