Remove shell environment patching in the renderer (fix #108804)

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'),