From ed6bb0790b236b6732dba147021a2eefe7d94a13 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 24 Feb 2023 15:41:33 +0100 Subject: [PATCH] sandbox - enable utility process in insiders by default (#175326) --- src/vs/code/electron-main/app.ts | 6 +++--- .../code/node/sharedProcess/sharedProcessMain.ts | 11 ++++++++--- .../sharedProcess/electron-main/sharedProcess.ts | 14 +++++++++++++- .../utilityProcessWorkerMainService.ts | 10 ++++++++-- .../windows/electron-main/windowsMainService.ts | 12 ++++++++++-- .../contrib/files/browser/files.contribution.ts | 7 ------- .../relauncher/browser/relauncher.contribution.ts | 6 ------ .../electron-sandbox/desktop.contribution.ts | 2 +- .../files/electron-sandbox/watcherClient.ts | 4 +--- .../utilityProcessWorkerWorkbenchService.ts | 8 ++++++++ 10 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 23c44b398cc..1e2de12ca67 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -1248,13 +1248,13 @@ export class CodeApplication extends Disposable { // Logging switch (type) { case WindowError.PROCESS_GONE: - this.logService.error(`SharedProcess: renderer process gone (reason: ${details?.reason || ''}, code: ${details?.exitCode || ''})`); + this.logService.error(`[SharedProcess] renderer process gone (reason: ${details?.reason || ''}, code: ${details?.exitCode || ''})`); break; case WindowError.UNRESPONSIVE: - this.logService.error('SharedProcess: detected unresponsive'); + this.logService.error('[SharedProcess] detected unresponsive'); break; case WindowError.LOAD: - this.logService.error(`SharedProcess: failed to load (reason: ${details?.reason || ''}, code: ${details?.exitCode || ''})`); + this.logService.error(`[SharedProcess] failed to load (reason: ${details?.reason || ''}, code: ${details?.exitCode || ''})`); break; } diff --git a/src/vs/code/node/sharedProcess/sharedProcessMain.ts b/src/vs/code/node/sharedProcess/sharedProcessMain.ts index 2f602186c76..b289c060ead 100644 --- a/src/vs/code/node/sharedProcess/sharedProcessMain.ts +++ b/src/vs/code/node/sharedProcess/sharedProcessMain.ts @@ -145,9 +145,14 @@ class SharedProcessMain extends Disposable { private registerListeners(): void { // Shared process lifecycle - const onExit = async () => { - this.lifecycleService?.fireOnWillShutdown(); - this.dispose(); + let didExit = false; + const onExit = () => { + if (!didExit) { + didExit = true; + + this.lifecycleService?.fireOnWillShutdown(); + this.dispose(); + } }; process.once('exit', onExit); if (isUtilityProcess(process)) { diff --git a/src/vs/platform/sharedProcess/electron-main/sharedProcess.ts b/src/vs/platform/sharedProcess/electron-main/sharedProcess.ts index 0fc12536e0c..cb5d7a3066c 100644 --- a/src/vs/platform/sharedProcess/electron-main/sharedProcess.ts +++ b/src/vs/platform/sharedProcess/electron-main/sharedProcess.ts @@ -42,7 +42,19 @@ export class SharedProcess extends Disposable implements ISharedProcess { private windowCloseListener: ((event: ElectronEvent) => void) | undefined = undefined; private utilityProcess: UtilityProcess | undefined = undefined; - private readonly useUtilityProcess = canUseUtilityProcess && this.configurationService.getValue('window.experimental.sharedProcessUseUtilityProcess'); + private readonly useUtilityProcess = (() => { + let useUtilityProcess = false; + if (canUseUtilityProcess) { + const sharedProcessUseUtilityProcess = this.configurationService.getValue('window.experimental.sharedProcessUseUtilityProcess'); + if (typeof sharedProcessUseUtilityProcess === 'boolean') { + useUtilityProcess = sharedProcessUseUtilityProcess; + } else { + useUtilityProcess = typeof product.quality === 'string' && product.quality !== 'stable'; + } + } + + return useUtilityProcess; + })(); constructor( private readonly machineId: string, diff --git a/src/vs/platform/utilityProcess/electron-main/utilityProcessWorkerMainService.ts b/src/vs/platform/utilityProcess/electron-main/utilityProcessWorkerMainService.ts index b5e54ffa725..70be47e3b27 100644 --- a/src/vs/platform/utilityProcess/electron-main/utilityProcessWorkerMainService.ts +++ b/src/vs/platform/utilityProcess/electron-main/utilityProcessWorkerMainService.ts @@ -58,9 +58,15 @@ export class UtilityProcessWorkerMainService extends Disposable implements IUtil this.workers.set(workerId, worker); const onDidTerminate = new DeferredPromise(); - Event.once(worker.onDidTerminate)(e => { + Event.once(worker.onDidTerminate)(reason => { + if (reason.code === 0) { + this.logService.trace(`[UtilityProcessWorker]: terminated normally with code ${reason.code}, signal: ${reason.signal}`); + } else { + this.logService.error(`[UtilityProcessWorker]: terminated unexpectedly with code ${reason.code}, signal: ${reason.signal}`); + } + this.workers.delete(workerId); - onDidTerminate.complete({ reason: e }); + onDidTerminate.complete({ reason }); }); return onDidTerminate.p; diff --git a/src/vs/platform/windows/electron-main/windowsMainService.ts b/src/vs/platform/windows/electron-main/windowsMainService.ts index 62fa52e0cbb..14d03fb6e26 100644 --- a/src/vs/platform/windows/electron-main/windowsMainService.ts +++ b/src/vs/platform/windows/electron-main/windowsMainService.ts @@ -1312,7 +1312,6 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic private async openInBrowserWindow(options: IOpenBrowserWindowOptions): Promise { const windowConfig = this.configurationService.getValue('window'); - const filesConfig = this.configurationService.getValue<{ experimental?: { watcherUseUtilityProcess?: boolean } } | undefined>('files'); const lastActiveWindow = this.getLastActiveWindow(); const defaultProfile = lastActiveWindow?.profile ?? this.userDataProfilesMainService.defaultProfile; @@ -1325,6 +1324,15 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic } } + let preferUtilityProcess = false; + if (canUseUtilityProcess) { + if (typeof windowConfig?.experimental?.sharedProcessUseUtilityProcess === 'boolean') { + preferUtilityProcess = windowConfig.experimental.sharedProcessUseUtilityProcess; + } else { + preferUtilityProcess = typeof product.quality === 'string' && product.quality !== 'stable'; + } + } + // Build up the window configuration from provided options, config and environment const configuration: INativeWindowConfiguration = { @@ -1390,7 +1398,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic policiesData: this.policyService.serialize(), continueOn: this.environmentMainService.continueOn, - preferUtilityProcess: canUseUtilityProcess ? (filesConfig?.experimental?.watcherUseUtilityProcess ?? windowConfig?.experimental?.sharedProcessUseUtilityProcess ?? false) : false + preferUtilityProcess }; // New window diff --git a/src/vs/workbench/contrib/files/browser/files.contribution.ts b/src/vs/workbench/contrib/files/browser/files.contribution.ts index 54f7839e0d2..ffd76194863 100644 --- a/src/vs/workbench/contrib/files/browser/files.contribution.ts +++ b/src/vs/workbench/contrib/files/browser/files.contribution.ts @@ -260,13 +260,6 @@ configurationRegistry.registerConfiguration({ 'description': nls.localize('watcherInclude', "Configure extra paths to watch for changes inside the workspace. By default, all workspace folders will be watched recursively, except for folders that are symbolic links. You can explicitly add absolute or relative paths to support watching folders that are symbolic links. Relative paths will be resolved to an absolute path using the currently opened workspace."), 'scope': ConfigurationScope.RESOURCE }, - 'files.experimental.watcherUseUtilityProcess': { // TODO@bpasero remove me once sandbox is enabled by default - type: 'boolean', - description: nls.localize('watcherUseUtilityProcess', "When enabled, the file watcher will be launched using the new UtilityProcess Electron API."), - default: false, //typeof product.quality === 'string' && product.quality !== 'stable', // disabled by default in stable for now - ignoreSync: true, - 'scope': ConfigurationScope.APPLICATION - }, 'files.hotExit': hotExitConfiguration, 'files.defaultLanguage': { 'type': 'string', diff --git a/src/vs/workbench/contrib/relauncher/browser/relauncher.contribution.ts b/src/vs/workbench/contrib/relauncher/browser/relauncher.contribution.ts index 45c9bd63d79..f482f45334a 100644 --- a/src/vs/workbench/contrib/relauncher/browser/relauncher.contribution.ts +++ b/src/vs/workbench/contrib/relauncher/browser/relauncher.contribution.ts @@ -22,7 +22,6 @@ import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/ import { IProductService } from 'vs/platform/product/common/productService'; interface IConfiguration extends IWindowsConfiguration { - files?: { experimental?: { watcherUseUtilityProcess?: boolean } }; update?: { mode?: string }; debug?: { console?: { wordWrap?: boolean } }; editor?: { accessibilitySupport?: 'on' | 'off' | 'auto' }; @@ -39,7 +38,6 @@ export class SettingsChangeRelauncher extends Disposable implements IWorkbenchCo 'window.experimental.windowControlsOverlay.enabled', 'window.experimental.useSandbox', 'window.experimental.sharedProcessUseUtilityProcess', - 'files.experimental.watcherUseUtilityProcess', 'window.nativeTabs', 'window.nativeFullScreen', 'window.clickThroughInactive', @@ -53,7 +51,6 @@ export class SettingsChangeRelauncher extends Disposable implements IWorkbenchCo private readonly titleBarStyle = new ChangeObserver<'native' | 'custom'>('string'); private readonly windowControlsOverlayEnabled = new ChangeObserver('boolean'); private readonly windowSandboxEnabled = new ChangeObserver('boolean'); - private readonly fileWatcherUtilityProcessEnabled = new ChangeObserver('boolean'); private readonly sharedProcessUtilityProcessEnabled = new ChangeObserver('boolean'); private readonly nativeTabs = new ChangeObserver('boolean'); private readonly nativeFullScreen = new ChangeObserver('boolean'); @@ -100,9 +97,6 @@ export class SettingsChangeRelauncher extends Disposable implements IWorkbenchCo // Windows: Sandbox processChanged(this.windowSandboxEnabled.handleChange(config.window?.experimental?.useSandbox)); - // File Watcher: Utility Process - processChanged(this.fileWatcherUtilityProcessEnabled.handleChange(config.files?.experimental?.watcherUseUtilityProcess)); - // Shared Process: Utility Process processChanged(this.sharedProcessUtilityProcessEnabled.handleChange(config.window?.experimental?.sharedProcessUseUtilityProcess)); diff --git a/src/vs/workbench/electron-sandbox/desktop.contribution.ts b/src/vs/workbench/electron-sandbox/desktop.contribution.ts index 1c7aa6f610a..a9f40dce969 100644 --- a/src/vs/workbench/electron-sandbox/desktop.contribution.ts +++ b/src/vs/workbench/electron-sandbox/desktop.contribution.ts @@ -270,7 +270,7 @@ import { applicationConfigurationNodeBase } from 'vs/workbench/common/configurat 'window.experimental.sharedProcessUseUtilityProcess': { // TODO@bpasero remove me once sandbox is final type: 'boolean', description: localize('experimentalUseSharedProcessUseUtilityProcess', "Experimental: When enabled, the window will have sandbox mode enabled via Electron API."), - default: false, //typeof product.quality === 'string' && product.quality !== 'stable', // disabled by default in stable for now + default: typeof product.quality === 'string' && product.quality !== 'stable', // disabled by default in stable for now 'scope': ConfigurationScope.APPLICATION, ignoreSync: true } diff --git a/src/vs/workbench/services/files/electron-sandbox/watcherClient.ts b/src/vs/workbench/services/files/electron-sandbox/watcherClient.ts index 961c413d0cd..5f9237fd930 100644 --- a/src/vs/workbench/services/files/electron-sandbox/watcherClient.ts +++ b/src/vs/workbench/services/files/electron-sandbox/watcherClient.ts @@ -45,9 +45,7 @@ export class UniversalWatcherClient extends AbstractUniversalWatcherClient { if (reason?.code === 0) { this.trace(`terminated by itself with code ${reason.code}, signal: ${reason.signal}`); } else { - if (reason) { - this.onError(`terminated by itself unexpectedly with code ${reason.code}, signal: ${reason.signal}`); - } + this.onError(`terminated by itself unexpectedly with code ${reason?.code}, signal: ${reason?.signal}`); } }); diff --git a/src/vs/workbench/services/utilityProcess/electron-sandbox/utilityProcessWorkerWorkbenchService.ts b/src/vs/workbench/services/utilityProcess/electron-sandbox/utilityProcessWorkerWorkbenchService.ts index 801b5d805fc..83d57784e70 100644 --- a/src/vs/workbench/services/utilityProcess/electron-sandbox/utilityProcessWorkerWorkbenchService.ts +++ b/src/vs/workbench/services/utilityProcess/electron-sandbox/utilityProcessWorkerWorkbenchService.ts @@ -132,6 +132,14 @@ export class UtilityProcessWorkerWorkbenchService extends Disposable implements const client = disposables.add(new MessagePortClient(port, `window:${this.windowId},module:${process.moduleId}`)); this.logService.trace('Renderer->UtilityProcess#createWorkerChannel: connection established'); + onDidTerminate.then(({ reason }) => { + if (reason?.code === 0) { + this.logService.trace(`[UtilityProcessWorker]: terminated normally with code ${reason.code}, signal: ${reason.signal}`); + } else { + this.logService.error(`[UtilityProcessWorker]: terminated unexpectedly with code ${reason?.code}, signal: ${reason?.signal}`); + } + }); + return { client, onDidTerminate, dispose: () => disposables.dispose() }; }