debt - better separation of extHostMain and extHostProcess

src/vs/workbench/api/common/extHostLogService.ts

@@ -10,21 +10,16 @@ import { ExtensionHostLogFileName } from 'vs/workbench/services/extensions/commo
 import { URI } from 'vs/base/common/uri';
 import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
 
-export interface ILogServiceCreateFn {
-	(): ILogService;
-}
-
 export class ExtHostLogService extends DelegatedLogService implements ILogService, ExtHostLogServiceShape {
 
 	private _logsPath: string;
 	readonly logFile: URI;
 
 	constructor(
-		logLevel: LogLevel,
+		delegate: ILogService,
 		logsPath: string,
-		factory: ILogServiceCreateFn
 	) {
-		super(factory());
+		super(delegate);
 		this._logsPath = logsPath;
 		this.logFile = URI.file(join(logsPath, `${ExtensionHostLogFileName}.log`));
 	}

src/vs/workbench/services/extensions/node/extensionHostMain.ts

@@ -10,7 +10,7 @@ import { Counter } from 'vs/base/common/numbers';
 import { URI, setUriThrowOnMissingScheme } from 'vs/base/common/uri';
 import { IURITransformer } from 'vs/base/common/uriIpc';
 import { IMessagePassingProtocol } from 'vs/base/parts/ipc/common/ipc';
-import { IEnvironment, IInitData, MainContext, MainThreadConsoleShape } from 'vs/workbench/api/common/extHost.protocol';
+import { IInitData, MainContext, MainThreadConsoleShape } from 'vs/workbench/api/common/extHost.protocol';
 import { ExtHostConfiguration } from 'vs/workbench/api/common/extHostConfiguration';
 import { ExtHostExtensionService } from 'vs/workbench/api/node/extHostExtensionService';
 import { ExtHostLogService } from 'vs/workbench/api/common/extHostLogService';
@@ -18,61 +18,48 @@ import { ExtHostWorkspace } from 'vs/workbench/api/common/extHostWorkspace';
 import { RPCProtocol } from 'vs/workbench/services/extensions/common/rpcProtocol';
 import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
 import { withNullAsUndefined } from 'vs/base/common/types';
-import { ExtensionHostLogFileName } from 'vs/workbench/services/extensions/common/extensions';
-import { createSpdLogService } from 'vs/platform/log/node/spdlogService';
+import { ILogService } from 'vs/platform/log/common/log';
 
 // we don't (yet) throw when extensions parse
 // uris that have no scheme
 setUriThrowOnMissingScheme(false);
 
-const nativeExit = process.exit.bind(process);
-function patchProcess(allowExit: boolean) {
-	process.exit = function (code?: number) {
-		if (allowExit) {
-			exit(code);
-		} else {
-			const err = new Error('An extension called process.exit() and this was prevented.');
-			console.warn(err.stack);
-		}
-	} as (code?: number) => never;
+export interface IExitFn {
+	(code?: number): any;
+}
 
-	process.crash = function () {
-		const err = new Error('An extension called process.crash() and this was prevented.');
-		console.warn(err.stack);
-	};
+export interface IConsolePatchFn {
+	(mainThreadConsole: MainThreadConsoleShape): any;
 }
 
-export function exit(code?: number) {
-	nativeExit(code);
+export interface ILogServiceFn {
+	(initData: IInitData): ILogService;
 }
 
 export class ExtensionHostMain {
 
-
 	private _isTerminating: boolean;
-	private readonly _environment: IEnvironment;
+	private readonly _exitFn: IExitFn;
 	private readonly _extensionService: ExtHostExtensionService;
 	private readonly _extHostLogService: ExtHostLogService;
 	private disposables: IDisposable[] = [];
 
 	private _searchRequestIdProvider: Counter;
 
-	constructor(protocol: IMessagePassingProtocol, initData: IInitData) {
+	constructor(protocol: IMessagePassingProtocol, initData: IInitData, exitFn: IExitFn, consolePatchFn: IConsolePatchFn, logServiceFn: ILogServiceFn) {
 		this._isTerminating = false;
+		this._exitFn = exitFn;
 		const uriTransformer: IURITransformer | null = null;
 		const rpcProtocol = new RPCProtocol(protocol, null, uriTransformer);
 
 		// ensure URIs are transformed and revived
 		initData = this.transform(initData, rpcProtocol);
-		this._environment = initData.environment;
-
-		const allowExit = !!this._environment.extensionTestsLocationURI; // to support other test frameworks like Jasmin that use process.exit (https://github.com/Microsoft/vscode/issues/37708)
-		patchProcess(allowExit);
 
-		this._patchPatchedConsole(rpcProtocol.getProxy(MainContext.MainThreadConsole));
+		// allow to patch console
+		consolePatchFn(rpcProtocol.getProxy(MainContext.MainThreadConsole));
 
 		// services
-		this._extHostLogService = new ExtHostLogService(initData.logLevel, initData.logsLocation.fsPath, () => createSpdLogService(ExtensionHostLogFileName, initData.logLevel, initData.logsLocation.fsPath));
+		this._extHostLogService = new ExtHostLogService(logServiceFn(initData), initData.logsLocation.fsPath);
 		this.disposables.push(this._extHostLogService);
 
 		this._searchRequestIdProvider = new Counter();
@@ -82,7 +69,7 @@ export class ExtensionHostMain {
 		this._extHostLogService.trace('initData', initData);
 
 		const extHostConfiguraiton = new ExtHostConfiguration(rpcProtocol.getProxy(MainContext.MainThreadConfiguration), extHostWorkspace);
-		this._extensionService = new ExtHostExtensionService(nativeExit, initData, rpcProtocol, extHostWorkspace, extHostConfiguraiton, this._environment, this._extHostLogService);
+		this._extensionService = new ExtHostExtensionService(exitFn, initData, rpcProtocol, extHostWorkspace, extHostConfiguraiton, initData.environment, this._extHostLogService);
 
 		// error forwarding and stack trace scanning
 		Error.stackTraceLimit = 100; // increase number of stack frames (from 10, https://github.com/v8/v8/wiki/Stack-Trace-API)
@@ -118,18 +105,6 @@ export class ExtensionHostMain {
 		});
 	}
 
-	private _patchPatchedConsole(mainThreadConsole: MainThreadConsoleShape): void {
-		// The console is already patched to use `process.send()`
-		const nativeProcessSend = process.send!;
-		process.send = (...args: any[]) => {
-			if (args.length === 0 || !args[0] || args[0].type !== '__$console') {
-				return nativeProcessSend.apply(process, args);
-			}
-
-			mainThreadConsole.$logExtensionHostMessage(args[0]);
-		};
-	}
-
 	terminate(): void {
 		if (this._isTerminating) {
 			// we are already shutting down...
@@ -147,7 +122,7 @@ export class ExtensionHostMain {
 
 		// Give extensions 1 second to wrap up any async dispose, then exit in at most 4 seconds
 		setTimeout(() => {
-			Promise.race([timeout(4000), extensionsDeactivated]).then(() => exit(), () => exit());
+			Promise.race([timeout(4000), extensionsDeactivated]).finally(() => this._exitFn());
 		}, 1000);
 	}
 

src/vs/workbench/services/extensions/node/extensionHostProcess.ts

@@ -11,10 +11,12 @@ import { IMessagePassingProtocol } from 'vs/base/parts/ipc/common/ipc';
 import { PersistentProtocol, ProtocolConstants } from 'vs/base/parts/ipc/common/ipc.net';
 import { NodeSocket } from 'vs/base/parts/ipc/node/ipc.net';
 import product from 'vs/platform/product/node/product';
-import { IInitData } from 'vs/workbench/api/common/extHost.protocol';
+import { IInitData, MainThreadConsoleShape } from 'vs/workbench/api/common/extHost.protocol';
 import { MessageType, createMessageOfType, isMessageOfType, IExtHostSocketMessage, IExtHostReadyMessage } from 'vs/workbench/services/extensions/common/extensionHostProtocol';
-import { exit, ExtensionHostMain } from 'vs/workbench/services/extensions/node/extensionHostMain';
+import { ExtensionHostMain, IExitFn, ILogServiceFn } from 'vs/workbench/services/extensions/node/extensionHostMain';
 import { VSBuffer } from 'vs/base/common/buffer';
+import { createSpdLogService } from 'vs/platform/log/node/spdlogService';
+import { ExtensionHostLogFileName } from 'vs/workbench/services/extensions/common/extensions';
 
 // With Electron 2.x and node.js 8.x the "natives" module
 // can cause a native crash (see https://github.com/nodejs/node/issues/19891 and
@@ -34,6 +36,39 @@ import { VSBuffer } from 'vs/base/common/buffer';
 	};
 })();
 
+// custom process.exit logic...
+const nativeExit: IExitFn = process.exit.bind(process);
+function patchProcess(allowExit: boolean) {
+	process.exit = function (code?: number) {
+		if (allowExit) {
+			nativeExit(code);
+		} else {
+			const err = new Error('An extension called process.exit() and this was prevented.');
+			console.warn(err.stack);
+		}
+	} as (code?: number) => never;
+
+	process.crash = function () {
+		const err = new Error('An extension called process.crash() and this was prevented.');
+		console.warn(err.stack);
+	};
+}
+
+// use IPC messages to forward console-calls
+function patchPatchedConsole(mainThreadConsole: MainThreadConsoleShape): void {
+	// The console is already patched to use `process.send()`
+	const nativeProcessSend = process.send!;
+	process.send = (...args: any[]) => {
+		if (args.length === 0 || !args[0] || args[0].type !== '__$console') {
+			return nativeProcessSend.apply(process, args);
+		}
+
+		mainThreadConsole.$logExtensionHostMessage(args[0]);
+	};
+}
+
+const createLogService: ILogServiceFn = initData => createSpdLogService(ExtensionHostLogFileName, initData.logLevel, initData.logsLocation.fsPath);
+
 interface IRendererConnection {
 	protocol: IMessagePassingProtocol;
 	initData: IInitData;
@@ -42,7 +77,7 @@ interface IRendererConnection {
 // This calls exit directly in case the initialization is not finished and we need to exit
 // Otherwise, if initialization completed we go to extensionHostMain.terminate()
 let onTerminate = function () {
-	exit();
+	nativeExit();
 };
 
 function _createExtHostProtocol(): Promise<IMessagePassingProtocol> {
@@ -149,7 +184,7 @@ function connectToRenderer(protocol: IMessagePassingProtocol): Promise<IRenderer
 			if (rendererCommit && myCommit) {
 				// Running in the built version where commits are defined
 				if (rendererCommit !== myCommit) {
-					exit(55);
+					nativeExit(55);
 				}
 			}
 
@@ -224,8 +259,13 @@ createExtHostProtocol().then(protocol => {
 	// connect to main side
 	return connectToRenderer(protocol);
 }).then(renderer => {
+	const { initData } = renderer;
 	// setup things
-	const extensionHostMain = new ExtensionHostMain(renderer.protocol, renderer.initData);
+	patchProcess(!!initData.environment.extensionTestsLocationURI); // to support other test frameworks like Jasmin that use process.exit (https://github.com/Microsoft/vscode/issues/37708)
+
+	const extensionHostMain = new ExtensionHostMain(renderer.protocol, initData, nativeExit, patchPatchedConsole, createLogService);
+
+	// rewrite onTerminate-function to be a proper shutdown
 	onTerminate = () => extensionHostMain.terminate();
 }).catch(err => console.error(err));