From d8cacdc1fd20fa0e066e62f5271ddc7bbfda51c1 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Tue, 5 Dec 2017 17:53:50 +0100 Subject: [PATCH] fix spdlog service not closing file handle on reload related to #39659 --- package.json | 4 ++-- src/typings/spdlog.d.ts | 1 + src/vs/code/electron-browser/sharedProcessMain.ts | 1 + src/vs/code/electron-main/main.ts | 2 ++ src/vs/platform/log/common/log.ts | 14 +++++++++++++- src/vs/platform/log/node/spdlogService.ts | 5 ++--- src/vs/workbench/electron-browser/shell.ts | 2 ++ src/vs/workbench/node/extensionHostMain.ts | 6 ++++++ yarn.lock | 6 +++--- 9 files changed, 32 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 35b362ce1a4..9a31c00c44c 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "node-pty": "0.7.3", "nsfw": "1.0.16", "semver": "4.3.6", - "spdlog": "0.2.1", + "spdlog": "0.3.0", "v8-inspect-profiler": "^0.0.6", "vscode-chokidar": "1.6.2", "vscode-debugprotocol": "1.25.0", @@ -127,4 +127,4 @@ "windows-mutex": "^0.2.0", "windows-process-tree": "0.1.6" } -} \ No newline at end of file +} diff --git a/src/typings/spdlog.d.ts b/src/typings/spdlog.d.ts index e23c7e4b493..ef93a9cf800 100644 --- a/src/typings/spdlog.d.ts +++ b/src/typings/spdlog.d.ts @@ -29,5 +29,6 @@ declare module 'spdlog' { critical(message: string); setLevel(level: number); flush(): void; + drop(): void; } } \ No newline at end of file diff --git a/src/vs/code/electron-browser/sharedProcessMain.ts b/src/vs/code/electron-browser/sharedProcessMain.ts index e6ff2f71042..934007fcceb 100644 --- a/src/vs/code/electron-browser/sharedProcessMain.ts +++ b/src/vs/code/electron-browser/sharedProcessMain.ts @@ -80,6 +80,7 @@ function main(server: Server, initData: ISharedProcessInitData, configuration: I const environmentService = new EnvironmentService(initData.args, process.execPath); const logService = new SpdLogService('sharedprocess', environmentService); + process.once('exit', () => logService.dispose()); registerGlobalLogService(logService); logService.info('main', JSON.stringify(configuration)); diff --git a/src/vs/code/electron-main/main.ts b/src/vs/code/electron-main/main.ts index a388de788ac..03e71971d57 100644 --- a/src/vs/code/electron-main/main.ts +++ b/src/vs/code/electron-main/main.ts @@ -51,7 +51,9 @@ function createServices(args: ParsedArgs): IInstantiationService { const spdlogService = new SpdLogService('main', environmentService); const legacyLogService = new LegacyLogMainService(environmentService); const logService = new MultiplexLogService([legacyLogService, spdlogService]); + registerGlobalLogService(logService); + process.once('exit', () => logService.dispose()); // Eventually cleanup setTimeout(() => spdlogService.cleanup().then(null, err => console.error(err)), 10000); diff --git a/src/vs/platform/log/common/log.ts b/src/vs/platform/log/common/log.ts index bc7cc969f3f..237967df375 100644 --- a/src/vs/platform/log/common/log.ts +++ b/src/vs/platform/log/common/log.ts @@ -8,6 +8,7 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { createDecorator as createServiceDecorator } from 'vs/platform/instantiation/common/instantiation'; import { createDecorator } from 'vs/base/common/decorators'; +import { IDisposable } from 'vs/base/common/lifecycle'; export const ILogService = createServiceDecorator('logService'); @@ -21,7 +22,7 @@ export enum LogLevel { Off } -export interface ILogService { +export interface ILogService extends IDisposable { _serviceBrand: any; setLevel(level: LogLevel): void; @@ -81,6 +82,10 @@ export class LegacyLogMainService implements ILogService { console.error(`\x1b[90m[main ${new Date().toLocaleTimeString()}]\x1b[0m`, message, ...args); } } + + dispose(): void { + // noop + } } export class MultiplexLogService implements ILogService { @@ -129,6 +134,12 @@ export class MultiplexLogService implements ILogService { logService.critical(message, ...args); } } + + dispose(): void { + for (const logService of this.logServices) { + logService.dispose(); + } + } } export class NoopLogService implements ILogService { @@ -140,6 +151,7 @@ export class NoopLogService implements ILogService { warn(message: string, ...args: any[]): void { } error(message: string | Error, ...args: any[]): void { } critical(message: string | Error, ...args: any[]): void { } + dispose(): void { } } let globalLogService: ILogService = new NoopLogService(); diff --git a/src/vs/platform/log/node/spdlogService.ts b/src/vs/platform/log/node/spdlogService.ts index d554b355a35..3ac203cb01a 100644 --- a/src/vs/platform/log/node/spdlogService.ts +++ b/src/vs/platform/log/node/spdlogService.ts @@ -10,7 +10,6 @@ import { ILogService, LogLevel } from 'vs/platform/log/common/log'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { RotatingLogger, setAsyncMode } from 'spdlog'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -import { fromNodeEventEmitter } from 'vs/base/common/event'; import { TPromise } from 'vs/base/common/winjs.base'; import { readdir, rimraf } from 'vs/base/node/pfs'; @@ -30,8 +29,6 @@ export class SpdLogService implements ILogService { const logfilePath = path.join(environmentService.logsPath, `${processName}.log`); this.logger = new RotatingLogger(processName, logfilePath, 1024 * 1024 * 5, 6); this.setLevel(environmentService.logLevel); - - fromNodeEventEmitter(process, 'exit')(() => this.logger.flush(), null, this.disposables); } /** @@ -79,6 +76,8 @@ export class SpdLogService implements ILogService { } dispose(): void { + this.logger.flush(); + this.logger.drop(); this.disposables = dispose(this.disposables); } diff --git a/src/vs/workbench/electron-browser/shell.ts b/src/vs/workbench/electron-browser/shell.ts index 88c73dfc813..8cfe5fdfc67 100644 --- a/src/vs/workbench/electron-browser/shell.ts +++ b/src/vs/workbench/electron-browser/shell.ts @@ -287,6 +287,8 @@ export class WorkbenchShell { serviceCollection.set(IConfigurationService, this.configurationService); serviceCollection.set(IEnvironmentService, this.environmentService); serviceCollection.set(ILogService, this.logService); + disposables.push(this.logService); + serviceCollection.set(ITimerService, this.timerService); serviceCollection.set(IStorageService, this.storageService); this.mainProcessServices.forEach((serviceIdentifier, serviceInstance) => { diff --git a/src/vs/workbench/node/extensionHostMain.ts b/src/vs/workbench/node/extensionHostMain.ts index ecda2c17ed5..e4be4be2805 100644 --- a/src/vs/workbench/node/extensionHostMain.ts +++ b/src/vs/workbench/node/extensionHostMain.ts @@ -25,6 +25,7 @@ import { ExtensionActivatedByEvent } from 'vs/workbench/api/node/extHostExtensio import { EnvironmentService } from 'vs/platform/environment/node/environmentService'; import { SpdLogService } from 'vs/platform/log/node/spdlogService'; import { registerGlobalLogService } from 'vs/platform/log/common/log'; +import { IDisposable, dispose } from 'vs/base/common/lifecycle'; // const nativeExit = process.exit.bind(process); function patchProcess(allowExit: boolean) { @@ -76,6 +77,7 @@ export class ExtensionHostMain { private _environment: IEnvironment; private _extensionService: ExtHostExtensionService; private _extHostConfiguration: ExtHostConfiguration; + private disposables: IDisposable[] = []; constructor(rpcProtocol: RPCProtocol, initData: IInitData) { this._environment = initData.environment; @@ -89,7 +91,9 @@ export class ExtensionHostMain { const extHostWorkspace = new ExtHostWorkspace(threadService, initData.workspace); const environmentService = new EnvironmentService(initData.args, initData.execPath); const logService = new SpdLogService(`exthost${initData.windowId}`, environmentService); + registerGlobalLogService(logService); + this.disposables.push(logService); logService.info('main {0}', initData); @@ -146,6 +150,8 @@ export class ExtensionHostMain { } this._isTerminating = true; + this.disposables = dispose(this.disposables); + errors.setUnexpectedErrorHandler((err) => { // TODO: write to log once we have one }); diff --git a/yarn.lock b/yarn.lock index a378779ecc5..72f6182e162 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5038,9 +5038,9 @@ sparkles@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/sparkles/-/sparkles-1.0.0.tgz#1acbbfb592436d10bbe8f785b7cc6f82815012c3" -spdlog@0.2.1: - version "0.2.1" - resolved "https://registry.yarnpkg.com/spdlog/-/spdlog-0.2.1.tgz#1a9de952ccffe9b9227dd20306aca7e428621fa1" +spdlog@0.3.0: + version "0.3.0" + resolved "https://registry.yarnpkg.com/spdlog/-/spdlog-0.3.0.tgz#8fc0dfa407ccb97854bd026beb91267adeed5cf3" dependencies: bindings "^1.3.0" mkdirp "^0.5.1"