sandbox - utility process cleanup

src/vs/code/electron-main/app.ts

@@ -1263,6 +1263,7 @@ export class CodeApplication extends Disposable {
 				type: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The type of shared process crash to understand the nature of the crash better.' };
 				reason: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'The reason of the shared process crash to understand the nature of the crash better.' };
 				code: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The exit code of the shared process crash to understand the nature of the crash better.' };
+				utilityprocess: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'If the shared process is using utility process or a hidden window.' };
 				visible: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Whether the shared process window was visible or not.' };
 				shuttingdown: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Whether the application is shutting down when the crash happens.' };
 				owner: 'bpasero';
@@ -1274,13 +1275,15 @@ export class CodeApplication extends Disposable {
 				reason: string | undefined;
 				code: number | undefined;
 				visible: boolean;
+				utilityprocess: string;
 				shuttingdown: boolean;
 			};
 			telemetryService.publicLog2<SharedProcessErrorEvent, SharedProcessErrorClassification>('sharedprocesserror', {
 				type,
 				reason: details?.reason,
 				code: details?.exitCode,
 				visible: sharedProcess.isVisible(),
+				utilityprocess: sharedProcess.usingUtilityProcess() ? '1' : '0', // TODO@bpasero remove this once sandbox is enabled by default
 				shuttingdown: willShutdown
 			});
 		}));

src/vs/platform/environment/common/argv.ts

@@ -67,6 +67,8 @@ export interface NativeParsedArgs {
 	'inspect-brk-search'?: string;
 	'inspect-ptyhost'?: string;
 	'inspect-brk-ptyhost'?: string;
+	'inspect-sharedprocess'?: string;
+	'inspect-brk-sharedprocess'?: string;
 	'disable-extensions'?: boolean;
 	'disable-extension'?: string[]; // undefined or array of 1 or more
 	'list-extensions'?: boolean;

src/vs/platform/environment/common/environmentService.ts

@@ -11,7 +11,7 @@ import { env } from 'vs/base/common/process';
 import { joinPath } from 'vs/base/common/resources';
 import { URI } from 'vs/base/common/uri';
 import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
-import { ExtensionKind, IDebugParams, IExtensionHostDebugParams, INativeEnvironmentService } from 'vs/platform/environment/common/environment';
+import { ExtensionKind, IExtensionHostDebugParams, INativeEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IProductService } from 'vs/platform/product/common/productService';
 
 export const EXTENSION_IDENTIFIER_WITH_LOG_REGEX = /^([^.]+\..+):(.+)$/;
@@ -213,7 +213,7 @@ export abstract class AbstractNativeEnvironmentService implements INativeEnviron
 	}
 
 	@memoize
-	get debugExtensionHost(): IExtensionHostDebugParams { return parseExtensionHostPort(this.args, this.isBuilt); }
+	get debugExtensionHost(): IExtensionHostDebugParams { return parseExtensionHostDebugPort(this.args, this.isBuilt); }
 	get debugRenderer(): boolean { return !!this.args.debugRenderer; }
 
 	get isBuilt(): boolean { return !env['VSCODE_DEV']; }
@@ -278,17 +278,13 @@ export abstract class AbstractNativeEnvironmentService implements INativeEnviron
 	) { }
 }
 
