atomic ext host restart (fix microsoft#78609)

Benjamin Pasero · Benjamin Pasero · commit a383dc060762 · 2019-08-07T11:03:57.000+02:00
diff --git a/src/vs/workbench/contrib/relauncher/electron-browser/relauncher.contribution.ts b/src/vs/workbench/contrib/relauncher/electron-browser/relauncher.contribution.ts
@@ -167,8 +167,9 @@ export class WorkspaceChangeExtHostRelauncher extends Disposable implements IWor
 			if (!!environmentService.extensionTestsLocationURI) {
 				return; // no restart when in tests: see https://github.com/Microsoft/vscode/issues/66936
 			}
+
 			if (environmentService.configuration.remoteAuthority) {
-				windowSevice.reloadWindow(); // TODO aeschli, workaround
+				windowSevice.reloadWindow(); // TODO@aeschli, workaround
 			} else {
 				extensionService.restartExtensionHost();
 			}
diff --git a/src/vs/workbench/services/extensions/common/abstractExtensionService.ts b/src/vs/workbench/services/extensions/common/abstractExtensionService.ts
@@ -170,14 +170,10 @@ export abstract class AbstractExtensionService extends Disposable implements IEx
 		this._startExtensionHostProcess(false, Array.from(this._allRequestedActivateEvents.keys()));
 	}
 
