Merge pull request #143999 from microsoft/ben/march

March debt changes

package.json

@@ -96,7 +96,7 @@
   },
   "devDependencies": {
     "7zip": "0.0.6",
-    "@playwright/test": "1.18.0",
+    "@playwright/test": "1.19.2",
     "@types/applicationinsights": "0.20.0",
     "@types/cookie": "^0.3.3",
     "@types/copy-webpack-plugin": "^6.0.3",

src/vs/base/parts/storage/common/storage.ts

@@ -74,7 +74,7 @@ export interface IStorage extends IDisposable {
 	close(): Promise<void>;
 }
 
-enum StorageState {
+export enum StorageState {
 	None,
 	Initialized,
 	Closed

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

@@ -60,7 +60,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle
 import { IIssueMainService, IssueMainService } from 'vs/platform/issue/electron-main/issueMainService';
 import { IKeyboardLayoutMainService, KeyboardLayoutMainService } from 'vs/platform/keyboardLayout/electron-main/keyboardLayoutMainService';
 import { ILaunchMainService, LaunchMainService } from 'vs/platform/launch/electron-main/launchMainService';
-import { ILifecycleMainService, LifecycleMainPhase } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { ILifecycleMainService, LifecycleMainPhase, ShutdownReason } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
 import { ILoggerService, ILogService } from 'vs/platform/log/common/log';
 import { LoggerChannel, LogLevelChannel } from 'vs/platform/log/common/logIpc';
 import { IMenubarMainService, MenubarMainService } from 'vs/platform/menubar/electron-main/menubarMainService';
@@ -471,6 +471,16 @@ export class CodeApplication extends Disposable {
 
 		// Main process server (electron IPC based)
 		const mainProcessElectronServer = new ElectronIPCServer();
+		this.lifecycleMainService.onWillShutdown(e => {
+			if (e.reason === ShutdownReason.KILL) {
+				// When we go down abnormally, make sure to free up
+				// any IPC we accept from other windows to reduce
+				// the chance of doing work after we go down. Kill
+				// is special in that it does not orderly shutdown
+				// windows.
+				mainProcessElectronServer.dispose();
+			}
+		});
 
 		// Resolve unique machine ID
 		this.logService.trace('Resolving machine identifier...');

src/vs/platform/lifecycle/electron-main/lifecycleMainService.ts

@@ -12,7 +12,6 @@ import { cwd } from 'vs/base/common/process';
 import { assertIsDefined } from 'vs/base/common/types';
 import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { handleVetos } from 'vs/platform/lifecycle/common/lifecycle';
 import { ILogService } from 'vs/platform/log/common/log';
 import { IStateMainService } from 'vs/platform/state/electron-main/state';
 import { ICodeWindow, LoadReason, UnloadReason } from 'vs/platform/window/electron-main/window';
@@ -38,25 +37,6 @@ interface WindowLoadEvent {
 	reason: LoadReason;
 }
 
-interface WindowUnloadEvent {
-
-	/**
-	 * The window that is unloading.
-	 */
-	window: ICodeWindow;
-
-	/**
-	 * More details why the window is unloading.
-	 */
-	reason: UnloadReason;
-
-	/**
-	 * A way to join the unloading of the window and optionally
-	 * veto the unload to finish.
-	 */
-	veto(value: boolean | Promise<boolean>): void;
-}
-
 export const enum ShutdownReason {
 
 	/**
@@ -124,12 +104,6 @@ export interface ILifecycleMainService {
 	 */
 	readonly onWillLoadWindow: Event<WindowLoadEvent>;
 
-	/**
-	 * An event that fires before a window is about to unload. Listeners can veto this event to prevent
-	 * the window from unloading.
-	 */
-	readonly onBeforeUnloadWindow: Event<WindowUnloadEvent>;
-
 	/**
 	 * An event that fires before a window closes. This event is fired after any veto has been dealt
 	 * with so that listeners know for sure that the window will close without veto.
@@ -220,9 +194,6 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 	private readonly _onBeforeCloseWindow = this._register(new Emitter<ICodeWindow>());
 	readonly onBeforeCloseWindow = this._onBeforeCloseWindow.event;
 
-	private readonly _onBeforeUnloadWindow = this._register(new Emitter<WindowUnloadEvent>());
-	readonly onBeforeUnloadWindow = this._onBeforeUnloadWindow.event;
-
 	private _quitRequested = false;
 	get quitRequested(): boolean { return this._quitRequested; }
 
@@ -241,6 +212,8 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 
 	private pendingWillShutdownPromise: Promise<void> | undefined = undefined;
 
+	private readonly mapWindowIdToPendingUnload = new Map<number, Promise<boolean>>();
+
 	private readonly phaseWhen = new Map<LifecycleMainPhase, Barrier>();
 
 	constructor(
@@ -469,7 +442,24 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 		}
 	}
 
-	async unload(window: ICodeWindow, reason: UnloadReason): Promise<boolean /* veto */> {
+	unload(window: ICodeWindow, reason: UnloadReason): Promise<boolean /* veto */> {
+
+		// Ensure there is only 1 unload running at the same time
+		const pendingUnloadPromise = this.mapWindowIdToPendingUnload.get(window.id);
+		if (pendingUnloadPromise) {
+			return pendingUnloadPromise;
+		}
+
+		// Start unload and remember in map until finished
+		const unloadPromise = this.doUnload(window, reason).finally(() => {
+			this.mapWindowIdToPendingUnload.delete(window.id);
+		});
+		this.mapWindowIdToPendingUnload.set(window.id, unloadPromise);
+
+		return unloadPromise;
+	}
+
+	private async doUnload(window: ICodeWindow, reason: UnloadReason): Promise<boolean /* veto */> {
 
 		// Always allow to unload a window that is not yet ready
 		if (!window.isReady) {
@@ -487,16 +477,6 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 			return this.handleWindowUnloadVeto(veto);
 		}
 
-		// then check for vetos in the main side
-		veto = await this.onBeforeUnloadWindowInMain(window, windowUnloadReason);
-		if (veto) {
-			this.logService.trace(`Lifecycle#unload() - veto in main (window ID ${window.id})`);
-
-			return this.handleWindowUnloadVeto(veto);
-		}
-
-		this.logService.trace(`Lifecycle#unload() - no veto (window ID ${window.id})`);
-
 		// finally if there are no vetos, unload the renderer
 		await this.onWillUnloadWindowInRenderer(window, windowUnloadReason);
 
@@ -543,20 +523,6 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 		});
 	}
 
-	private onBeforeUnloadWindowInMain(window: ICodeWindow, reason: UnloadReason): Promise<boolean /* veto */> {
-		const vetos: (boolean | Promise<boolean>)[] = [];
-
-		this._onBeforeUnloadWindow.fire({
-			reason,
-			window,
-			veto(value) {
-				vetos.push(value);
-			}
-		});
-
-		return handleVetos(vetos, err => this.logService.error(err));
-	}
-
 	private onWillUnloadWindowInRenderer(window: ICodeWindow, reason: UnloadReason): Promise<void> {
 		return new Promise<void>(resolve => {
 			const oneTimeEventToken = this.oneTimeListenerTokenGenerator++;
@@ -633,8 +599,8 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 		};
 		app.once('quit', quitListener);
 
-		// app.relaunch() does not quit automatically, so we quit first,
-		// check for vetoes and then relaunch from the app.on('quit') event
+		// `app.relaunch()` does not quit automatically, so we quit first,
+		// check for vetoes and then relaunch from the `app.on('quit')` event
 		const veto = await this.quit(true /* will restart */);
 		if (veto) {
 			app.removeListener('quit', quitListener);
@@ -657,10 +623,13 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 
 		await Promise.race([
 
-			// still do not block more than 1s
+			// Still do not block more than 1s
 			timeout(1000),
 
-			// destroy any opened window
+			// Destroy any opened window: we do not unload windows here because
+			// there is a chance that the unload is veto'd or long running due
+			// to a participant within the window. this is not wanted when we
+			// are asked to kill the application.
 			(async () => {
 				for (const window of BrowserWindow.getAllWindows()) {
 					if (window && !window.isDestroyed()) {

src/vs/platform/storage/electron-main/storageMain.ts

@@ -8,7 +8,7 @@ import { Emitter, Event } from 'vs/base/common/event';
 import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
 import { join } from 'vs/base/common/path';
 import { Promises } from 'vs/base/node/pfs';
-import { InMemoryStorageDatabase, IStorage, Storage, StorageHint } from 'vs/base/parts/storage/common/storage';
+import { InMemoryStorageDatabase, IStorage, Storage, StorageHint, StorageState } from 'vs/base/parts/storage/common/storage';
 import { ISQLiteStorageDatabaseLoggingOptions, SQLiteStorageDatabase } from 'vs/base/parts/storage/node/storage';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { ILogService, LogLevel } from 'vs/platform/log/common/log';
@@ -107,6 +107,8 @@ abstract class BaseStorageMain extends Disposable implements IStorageMain {
 	private readonly whenInitPromise = new DeferredPromise<void>();
 	readonly whenInit = this.whenInitPromise.p;
 
+	private state = StorageState.None;
+
 	constructor(
 		protected readonly logService: ILogService
 	) {
@@ -116,6 +118,10 @@ abstract class BaseStorageMain extends Disposable implements IStorageMain {
 	init(): Promise<void> {
 		if (!this.initializePromise) {
 			this.initializePromise = (async () => {
+				if (this.state !== StorageState.None) {
+					return; // either closed or already initialized
+				}
+
 				try {
 
 					// Create storage via subclasses
@@ -143,6 +149,11 @@ abstract class BaseStorageMain extends Disposable implements IStorageMain {
 				} catch (error) {
 					this.logService.error(`StorageMain#initialize(): Unable to init storage due to ${error}`);
 				} finally {
+
+					// Update state
+					this.state = StorageState.Initialized;
+
+					// Mark init promise as completed
 					this.whenInitPromise.complete();
 				}
 			})();
@@ -184,12 +195,15 @@ abstract class BaseStorageMain extends Disposable implements IStorageMain {
 
 		// Ensure we are not accidentally leaving
 		// a pending initialized storage behind in
-		// case close() was called before init()
-		// finishes
+		// case `close()` was called before `init()`
+		// finishes.
 		if (this.initializePromise) {
 			await this.initializePromise;
 		}
 
+		// Update state
+		this.state = StorageState.Closed;
+
 		// Propagate to storage lib
 		await this._storage.close();
 
@@ -316,10 +330,3 @@ export class WorkspaceStorageMain extends BaseStorageMain implements IStorageMai
 		}
 	}
 }
-
-export class InMemoryStorageMain extends BaseStorageMain {
-
-	protected async doCreate(): Promise<IStorage> {
-		return new Storage(new InMemoryStorageDatabase());
-	}
-}

src/vs/platform/storage/electron-main/storageMainService.ts

@@ -9,10 +9,10 @@ import { IStorage } from 'vs/base/parts/storage/common/storage';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { ILifecycleMainService, LifecycleMainPhase, ShutdownReason } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { ILifecycleMainService, LifecycleMainPhase } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
 import { ILogService } from 'vs/platform/log/common/log';
 import { AbstractStorageService, IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
-import { GlobalStorageMain, InMemoryStorageMain, IStorageMain, IStorageMainOptions, WorkspaceStorageMain } from 'vs/platform/storage/electron-main/storageMain';
+import { GlobalStorageMain, IStorageMain, IStorageMainOptions, WorkspaceStorageMain } from 'vs/platform/storage/electron-main/storageMain';
 import { IAnyWorkspaceIdentifier, IEmptyWorkspaceIdentifier, ISingleFolderWorkspaceIdentifier, IWorkspaceIdentifier } from 'vs/platform/workspace/common/workspace';
 
 //#region Storage Main Service (intent: make global and workspace storage accessible to windows from main process)
@@ -44,8 +44,6 @@ export class StorageMainService extends Disposable implements IStorageMainServic
 
 	declare readonly _serviceBrand: undefined;
 
-	private shutdownReason: ShutdownReason | undefined = undefined;
-
 	constructor(
 		@ILogService private readonly logService: ILogService,
 		@IEnvironmentService private readonly environmentService: IEnvironmentService,
@@ -82,9 +80,6 @@ export class StorageMainService extends Disposable implements IStorageMainServic
 		this._register(this.lifecycleMainService.onWillShutdown(e => {
 			this.logService.trace('storageMainService#onWillShutdown()');
 
-			// Remember shutdown reason
-			this.shutdownReason = e.reason;
-
 			// Global Storage
 			e.join(this.globalStorage.close());
 
@@ -137,14 +132,9 @@ export class StorageMainService extends Disposable implements IStorageMainServic
 	}
 
 	private createWorkspaceStorage(workspace: IWorkspaceIdentifier | ISingleFolderWorkspaceIdentifier | IEmptyWorkspaceIdentifier): IStorageMain {
-		if (this.shutdownReason === ShutdownReason.KILL) {
-			// Workaround for native crashes that we see when
-			// SQLite DBs are being created even after shutdown
-			// https://github.com/microsoft/vscode/issues/143186
-			return new InMemoryStorageMain(this.logService);
-		}
+		const workspaceStorage = new WorkspaceStorageMain(workspace, this.getStorageOptions(), this.logService, this.environmentService);
 
-		return new WorkspaceStorageMain(workspace, this.getStorageOptions(), this.logService, this.environmentService);
+		return workspaceStorage;
 	}
 
 	//#endregion

src/vs/platform/storage/test/electron-main/storageMainService.test.ts

@@ -57,7 +57,6 @@ suite('StorageMainService', function () {
 
 		onWillLoadWindow = Event.None;
 		onBeforeCloseWindow = Event.None;
-		onBeforeUnloadWindow = Event.None;
 
 		wasRestarted = false;
 		quitRequested = false;

src/vs/platform/workspaces/common/workspaces.ts

@@ -27,10 +27,10 @@ export interface IWorkspacesService {
 	readonly _serviceBrand: undefined;
 
 	// Workspaces Management
-	enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined>;
+	enterWorkspace(workspaceUri: URI): Promise<IEnterWorkspaceResult | undefined>;
 	createUntitledWorkspace(folders?: IWorkspaceFolderCreationData[], remoteAuthority?: string): Promise<IWorkspaceIdentifier>;
 	deleteUntitledWorkspace(workspace: IWorkspaceIdentifier): Promise<void>;
-	getWorkspaceIdentifier(workspacePath: URI): Promise<IWorkspaceIdentifier>;
+	getWorkspaceIdentifier(workspaceUri: URI): Promise<IWorkspaceIdentifier>;
 
 	// Workspaces History
 	readonly onDidChangeRecentlyOpened: Event<void>;

src/vs/workbench/browser/actions/workspaceCommands.ts

@@ -6,7 +6,7 @@
 import { localize } from 'vs/nls';
 import { hasWorkspaceFileExtension, IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { IWorkspaceEditingService } from 'vs/workbench/services/workspaces/common/workspaceEditing';
-import { dirname, removeTrailingPathSeparator } from 'vs/base/common/resources';
+import { dirname } from 'vs/base/common/resources';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { mnemonicButtonLabel } from 'vs/base/common/labels';
 import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands';
@@ -69,7 +69,7 @@ CommandsRegistry.registerCommand({
 			return;
 		}
 
-		await workspaceEditingService.addFolders(folders.map(folder => ({ uri: removeTrailingPathSeparator(folder) })));
+		await workspaceEditingService.addFolders(folders.map(folder => ({ uri: folder })));
 	}
 });
 
@@ -84,7 +84,7 @@ CommandsRegistry.registerCommand({
 			return;
 		}
 
-		await workspaceEditingService.updateFolders(0, contextService.getWorkspace().folders.length, folders.map(folder => ({ uri: removeTrailingPathSeparator(folder) })));
+		await workspaceEditingService.updateFolders(0, contextService.getWorkspace().folders.length, folders.map(folder => ({ uri: folder })));
 	}
 });