diff options
author | Benjamin Pasero <benjamin.pasero@microsoft.com> | 2021-05-03 14:31:35 +0300 |
---|---|---|
committer | Benjamin Pasero <benjamin.pasero@microsoft.com> | 2021-05-03 14:31:35 +0300 |
commit | 89731c820537c86073e13b5f52b24c587ad340df (patch) | |
tree | 26c47e130362e952b4f1137c5122addd380348e8 | |
parent | 16baed2dd411bb241fb259e67d37c84505cad5c7 (diff) |
Remove shell environment patching in the renderer (fix #108804)
-rw-r--r-- | src/bootstrap-window.js | 7 | ||||
-rw-r--r-- | src/vs/base/parts/sandbox/electron-browser/preload.js | 6 | ||||
-rw-r--r-- | src/vs/code/electron-main/app.ts | 95 | ||||
-rw-r--r-- | src/vs/workbench/contrib/files/browser/editors/binaryFileEditor.ts | 7 | ||||
-rw-r--r-- | src/vs/workbench/contrib/performance/browser/perfviewEditor.ts | 1 | ||||
-rw-r--r-- | src/vs/workbench/electron-sandbox/window.ts | 24 | ||||
-rw-r--r-- | src/vs/workbench/services/timer/browser/timerService.ts | 11 |
7 files changed, 36 insertions, 115 deletions
diff --git a/src/bootstrap-window.js b/src/bootstrap-window.js index ec669360321..c1f8f1841a4 100644 --- a/src/bootstrap-window.js +++ b/src/bootstrap-window.js @@ -179,13 +179,6 @@ require(modulePaths, async result => { try { - // 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 safeProcess.shellEnv(); - } - performance.mark('code/didWaitForShellEnv'); - // Callback only after process environment is resolved const callbackResult = resultCallback(result, configuration); if (callbackResult instanceof Promise) { diff --git a/src/vs/base/parts/sandbox/electron-browser/preload.js b/src/vs/base/parts/sandbox/electron-browser/preload.js index bae46cc3d95..67857146ad4 100644 --- a/src/vs/base/parts/sandbox/electron-browser/preload.js +++ b/src/vs/base/parts/sandbox/electron-browser/preload.js @@ -112,12 +112,6 @@ ipcRenderer.invoke('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); - } - return { ...process.env, ...shellEnv, ...userEnv }; })(); diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 2a8081fe2f2..934751f89b3 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -81,7 +81,7 @@ import { IKeyboardLayoutMainService, KeyboardLayoutMainService } from 'vs/platfo import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; import { isLaunchedFromCli } from 'vs/platform/environment/node/argvHelper'; import { isEqualOrParent } from 'vs/base/common/extpath'; -import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; +import { RunOnceScheduler } from 'vs/base/common/async'; import { IExtensionUrlTrustService } from 'vs/platform/extensionManagement/common/extensionUrlTrust'; import { ExtensionUrlTrustService } from 'vs/platform/extensionManagement/node/extensionUrlTrustService'; import { once } from 'vs/base/common/functional'; @@ -282,71 +282,29 @@ export class CodeApplication extends Disposable { //#region Bootstrap IPC Handlers - let slowShellResolveWarningShown = false; 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; - - let replied = false; - - function acceptShellEnv(env: IProcessEnvironment): void { - clearTimeout(shellEnvSlowWarningHandle); - clearTimeout(shellEnvTimeoutErrorHandle); - - if (!replied) { - replied = true; - - 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(); - - 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; - } + // 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! + const window = this.windowsMainService?.getWindowByWebContents(event.sender); // Note: this can be `undefined` for the shared process + 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 + return resolveShellEnv(this.logService, args, env); }); ipcMain.handle('vscode:writeNlsFile', (event, path: unknown, data: unknown) => { @@ -1012,7 +970,16 @@ export class CodeApplication extends Disposable { } // Start to fetch shell environment (if needed) after window has opened - resolveShellEnv(this.logService, this.environmentMainService.args, process.env); + // Since this operation can take a long time, we want to warm it up while + // the window is opening. + // We also print a warning if the resolution takes longer than 10s. + (async () => { + const slowResolveShellEnvWarning = this._register(new RunOnceScheduler(() => this.logService.warn('Resolving your shell environment is taking more than 10s. Please review your shell configuration. Learn more at https://go.microsoft.com/fwlink/?linkid=2149667.'), 10000)); + slowResolveShellEnvWarning.schedule(); + + await resolveShellEnv(this.logService, this.environmentMainService.args, process.env); + slowResolveShellEnvWarning.dispose(); + })(); // If enable-crash-reporter argv is undefined then this is a fresh start, // based on telemetry.enableCrashreporter settings, generate a UUID which diff --git a/src/vs/workbench/contrib/files/browser/editors/binaryFileEditor.ts b/src/vs/workbench/contrib/files/browser/editors/binaryFileEditor.ts index 7eed77c03ab..ff84a3b5905 100644 --- a/src/vs/workbench/contrib/files/browser/editors/binaryFileEditor.ts +++ b/src/vs/workbench/contrib/files/browser/editors/binaryFileEditor.ts @@ -49,13 +49,14 @@ export class BinaryFileEditor extends BaseBinaryResourceEditor { // Try to let the user pick an override if there is one availabe const overridenInput = await this.editorOverrideService.resolveEditorOverride(input, { ...options, override: EditorOverride.PICK, }, this.group); - let newOptions = overridenInput?.options ?? options; - newOptions = { ...newOptions, override: EditorOverride.DISABLED }; // Replace the overrriden input, with the text based input await this.editorService.replaceEditors([{ editor: input, replacement: overridenInput?.editor ?? input, - options: newOptions, + options: { + ...overridenInput?.options ?? options, + override: EditorOverride.DISABLED + } }], overridenInput?.group ?? this.group); } } diff --git a/src/vs/workbench/contrib/performance/browser/perfviewEditor.ts b/src/vs/workbench/contrib/performance/browser/perfviewEditor.ts index b30f6a2f554..b5e17cc2fe2 100644 --- a/src/vs/workbench/contrib/performance/browser/perfviewEditor.ts +++ b/src/vs/workbench/contrib/performance/browser/perfviewEditor.ts @@ -176,7 +176,6 @@ class PerfModelContentProvider implements ITextModelContentProvider { table.push(['window.loadUrl() => begin to require(workbench.desktop.main.js)', metrics.timers.ellapsedWindowLoadToRequire, '[main->renderer]', StartupKindToString(metrics.windowKind)]); table.push(['require(workbench.desktop.main.js)', metrics.timers.ellapsedRequire, '[renderer]', `cached data: ${(metrics.didUseCachedData ? 'YES' : 'NO')}${stats ? `, node_modules took ${stats.nodeRequireTotal}ms` : ''}`]); table.push(['wait for window config', metrics.timers.ellapsedWaitForWindowConfig, '[renderer]', undefined]); - table.push(['wait for shell environment', metrics.timers.ellapsedWaitForShellEnv, '[renderer]', undefined]); table.push(['init storage (global & workspace)', metrics.timers.ellapsedStorageInit, '[renderer]', undefined]); table.push(['init workspace service', metrics.timers.ellapsedWorkspaceServiceInit, '[renderer]', undefined]); if (isWeb) { diff --git a/src/vs/workbench/electron-sandbox/window.ts b/src/vs/workbench/electron-sandbox/window.ts index d4e4e0ed71f..23c3f609359 100644 --- a/src/vs/workbench/electron-sandbox/window.ts +++ b/src/vs/workbench/electron-sandbox/window.ts @@ -32,7 +32,7 @@ import { IWorkspaceFolderCreationData } from 'vs/platform/workspaces/common/work import { IIntegrityService } from 'vs/workbench/services/integrity/common/integrity'; import { isWindows, isMacintosh } from 'vs/base/common/platform'; import { IProductService } from 'vs/platform/product/common/productService'; -import { INotificationService, IPromptChoice, NeverShowAgainScope, Severity } from 'vs/platform/notification/common/notification'; +import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { INativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-sandbox/environmentService'; import { IAccessibilityService, AccessibilitySupport } from 'vs/platform/accessibility/common/accessibility'; @@ -186,28 +186,6 @@ export class NativeWindow extends Disposable { // Message support ipcRenderer.on('vscode:showInfoMessage', (event: unknown, message: string) => this.notificationService.info(message)); - // Shell Environment Issue Notifications - const choices: IPromptChoice[] = [{ - label: localize('learnMore', "Learn More"), - run: () => this.openerService.open('https://go.microsoft.com/fwlink/?linkid=2149667') - }]; - - ipcRenderer.on('vscode:showShellEnvSlowWarning', () => this.notificationService.prompt( - Severity.Warning, - localize('shellEnvSlowWarning', "Resolving your shell environment is taking very long. Please review your shell configuration."), - choices, - { - sticky: true, - neverShowAgain: { id: 'ignoreShellEnvSlowWarning', scope: NeverShowAgainScope.GLOBAL } - } - )); - - ipcRenderer.on('vscode:showShellEnvTimeoutError', () => this.notificationService.prompt( - Severity.Error, - localize('shellEnvTimeoutError', "Unable to resolve your shell environment in a reasonable time. Please review your shell configuration."), - choices - )); - // Fullscreen Events ipcRenderer.on('vscode:enterFullScreen', async () => { await this.lifecycleService.when(LifecyclePhase.Ready); diff --git a/src/vs/workbench/services/timer/browser/timerService.ts b/src/vs/workbench/services/timer/browser/timerService.ts index 27eb526dd99..dc5904fcaeb 100644 --- a/src/vs/workbench/services/timer/browser/timerService.ts +++ b/src/vs/workbench/services/timer/browser/timerService.ts @@ -50,7 +50,6 @@ export interface IMemoryInfo { "timers.ellapsedWindowLoad" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, "timers.ellapsedWindowLoadToRequire" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, "timers.ellapsedWaitForWindowConfig" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, - "timers.ellapsedWaitForShellEnv" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, "timers.ellapsedStorageInit" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, "timers.ellapsedWorkspaceServiceInit" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, "timers.ellapsedSharedProcesConnected" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true }, @@ -262,15 +261,6 @@ export interface IStartupMetrics { readonly ellapsedWaitForWindowConfig: number; /** - * The time it took to wait for resolving the shell environment. This time the workbench - * will not continue to load and be blocked entirely. - * - * * Happens in the renderer-process - * * Measured with the `willWaitForShellEnv` and `didWaitForShellEnv` performance marks. - */ - readonly ellapsedWaitForShellEnv: number; - - /** * The time it took to init the storage database connection from the workbench. * * * Happens in the renderer-process @@ -593,7 +583,6 @@ export abstract class AbstractTimerService implements ITimerService { ellapsedWindowLoadToRequire: this._marks.getDuration('code/willOpenNewWindow', 'code/willLoadWorkbenchMain'), ellapsedRequire: this._marks.getDuration('code/willLoadWorkbenchMain', 'code/didLoadWorkbenchMain'), ellapsedWaitForWindowConfig: this._marks.getDuration('code/willWaitForWindowConfig', 'code/didWaitForWindowConfig'), - ellapsedWaitForShellEnv: this._marks.getDuration('code/willWaitForShellEnv', 'code/didWaitForShellEnv'), ellapsedStorageInit: this._marks.getDuration('code/willInitStorage', 'code/didInitStorage'), ellapsedSharedProcesConnected: this._marks.getDuration('code/willConnectSharedProcess', 'code/didConnectSharedProcess'), ellapsedWorkspaceServiceInit: this._marks.getDuration('code/willInitWorkspaceService', 'code/didInitWorkspaceService'), |