-	public startExtensionHost(): void {
+	protected startExtensionHost(): void {
 		this._startExtensionHostProcess(false, Array.from(this._allRequestedActivateEvents.keys()));
 	}
 
-	public stopExtensionHost(): void {
-		this._stopExtensionHostProcess();
-	}
-
 	public activateByEvent(activationEvent: string): Promise<void> {
 		if (this._installedExtensionsReady.isOpen()) {
 			// Extensions have been scanned and interpreted
diff --git a/src/vs/workbench/services/extensions/common/extensions.ts b/src/vs/workbench/services/extensions/common/extensions.ts
@@ -219,16 +219,6 @@ export interface IExtensionService {
 	 */
 	restartExtensionHost(): void;
 
-	/**
-	 * Starts the extension host.
-	 */
-	startExtensionHost(): void;
-
-	/**
-	 * Stops the extension host.
-	 */
-	stopExtensionHost(): void;
-
 	/**
 	 * Modify the environment of the remote extension host
 	 * @param env New properties for the remote extension host
@@ -282,8 +272,6 @@ export class NullExtensionService implements IExtensionService {
 	getExtensionsStatus(): { [id: string]: IExtensionsStatus; } { return Object.create(null); }
 	getInspectPort(): number { return 0; }
 	restartExtensionHost(): void { }
-	startExtensionHost(): void { }
-	stopExtensionHost(): void { }
 	async setRemoteEnvironment(_env: { [key: string]: string | null }): Promise<void> { }
 	canAddExtension(): boolean { return false; }
 	canRemoveExtension(): boolean { return false; }
diff --git a/src/vs/workbench/services/workspace/electron-browser/workspaceEditingService.ts b/src/vs/workbench/services/workspace/electron-browser/workspaceEditingService.ts
@@ -220,6 +220,7 @@ export class WorkspaceEditingService implements IWorkspaceEditingService {
 	private async doAddFolders(foldersToAdd: IWorkspaceFolderCreationData[], index?: number, donotNotifyError: boolean = false): Promise<void> {
 		const state = this.contextService.getWorkbenchState();
 		if (this.environmentService.configuration.remoteAuthority) {
+
 			// Do not allow workspace folders with scheme different than the current remote scheme
 			const schemas = this.contextService.getWorkspace().folders.map(f => f.uri.scheme);
 			if (schemas.length && foldersToAdd.some(f => schemas.indexOf(f.uri.scheme) === -1)) {
@@ -286,31 +287,34 @@ export class WorkspaceEditingService implements IWorkspaceEditingService {
 		if (path && !await this.isValidTargetWorkspacePath(path)) {
 			return;
 		}
+
 		const remoteAuthority = this.environmentService.configuration.remoteAuthority;
 		const untitledWorkspace = await this.workspaceService.createUntitledWorkspace(folders, remoteAuthority);
 		if (path) {
 			await this.saveWorkspaceAs(untitledWorkspace, path);
 		} else {
 			path = untitledWorkspace.configPath;
 		}
+
 		return this.enterWorkspace(path);
 	}
 
 	async saveAndEnterWorkspace(path: URI): Promise<void> {
 		if (!await this.isValidTargetWorkspacePath(path)) {
 			return;
 		}
+
 		const workspaceIdentifier = this.getCurrentWorkspaceIdentifier();
 		if (!workspaceIdentifier) {
 			return;
 		}
+
 		await this.saveWorkspaceAs(workspaceIdentifier, path);
 
 		return this.enterWorkspace(path);
 	}
 
 	async isValidTargetWorkspacePath(path: URI): Promise<boolean> {
-
 		const windows = await this.windowsService.getWindows();
 
 		// Prevent overwriting a workspace that is currently opened in another window
@@ -382,30 +386,34 @@ export class WorkspaceEditingService implements IWorkspaceEditingService {
 		}
 
 		const workspace = await this.workspaceService.getWorkspaceIdentifier(path);
+
 		// Settings migration (only if we come from a folder workspace)
 		if (this.contextService.getWorkbenchState() === WorkbenchState.FOLDER) {
 			await this.migrateWorkspaceSettings(workspace);
 		}
+
 		const workspaceImpl = this.contextService as WorkspaceService;
 		await workspaceImpl.initialize(workspace);
 
-		// Restart extension host if first root folder changed (impact on deprecated workspace.rootPath API)
-		// Stop the extension host first to give extensions most time to shutdown
-		this.extensionService.stopExtensionHost();
-
 		const result = await this.windowService.enterWorkspace(path);
 		if (result) {
 			await this.migrateStorage(result.workspace);
+
 			// Reinitialize backup service
 			if (this.backupFileService instanceof BackupFileService) {
 				this.backupFileService.initialize(toBackupWorkspaceResource(result.backupPath!, this.environmentService));
 			}
 		}
 
+		// TODO@aeschli: workaround until restarting works
 		if (this.environmentService.configuration.remoteAuthority) {
-			this.windowService.reloadWindow(); // TODO aeschli: workaround until restarting works
-		} else {
-			this.extensionService.startExtensionHost();
+			this.windowService.reloadWindow();
+		}
+
+		// Restart the extension host: entering a workspace means a new location for
+		// storage and potentially a change in the workspace.rootPath property.
+		else {
+			this.extensionService.restartExtensionHost();
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -167,8 +167,9 @@ export class WorkspaceChangeExtHostRelauncher extends Disposable implements IWor`
`167`	`167`	`if (!!environmentService.extensionTestsLocationURI) {`
`168`	`168`	`return; // no restart when in tests: see https://github.com/Microsoft/vscode/issues/66936`
`169`	`169`	`}`
	`170`	`+`
`170`	`171`	`if (environmentService.configuration.remoteAuthority) {`
`171`		`- windowSevice.reloadWindow(); // TODO aeschli, workaround`
	`172`	`+ windowSevice.reloadWindow(); // TODO@aeschli, workaround`
`172`	`173`	`} else {`
`173`	`174`	`extensionService.restartExtensionHost();`
`174`	`175`	`}`
Original file line number	Diff line number	Diff line change
`@@ -170,14 +170,10 @@ export abstract class AbstractExtensionService extends Disposable implements IEx`
`170`	`170`	`this._startExtensionHostProcess(false, Array.from(this._allRequestedActivateEvents.keys()));`
`171`	`171`	`}`
`172`	`172`
`173`		`- public startExtensionHost(): void {`
	`173`	`+ protected startExtensionHost(): void {`
`174`	`174`	`this._startExtensionHostProcess(false, Array.from(this._allRequestedActivateEvents.keys()));`
`175`	`175`	`}`
`176`	`176`
`177`		`- public stopExtensionHost(): void {`
`178`		`- this._stopExtensionHostProcess();`
`179`		`- }`
`180`		`-`
`181`	`177`	`public activateByEvent(activationEvent: string): Promise<void> {`
`182`	`178`	`if (this._installedExtensionsReady.isOpen()) {`
`183`	`179`	`// Extensions have been scanned and interpreted`