From d894008ecc33feeac43bfb70f5ee3acd34dcdc3b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 10 May 2016 15:20:07 +0200 Subject: [PATCH] dispose telemetry service and appenders on shutdown --- src/vs/base/common/lifecycle.ts | 12 ++++++++-- .../telemetry/browser/telemetryService.ts | 3 --- .../test/node/telemetryService.test.ts | 2 +- src/vs/workbench/electron-browser/shell.ts | 24 +++++++++++-------- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/vs/base/common/lifecycle.ts b/src/vs/base/common/lifecycle.ts index 74042437db5..12268356fde 100644 --- a/src/vs/base/common/lifecycle.ts +++ b/src/vs/base/common/lifecycle.ts @@ -83,7 +83,15 @@ export abstract class Disposable implements IDisposable { export class Disposables extends Disposable { - public add(disposable: IDisposable): void { - this._register(disposable); + public add(e: T): T; + public add(...elements: IDisposable[]): void; + public add(arg: T | T[]): T { + if (!Array.isArray(arg)) { + return this._register(arg); + } else { + for (let element of arg) { + return this._register(element); + } + } } } diff --git a/src/vs/platform/telemetry/browser/telemetryService.ts b/src/vs/platform/telemetry/browser/telemetryService.ts index 16b52fb6ff4..af92593b8cb 100644 --- a/src/vs/platform/telemetry/browser/telemetryService.ts +++ b/src/vs/platform/telemetry/browser/telemetryService.ts @@ -137,9 +137,6 @@ export class TelemetryService implements ITelemetryService { public dispose(): void { this._disposables = dispose(this._disposables); - for (let appender of this._configuration.appender) { - appender.dispose(); - } } public timedPublicLog(name: string, data?: any): ITimerEvent { diff --git a/src/vs/platform/telemetry/test/node/telemetryService.test.ts b/src/vs/platform/telemetry/test/node/telemetryService.test.ts index a78b2b43f6d..7613aaeb240 100644 --- a/src/vs/platform/telemetry/test/node/telemetryService.test.ts +++ b/src/vs/platform/telemetry/test/node/telemetryService.test.ts @@ -91,7 +91,7 @@ suite('TelemetryService', () => { assert.equal(testAppender.getEventsCount(), 1); service.dispose(); - assert.equal(testAppender.isDisposed, true); + assert.equal(!testAppender.isDisposed, true); }); })); diff --git a/src/vs/workbench/electron-browser/shell.ts b/src/vs/workbench/electron-browser/shell.ts index f70ef0a6777..67726ad8562 100644 --- a/src/vs/workbench/electron-browser/shell.ts +++ b/src/vs/workbench/electron-browser/shell.ts @@ -13,7 +13,7 @@ import * as platform from 'vs/base/common/platform'; import {Dimension, Builder, $} from 'vs/base/browser/builder'; import dom = require('vs/base/browser/dom'); import aria = require('vs/base/browser/ui/aria/aria'); -import {dispose, IDisposable} from 'vs/base/common/lifecycle'; +import {dispose, IDisposable, Disposables} from 'vs/base/common/lifecycle'; import errors = require('vs/base/common/errors'); import {ContextViewService} from 'vs/platform/contextview/browser/contextViewService'; import timer = require('vs/base/common/timer'); @@ -198,6 +198,7 @@ export class WorkbenchShell { private initServiceCollection(): [InstantiationService, ServiceCollection] { let serviceCollection = new ServiceCollection(); let instantiationService = new InstantiationService(serviceCollection, true); + let disposables = new Disposables(); this.windowService = new WindowService(); @@ -205,39 +206,41 @@ export class WorkbenchShell { this.storageService = new Storage(this.contextService, window.localStorage, disableWorkspaceStorage ? inMemoryLocalStorageInstance : window.localStorage); if (this.configuration.env.isBuilt && !this.configuration.env.extensionDevelopmentPath && !!this.configuration.env.enableTelemetry) { - this.telemetryService = new TelemetryService({ + const config = { appender: createAppender(this.configuration.env), commonProperties: resolveCommonProperties(this.storageService, this.contextService), piiPaths: [this.configuration.env.appRoot, this.configuration.env.userExtensionsHome] - }, this.configurationService); + }; + let telemetryService = new TelemetryService(config, this.configurationService); + this.telemetryService = telemetryService; + disposables.add(telemetryService, ...config.appender); } else { this.telemetryService = NullTelemetryService; } this.messageService = new MessageService(this.contextService, this.windowService, this.telemetryService); - let fileService = new FileService( + let fileService = disposables.add(new FileService( this.configurationService, this.eventService, this.contextService, this.messageService - ); + )); + let lifecycleService = new LifecycleService(this.messageService, this.windowService); - this.toUnbind.push(lifecycleService.onShutdown(() => fileService.dispose())); - + this.toUnbind.push(lifecycleService.onShutdown(() => disposables.dispose())); this.threadService = new MainThreadService(this.contextService, this.messageService, this.windowService, lifecycleService); let extensionService = new MainProcessExtensionService(this.contextService, this.threadService, this.messageService); this.contextViewService = new ContextViewService(this.container, this.telemetryService, this.messageService); - let requestService = new RequestService( + let requestService = disposables.add(new RequestService( this.contextService, this.configurationService, this.telemetryService - ); - this.toUnbind.push(lifecycleService.onShutdown(() => requestService.dispose())); + )); let markerService = new MainProcessMarkerService(this.threadService); @@ -248,6 +251,7 @@ export class WorkbenchShell { let untitledEditorService = instantiationService.createInstance(UntitledEditorService); this.themeService = new ThemeService(extensionService, this.windowService, this.storageService); + serviceCollection.set(ITelemetryService, this.telemetryService); serviceCollection.set(IEventService, this.eventService); serviceCollection.set(IRequestService, requestService);