From 92281913a1b186b12da94a3af640128c0b563629 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 6 Apr 2021 15:37:42 +0200 Subject: [PATCH] shell env - change to ipc invoke/handle to speed up resolution --- src/bootstrap-window.js | 6 +- .../parts/sandbox/electron-browser/preload.js | 55 ++++----- .../parts/sandbox/electron-sandbox/globals.ts | 15 +-- src/vs/code/electron-main/app.ts | 106 +++++++++--------- .../electron-browser/terminal.contribution.ts | 2 +- .../shellEnvironmentService.ts | 2 +- 6 files changed, 81 insertions(+), 105 deletions(-) diff --git a/src/bootstrap-window.js b/src/bootstrap-window.js index c44267f3f3c..2bd694abf32 100644 --- a/src/bootstrap-window.js +++ b/src/bootstrap-window.js @@ -26,10 +26,6 @@ const safeProcess = preloadGlobals.process; const useCustomProtocol = safeProcess.sandboxed || typeof safeProcess.env['VSCODE_BROWSER_CODE_LOADING'] === 'string'; - // Start to resolve process.env before anything gets load - // so that we can run loading and resolving in parallel - const whenEnvResolved = safeProcess.resolveEnv(); - /** * @typedef {import('./vs/base/parts/sandbox/common/sandboxTypes').ISandboxConfiguration} ISandboxConfiguration * @@ -169,7 +165,7 @@ // Wait for process environment being fully resolved performance.mark('code/willWaitForShellEnv'); if (!safeProcess.env['VSCODE_SKIP_PROCESS_ENV_PATCHING'] /* TODO@bpasero for https://github.com/microsoft/vscode/issues/108804 */) { - await whenEnvResolved; + await safeProcess.shellEnv(); } performance.mark('code/didWaitForShellEnv'); diff --git a/src/vs/base/parts/sandbox/electron-browser/preload.js b/src/vs/base/parts/sandbox/electron-browser/preload.js index 469a0861e0d..bae46cc3d95 100644 --- a/src/vs/base/parts/sandbox/electron-browser/preload.js +++ b/src/vs/base/parts/sandbox/electron-browser/preload.js @@ -69,8 +69,13 @@ try { if (validateIPC(windowConfigIpcChannel)) { + + // Resolve configuration from electron-main configuration = await ipcRenderer.invoke(windowConfigIpcChannel); + // Apply `userEnv` directly + Object.assign(process.env, configuration.userEnv); + // Apply zoom level early before even building the // window DOM elements to avoid UI flicker. We always // have to set the zoom level from within the window @@ -90,42 +95,31 @@ //#region Resolve Shell Environment - /** @type {Promise | undefined} */ - let shellEnv = undefined; - /** * If VSCode is not run from a terminal, we should resolve additional * shell specific environment from the OS shell to ensure we are seeing * all development related environment variables. We do this from the * main process because it may involve spawning a shell. * - * @returns {Promise} + * @type {Promise} */ - async function resolveEnv() { - if (!shellEnv) { - const userEnv = (await resolveConfiguration).userEnv; + const resolveShellEnv = (async () => { - // Apply `userEnv` directly - Object.assign(process.env, userEnv); + // Resolve `userEnv` from configuration and + // `shellEnv` from the main side + const [userEnv, shellEnv] = await Promise.all([ + (async () => (await resolveConfiguration).userEnv)(), + ipcRenderer.invoke('vscode:fetchShellEnv') + ]); - // Resolve `shellEnv` from the main side - shellEnv = new Promise(function (resolve) { - ipcRenderer.once('vscode:acceptShellEnv', function (event, shellEnvResult) { - if (!process.env['VSCODE_SKIP_PROCESS_ENV_PATCHING'] /* TODO@bpasero for https://github.com/microsoft/vscode/issues/108804 */) { - // Assign all keys of the shell environment to our process environment - // But make sure that the user environment wins in the end over shell environment - Object.assign(process.env, shellEnvResult, userEnv); - } - - resolve({ ...process.env, ...shellEnvResult, ...userEnv }); - }); - - ipcRenderer.send('vscode:fetchShellEnv'); - }); + if (!process.env['VSCODE_SKIP_PROCESS_ENV_PATCHING'] /* TODO@bpasero for https://github.com/microsoft/vscode/issues/108804 */) { + // Assign all keys of the shell environment to our process environment + // But make sure that the user environment wins in the end over shell environment + Object.assign(process.env, shellEnv, userEnv); } - await shellEnv; - } + return { ...process.env, ...shellEnv, ...userEnv }; + })(); //#endregion @@ -309,15 +303,8 @@ /** * @returns {Promise} */ - getShellEnv() { - return shellEnv; - }, - - /** - * @returns {Promise} - */ - resolveEnv() { - return resolveEnv(); + shellEnv() { + return resolveShellEnv; }, /** diff --git a/src/vs/base/parts/sandbox/electron-sandbox/globals.ts b/src/vs/base/parts/sandbox/electron-sandbox/globals.ts index b6ad01854dc..4103c867f88 100644 --- a/src/vs/base/parts/sandbox/electron-sandbox/globals.ts +++ b/src/vs/base/parts/sandbox/electron-sandbox/globals.ts @@ -75,8 +75,8 @@ export interface ISandboxNodeProcess extends INodeProcess { getProcessMemoryInfo: () => Promise; /** - * A custom method we add to `process`: Resolve the true process environment to use and - * apply it to `process.env`. + * Returns a process environment that includes all shell environment variables even if + * the application was not started from a shell / terminal / console. * * There are different layers of environment that will apply: * - `process.env`: this is the actual environment of the process before this method @@ -87,17 +87,8 @@ export interface ISandboxNodeProcess extends INodeProcess { * from a terminal and changed certain variables * * The order of overwrites is `process.env` < `shellEnv` < `userEnv`. - * - * It is critical that every process awaits this method early on startup to get the right - * set of environment in `process.env`. */ - resolveEnv(): Promise; - - /** - * Returns a process environment that includes any shell environment even if the application - * was not started from a shell / terminal / console. - */ - getShellEnv(): Promise; + shellEnv(): Promise; } export interface IpcMessagePort { diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 43f3f2cfb00..e855325520e 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -281,71 +281,73 @@ export class CodeApplication extends Disposable { //#region Bootstrap IPC Handlers let slowShellResolveWarningShown = false; - ipcMain.on('vscode:fetchShellEnv', async event => { + ipcMain.handle('vscode:fetchShellEnv', event => { + return new Promise(async resolve => { - // DO NOT remove: not only usual windows are fetching the - // shell environment but also shared process, issue reporter - // etc, so we need to reply via `webContents` always - const webContents = event.sender; + // DO NOT remove: not only usual windows are fetching the + // shell environment but also shared process, issue reporter + // etc, so we need to reply via `webContents` always + const webContents = event.sender; - let replied = false; + let replied = false; - function acceptShellEnv(env: IProcessEnvironment): void { - clearTimeout(shellEnvSlowWarningHandle); - clearTimeout(shellEnvTimeoutErrorHandle); + function acceptShellEnv(env: IProcessEnvironment): void { + clearTimeout(shellEnvSlowWarningHandle); + clearTimeout(shellEnvTimeoutErrorHandle); - if (!replied) { - replied = true; + if (!replied) { + replied = true; - if (!webContents.isDestroyed()) { - webContents.send('vscode:acceptShellEnv', env); + if (!webContents.isDestroyed()) { + resolve(env); + } } } - } - // Handle slow shell environment resolve calls: - // - a warning after 3s but continue to resolve (only once in active window) - // - an error after 10s and stop trying to resolve (in every window where this happens) - const cts = new CancellationTokenSource(); + // Handle slow shell environment resolve calls: + // - a warning after 3s but continue to resolve (only once in active window) + // - an error after 10s and stop trying to resolve (in every window where this happens) + const cts = new CancellationTokenSource(); - const shellEnvSlowWarningHandle = setTimeout(() => { - if (!slowShellResolveWarningShown) { - this.windowsMainService?.sendToFocused('vscode:showShellEnvSlowWarning', cts.token); - slowShellResolveWarningShown = true; + const shellEnvSlowWarningHandle = setTimeout(() => { + if (!slowShellResolveWarningShown) { + this.windowsMainService?.sendToFocused('vscode:showShellEnvSlowWarning', cts.token); + slowShellResolveWarningShown = true; + } + }, 3000); + + const window = this.windowsMainService?.getWindowByWebContents(event.sender); // Note: this can be `undefined` for the shared process!! + const shellEnvTimeoutErrorHandle = setTimeout(() => { + cts.dispose(true); + window?.sendWhenReady('vscode:showShellEnvTimeoutError', CancellationToken.None); + acceptShellEnv({}); + }, 10000); + + // Prefer to use the args and env from the target window + // when resolving the shell env. It is possible that + // a first window was opened from the UI but a second + // from the CLI and that has implications for whether to + // resolve the shell environment or not. + // + // Window can be undefined for e.g. the shared process + // that is not part of our windows registry! + let args: NativeParsedArgs; + let env: IProcessEnvironment; + if (window?.config) { + args = window.config; + env = { ...process.env, ...window.config.userEnv }; + } else { + args = this.environmentMainService.args; + env = process.env; } - }, 3000); - const window = this.windowsMainService?.getWindowByWebContents(event.sender); // Note: this can be `undefined` for the shared process!! - const shellEnvTimeoutErrorHandle = setTimeout(() => { - cts.dispose(true); - window?.sendWhenReady('vscode:showShellEnvTimeoutError', CancellationToken.None); - acceptShellEnv({}); - }, 10000); - - // Prefer to use the args and env from the target window - // when resolving the shell env. It is possible that - // a first window was opened from the UI but a second - // from the CLI and that has implications for whether to - // resolve the shell environment or not. - // - // Window can be undefined for e.g. the shared process - // that is not part of our windows registry! - let args: NativeParsedArgs; - let env: IProcessEnvironment; - if (window?.config) { - args = window.config; - env = { ...process.env, ...window.config.userEnv }; - } else { - args = this.environmentMainService.args; - env = process.env; - } - - // Resolve shell env - const shellEnv = await resolveShellEnv(this.logService, args, env); - acceptShellEnv(shellEnv); + // Resolve shell env + const shellEnv = await resolveShellEnv(this.logService, args, env); + acceptShellEnv(shellEnv); + }); }); - ipcMain.handle('vscode:writeNlsFile', async (event, path: unknown, data: unknown) => { + ipcMain.handle('vscode:writeNlsFile', (event, path: unknown, data: unknown) => { const uri = this.validateNlsPath([path]); if (!uri || typeof data !== 'string') { throw new Error('Invalid operation (vscode:writeNlsFile)'); diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts index 6b8955477c1..943630caba9 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts @@ -27,5 +27,5 @@ workbenchRegistry.registerWorkbenchContribution(TerminalNativeContribution, Life // Register configurations const configurationRegistry = Registry.as(Extensions.Configuration); -const systemShell = async (p: Platform) => getSystemShell(p, await process.getShellEnv()); +const systemShell = async (p: Platform) => getSystemShell(p, await process.shellEnv()); getTerminalShellConfiguration(systemShell).then(config => configurationRegistry.registerConfiguration(config)); diff --git a/src/vs/workbench/services/environment/electron-sandbox/shellEnvironmentService.ts b/src/vs/workbench/services/environment/electron-sandbox/shellEnvironmentService.ts index fe1416ab970..a0c728b7c2d 100644 --- a/src/vs/workbench/services/environment/electron-sandbox/shellEnvironmentService.ts +++ b/src/vs/workbench/services/environment/electron-sandbox/shellEnvironmentService.ts @@ -22,7 +22,7 @@ export class ShellEnvironmentService implements IShellEnvironmentService { declare readonly _serviceBrand: undefined; getShellEnv(): Promise { - return process.getShellEnv(); + return process.shellEnv(); } }