-export function parseExtensionHostPort(args: NativeParsedArgs, isBuild: boolean): IExtensionHostDebugParams {
+export function parseExtensionHostDebugPort(args: NativeParsedArgs, isBuild: boolean): IExtensionHostDebugParams {
 	return parseDebugParams(args['inspect-extensions'], args['inspect-brk-extensions'], 5870, isBuild, args.debugId, args.extensionEnvironment);
 }
 
-export function parsePtyHostPort(args: NativeParsedArgs, isBuild: boolean): IDebugParams {
-	return parseDebugParams(args['inspect-ptyhost'], args['inspect-brk-ptyhost'], 5877, isBuild, args.extensionEnvironment);
-}
-
-function parseDebugParams(debugArg: string | undefined, debugBrkArg: string | undefined, defaultBuildPort: number, isBuild: boolean, debugId?: string, environmentString?: string): IExtensionHostDebugParams {
+export function parseDebugParams(debugArg: string | undefined, debugBrkArg: string | undefined, defaultBuildPort: number, isBuilt: boolean, debugId?: string, environmentString?: string): IExtensionHostDebugParams {
 	const portStr = debugBrkArg || debugArg;
-	const port = Number(portStr) || (!isBuild ? defaultBuildPort : null);
+	const port = Number(portStr) || (!isBuilt ? defaultBuildPort : null);
 	const brk = port ? Boolean(!!debugBrkArg) : false;
 	let env: Record<string, string> | undefined;
 	if (environmentString) {

src/vs/platform/environment/node/argv.ts

@@ -128,6 +128,8 @@ export const OPTIONS: OptionDescriptions<Required<NativeParsedArgs>> = {
 	'inspect-brk-ptyhost': { type: 'string', allowEmptyValue: true },
 	'inspect-search': { type: 'string', deprecates: ['debugSearch'], allowEmptyValue: true },
 	'inspect-brk-search': { type: 'string', deprecates: ['debugBrkSearch'], allowEmptyValue: true },
+	'inspect-sharedprocess': { type: 'string', allowEmptyValue: true },
+	'inspect-brk-sharedprocess': { type: 'string', allowEmptyValue: true },
 	'export-default-configuration': { type: 'string' },
 	'install-source': { type: 'string' },
 	'enable-smoke-test-driver': { type: 'boolean' },

src/vs/platform/environment/node/environmentService.ts

@@ -5,7 +5,8 @@
 
 import { homedir, tmpdir } from 'os';
 import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
-import { AbstractNativeEnvironmentService } from 'vs/platform/environment/common/environmentService';
+import { IDebugParams } from 'vs/platform/environment/common/environment';
+import { AbstractNativeEnvironmentService, parseDebugParams } from 'vs/platform/environment/common/environmentService';
 import { getUserDataPath } from 'vs/platform/environment/node/userDataPath';
 import { IProductService } from 'vs/platform/product/common/productService';
 
@@ -19,3 +20,11 @@ export class NativeEnvironmentService extends AbstractNativeEnvironmentService {
 		}, productService);
 	}
 }
+
+export function parsePtyHostDebugPort(args: NativeParsedArgs, isBuild: boolean): IDebugParams {
+	return parseDebugParams(args['inspect-ptyhost'], args['inspect-brk-ptyhost'], 5877, isBuild, args.extensionEnvironment);
+}
+
+export function parseSharedProcessDebugPort(args: NativeParsedArgs, isBuild: boolean): IDebugParams {
+	return parseDebugParams(args['inspect-sharedprocess'], args['inspect-brk-sharedprocess'], 5879, isBuild, args.extensionEnvironment);
+}

src/vs/platform/environment/test/node/environmentService.test.ts

@@ -4,15 +4,15 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as assert from 'assert';
-import { parseExtensionHostPort } from 'vs/platform/environment/common/environmentService';
+import { parseExtensionHostDebugPort } from 'vs/platform/environment/common/environmentService';
 import { OPTIONS, parseArgs } from 'vs/platform/environment/node/argv';
 import { NativeEnvironmentService } from 'vs/platform/environment/node/environmentService';
 import product from 'vs/platform/product/common/product';
 
 suite('EnvironmentService', () => {
 
 	test('parseExtensionHostPort when built', () => {
-		const parse = (a: string[]) => parseExtensionHostPort(parseArgs(a, OPTIONS), true);
+		const parse = (a: string[]) => parseExtensionHostDebugPort(parseArgs(a, OPTIONS), true);
 
 		assert.deepStrictEqual(parse([]), { port: null, break: false, env: undefined, debugId: undefined });
 		assert.deepStrictEqual(parse(['--debugPluginHost']), { port: null, break: false, env: undefined, debugId: undefined });
@@ -30,7 +30,7 @@ suite('EnvironmentService', () => {
 	});
 
 	test('parseExtensionHostPort when unbuilt', () => {
-		const parse = (a: string[]) => parseExtensionHostPort(parseArgs(a, OPTIONS), false);
+		const parse = (a: string[]) => parseExtensionHostDebugPort(parseArgs(a, OPTIONS), false);
 
 		assert.deepStrictEqual(parse([]), { port: 5870, break: false, env: undefined, debugId: undefined });
 		assert.deepStrictEqual(parse(['--debugPluginHost']), { port: 5870, break: false, env: undefined, debugId: undefined });

src/vs/platform/extensions/electron-main/extensionHostStarter.ts

@@ -118,6 +118,7 @@ export class ExtensionHostStarter implements IDisposable, IExtensionHostStarter
 		this._getExtHost(id).start({
 			...opts,
 			type: 'extensionHost',
+			entryPoint: 'vs/workbench/api/node/extensionHostProcess',
 			args: ['--skipWorkspaceStorageLock'],
 			execArgv: opts.execArgv,
 			allowLoadingUnsignedLibraries: true,

src/vs/platform/sharedProcess/electron-main/sharedProcess.ts

@@ -28,8 +28,8 @@ import { ILoggerMainService } from 'vs/platform/log/electron-main/loggerService'
 import { UtilityProcess } from 'vs/platform/utilityProcess/electron-main/utilityProcess';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { NullTelemetryService } from 'vs/platform/telemetry/common/telemetryUtils';
-import { deepClone } from 'vs/base/common/objects';
-import { removeDangerousEnvVariables } from 'vs/base/common/processes';
+import { canUseUtilityProcess } from 'vs/base/parts/sandbox/electron-main/electronTypes';
+import { parseSharedProcessDebugPort } from 'vs/platform/environment/node/environmentService';
 
 export class SharedProcess extends Disposable implements ISharedProcess {
 
@@ -42,7 +42,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 	private windowCloseListener: ((event: ElectronEvent) => void) | undefined = undefined;
 
 	private utilityProcess: UtilityProcess | undefined = undefined;
-	private readonly useUtilityProcess = this.configurationService.getValue<boolean>('window.experimental.sharedProcessUseUtilityProcess');
+	private readonly useUtilityProcess = canUseUtilityProcess && this.configurationService.getValue<boolean>('window.experimental.sharedProcessUseUtilityProcess');
 
 	constructor(
 		private readonly machineId: string,
@@ -60,6 +60,10 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 		super();
 
 		this.registerListeners();
+
+		if (this.useUtilityProcess) {
+			this.logService.info('[SharedProcess] using utility process');
+		}
 	}
 
 	private registerListeners(): void {
@@ -75,7 +79,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 	}
 
 	private async onWindowConnection(e: IpcMainEvent, nonce: string): Promise<void> {
-		this.logService.trace('SharedProcess: on vscode:createSharedProcessMessageChannel');
+		this.logService.trace('[SharedProcess] on vscode:createSharedProcessMessageChannel');
 
 		// release barrier if this is the first window connection
 		if (!this.firstWindowConnectionBarrier.isOpen()) {
@@ -105,7 +109,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 	}
 
 	private onWorkerConnection(e: IpcMainEvent, configuration: IUtilityProcessWorkerConfiguration): void {
-		this.logService.trace('SharedProcess: onWorkerConnection', configuration);
+		this.logService.trace('[SharedProcess] onWorkerConnection', configuration);
 
 		const disposables = new DisposableStore();
 
@@ -114,7 +118,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 				return; // the shared process is already gone, no need to dispose anything
 			}
 
-			this.logService.trace(`SharedProcess: disposing worker (reason: '${reason}')`, configuration);
+			this.logService.trace(`[SharedProcess] disposing worker (reason: '${reason}')`, configuration);
 
 			// Only once!
 			disposables.dispose();
@@ -142,7 +146,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 	}
 
 	private onWillShutdown(): void {
-		this.logService.trace('SharedProcess: onWillShutdown');
+		this.logService.trace('[SharedProcess] onWillShutdown');
 
 		if (this.utilityProcess) {
 			this.utilityProcess.postMessage('vscode:electron-main->shared-process=exit');
@@ -171,10 +175,10 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 			// Electron seems to crash on Windows without this setTimeout :|
 			setTimeout(() => {
 				try {
-					this.logService.trace('SharedProcess: onWillShutdown window.close()');
+					this.logService.trace('[SharedProcess] onWillShutdown window.close()');
 					window.close();
 				} catch (error) {
-					this.logService.trace(`SharedProcess: onWillShutdown window.close() error: ${error}`); // ignore, as electron is already shutting down
+					this.logService.trace(`[SharedProcess] onWillShutdown window.close() error: ${error}`); // ignore, as electron is already shutting down
 				}
 
 				this.window = undefined;
@@ -214,7 +218,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 				}
 
 				await whenReady.p;
-				this.logService.info('SharedProcess: IPC ready');
+				this.logService.trace('[SharedProcess] Overall ready');
 			})();
 		}
 
@@ -241,7 +245,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 				}
 
 				await sharedProcessIpcReady.p;
-				this.logService.info('SharedProcess: IPC ready');
+				this.logService.trace('[SharedProcess] IPC ready');
 			})();
 		}
 
@@ -264,29 +268,25 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 	private createUtilityProcess(): void {
 		this.utilityProcess = this._register(new UtilityProcess(this.logService, NullTelemetryService, this.lifecycleMainService));
 
+		const inspectParams = parseSharedProcessDebugPort(this.environmentMainService.args, this.environmentMainService.isBuilt);
+		let execArgv: string[] | undefined = undefined;
+		if (inspectParams.port) {
+			execArgv = ['--nolazy'];
+			if (inspectParams.break) {
+				execArgv.push(`--inspect-brk=${inspectParams.port}`);
+			} else {
+				execArgv.push(`--inspect=${inspectParams.port}`);
+			}
+		}
+
 		this.utilityProcess.start({
 			type: 'shared-process',
+			entryPoint: 'vs/code/electron-browser/sharedProcess/sharedProcessMain',
 			payload: this.createSharedProcessConfiguration(),
-			env: this.getEnv(),
-			execArgv: (!this.environmentMainService.isBuilt || this.environmentMainService.verbose) ? ['--nolazy', '--inspect=5896'] : undefined, // TODO@bpasero this make configurable
+			execArgv
 		});
 	}
 
-	private getEnv(): NodeJS.ProcessEnv {
-		const env: NodeJS.ProcessEnv = {
-			...deepClone(process.env),
-			VSCODE_AMD_ENTRYPOINT: 'vs/code/electron-browser/sharedProcess/sharedProcessMain',
-			VSCODE_PIPE_LOGGING: 'true',
-			VSCODE_VERBOSE_LOGGING: 'true',
-			VSCODE_PARENT_PID: String(process.pid)
-		};
-
-		// Sanitize environment
-		removeDangerousEnvVariables(env);
-
-		return env;
-	}
-
 	private createWindow(): void {
 		const configObjectUrl = this._register(this.protocolMainService.createIPCObjectUrl<ISharedProcessConfiguration>());
 
@@ -401,6 +401,10 @@ export class SharedProcess extends Disposable implements ISharedProcess {
 		return this.window?.isVisible() ?? false;
 	}
 
+	usingUtilityProcess(): boolean {
+		return !!this.utilityProcess;
+	}
+
 	private isWindowAlive(): boolean {
 		const window = this.window;
 		if (!window) {

src/vs/platform/terminal/node/ptyHostService.ts

@@ -11,7 +11,7 @@ import { ProxyChannel } from 'vs/base/parts/ipc/common/ipc';
 import { Client, IIPCOptions } from 'vs/base/parts/ipc/node/ipc.cp';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { IEnvironmentService, INativeEnvironmentService } from 'vs/platform/environment/common/environment';
-import { parsePtyHostPort } from 'vs/platform/environment/common/environmentService';
+import { parsePtyHostDebugPort } from 'vs/platform/environment/node/environmentService';
 import { getResolvedShellEnv } from 'vs/platform/shell/node/shellEnv';
 import { ILogService, ILoggerService } from 'vs/platform/log/common/log';
 import { RequestStore } from 'vs/platform/terminal/common/requestStore';
@@ -149,7 +149,7 @@ export class PtyHostService extends Disposable implements IPtyService {
 			}
 		};
 
-		const ptyHostDebug = parsePtyHostPort(this._environmentService.args, this._environmentService.isBuilt);
+		const ptyHostDebug = parsePtyHostDebugPort(this._environmentService.args, this._environmentService.isBuilt);
 		if (ptyHostDebug) {
 			if (ptyHostDebug.break && ptyHostDebug.port) {
 				opts.debugBrk = ptyHostDebug.port;

src/vs/platform/utilityProcess/electron-main/utilityProcess.ts

@@ -15,6 +15,8 @@ import { IWindowsMainService } from 'vs/platform/windows/electron-main/windows';
 import Severity from 'vs/base/common/severity';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { ILifecycleMainService } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { removeDangerousEnvVariables } from 'vs/base/common/processes';
+import { deepClone } from 'vs/base/common/objects';
 
 export interface IUtilityProcessConfiguration {
 
@@ -23,6 +25,11 @@ export interface IUtilityProcessConfiguration {
 	 */
 	readonly type: string;
 
+	/**
+	 * The entry point to load in the utility process.
+	 */
+	readonly entryPoint: string;
+
 	/**
 	 * An optional serializable object to be sent into the utility process
 	 * as first message alongside the message port.
@@ -55,6 +62,13 @@ export interface IUtilityProcessConfiguration {
 	 * with other components.
 	 */
 	readonly correlationId?: string;
+
+	/**
+	 * Optional pid of the parent process. If set, the
+	 * utility process will be terminated when the parent
+	 * process exits.
+	 */
+	readonly parentLifecycleBound?: number;
 }
 
 export interface IWindowUtilityProcessConfiguration extends IUtilityProcessConfiguration {
@@ -195,15 +209,7 @@ export class UtilityProcess extends Disposable {
 		const execArgv = [...this.configuration.execArgv ?? [], `--vscode-utility-kind=${this.configuration.type}`];
 		const allowLoadingUnsignedLibraries = this.configuration.allowLoadingUnsignedLibraries;
 		const stdio = 'pipe';
-
-		let env: { [key: string]: any } | undefined = this.configuration.env;
-		if (env) {
-			env = { ...env }; // make a copy since we may be going to mutate it
-
-			for (const key of Object.keys(env)) {
-				env[key] = String(env[key]); // make sure all values are strings, otherwise the process will not start
-			}
-		}
+		const env = this.createEnv(configuration);
 
 		this.log('creating new...', Severity.Info);
 
@@ -222,6 +228,26 @@ export class UtilityProcess extends Disposable {
 		return true;
 	}
 
+	private createEnv(configuration: IUtilityProcessConfiguration): { [key: string]: any } {
+		const env: { [key: string]: any } = configuration.env ? { ...configuration.env } : { ...deepClone(process.env) };
+
+		// Apply support environment variables from config
+		env['VSCODE_AMD_ENTRYPOINT'] = configuration.entryPoint;
+		if (typeof configuration.parentLifecycleBound === 'number') {
+			env['VSCODE_PARENT_PID'] = String(configuration.parentLifecycleBound);
+		}
+
+		// Remove any environment variables that are not allowed
+		removeDangerousEnvVariables(env);
+
+		// Ensure all values are strings, otherwise the process will not start
+		for (const key of Object.keys(env)) {
+			env[key] = String(env[key]);
+		}
+
+		return env;
+	}
+
 	private registerListeners(process: UtilityProcessProposedApi.UtilityProcess, configuration: IUtilityProcessConfiguration, serviceName: string, isWindowSandboxed: boolean): void {
 
 		// Stdout