backup - cancel pending operations on shutdown and handle cancellation better (#138055)

src/vs/workbench/services/workingCopy/common/workingCopyBackup.ts

@@ -71,7 +71,7 @@ export interface IWorkingCopyBackupService {
 	/**
 	 * Discards the working copy backup associated with the identifier if it exists.
 	 */
-	discardBackup(identifier: IWorkingCopyIdentifier): Promise<void>;
+	discardBackup(identifier: IWorkingCopyIdentifier, token?: CancellationToken): Promise<void>;
 
 	/**
 	 * Discards all working copy backups.

src/vs/workbench/services/workingCopy/common/workingCopyBackupService.ts

@@ -170,8 +170,8 @@ export abstract class WorkingCopyBackupService implements IWorkingCopyBackupServ
 		return this.impl.backup(identifier, content, versionId, meta, token);
 	}
 
-	discardBackup(identifier: IWorkingCopyIdentifier): Promise<void> {
-		return this.impl.discardBackup(identifier);
+	discardBackup(identifier: IWorkingCopyIdentifier, token?: CancellationToken): Promise<void> {
+		return this.impl.discardBackup(identifier, token);
 	}
 
 	discardBackups(filter?: { except: IWorkingCopyIdentifier[] }): Promise<void> {
@@ -322,7 +322,12 @@ class WorkingCopyBackupServiceImpl extends Disposable implements IWorkingCopyBac
 			// Write backup via file service
 			await this.fileService.writeFile(backupResource, backupBuffer);
 
+			//
 			// Update model
+			//
+			// Note: not checking for cancellation here because a successful
+			// write into the backup file should be noted in the model to
+			// prevent the model being out of sync with the backup file
 			model.add(backupResource, versionId, meta);
 		});
 	}
@@ -357,18 +362,32 @@ class WorkingCopyBackupServiceImpl extends Disposable implements IWorkingCopyBac
 		}
 	}
 
-	discardBackup(identifier: IWorkingCopyIdentifier): Promise<void> {
+	discardBackup(identifier: IWorkingCopyIdentifier, token?: CancellationToken): Promise<void> {
 		const backupResource = this.toBackupResource(identifier);
 
-		return this.doDiscardBackup(backupResource);
+		return this.doDiscardBackup(backupResource, token);
 	}
 
-	private async doDiscardBackup(backupResource: URI): Promise<void> {
+	private async doDiscardBackup(backupResource: URI, token?: CancellationToken): Promise<void> {
 		const model = await this.ready;
+		if (token?.isCancellationRequested) {
+			return;
+		}
 
 		return this.ioOperationQueues.queueFor(backupResource).queue(async () => {
+			if (token?.isCancellationRequested) {
+				return;
+			}
+
+			// Delete backup file ignoring any file not found errors
 			await this.deleteIgnoreFileNotFound(backupResource);
 
+			//
+			// Update model
+			//
+			// Note: not checking for cancellation here because a successful
+			// delete of the backup file should be noted in the model to
+			// prevent the model being out of sync with the backup file
 			model.remove(backupResource);
 		});
 	}

src/vs/workbench/services/workingCopy/common/workingCopyBackupTracker.ts

@@ -41,7 +41,9 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
 		super();
 
 		// Fill in initial dirty working copies
-		this.workingCopyService.dirtyWorkingCopies.forEach(workingCopy => this.onDidRegister(workingCopy));
+		for (const workingCopy of this.workingCopyService.dirtyWorkingCopies) {
+			this.onDidRegister(workingCopy);
+		}
 
 		this.registerListeners();
 	}
@@ -69,8 +71,8 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
 	// content has been made before closing.
 	private readonly mapWorkingCopyToContentVersion = new Map<IWorkingCopy, number>();
 
-	// A map of scheduled pending backups for working copies
-	protected readonly pendingBackups = new Map<IWorkingCopy, IDisposable>();
+	// A map of scheduled pending backup operations for working copies
+	protected readonly pendingBackupOperations = new Map<IWorkingCopy, IDisposable>();
 
 	// Delay creation of backups when content changes to avoid too much
 	// load on the backup service when the user is typing into the editor
@@ -127,7 +129,7 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
 	private scheduleBackup(workingCopy: IWorkingCopy): void {
 
 		// Clear any running backup operation
-		this.cancelBackup(workingCopy);
+		this.cancelBackupOperation(workingCopy);
 
 		this.logService.trace(`[backup tracker] scheduling backup`, workingCopy.resource.toString(true), workingCopy.typeId);
 
@@ -158,18 +160,16 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
 				}
 			}
 
-			if (cts.token.isCancellationRequested) {
-				return;
+			// Clear disposable unless we got canceled which would
+			// indicate another operation has started meanwhile
+			if (!cts.token.isCancellationRequested) {
+				this.pendingBackupOperations.delete(workingCopy);
 			}
-
-			// Clear disposable
-			this.pendingBackups.delete(workingCopy);
-
 		}, this.getBackupScheduleDelay(workingCopy));
 
 		// Keep in map for disposal as needed
-		this.pendingBackups.set(workingCopy, toDisposable(() => {
-			this.logService.trace(`[backup tracker] clearing pending backup`, workingCopy.resource.toString(true), workingCopy.typeId);
+		this.pendingBackupOperations.set(workingCopy, toDisposable(() => {
+			this.logService.trace(`[backup tracker] clearing pending backup creation`, workingCopy.resource.toString(true), workingCopy.typeId);
 
 			cts.dispose(true);
 			clearTimeout(handle);
@@ -190,18 +190,48 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
 	}
 
 	private discardBackup(workingCopy: IWorkingCopy): void {
-		this.logService.trace(`[backup tracker] discarding backup`, workingCopy.resource.toString(true), workingCopy.typeId);
 
 		// Clear any running backup operation
-		this.cancelBackup(workingCopy);
+		this.cancelBackupOperation(workingCopy);
 
-		// Forward to working copy backup service
-		this.workingCopyBackupService.discardBackup(workingCopy);
+		// Schedule backup discard asap
+		const cts = new CancellationTokenSource();
+		(async () => {
+			this.logService.trace(`[backup tracker] discarding backup`, workingCopy.resource.toString(true), workingCopy.typeId);
+
+			// Discard backup
+			try {
+				await this.workingCopyBackupService.discardBackup(workingCopy, cts.token);
+			} catch (error) {
+				this.logService.error(error);
+			}
+
+			// Clear disposable unless we got canceled which would
+			// indicate another operation has started meanwhile
+			if (!cts.token.isCancellationRequested) {
+				this.pendingBackupOperations.delete(workingCopy);
+			}
+		})();
+
+		// Keep in map for disposal as needed
+		this.pendingBackupOperations.set(workingCopy, toDisposable(() => {
+			this.logService.trace(`[backup tracker] clearing pending backup discard`, workingCopy.resource.toString(true), workingCopy.typeId);
+
+			cts.dispose(true);
+		}));
 	}
 
-	private cancelBackup(workingCopy: IWorkingCopy): void {
-		dispose(this.pendingBackups.get(workingCopy));
-		this.pendingBackups.delete(workingCopy);
+	private cancelBackupOperation(workingCopy: IWorkingCopy): void {
+		dispose(this.pendingBackupOperations.get(workingCopy));
+		this.pendingBackupOperations.delete(workingCopy);
+	}
+
+	protected cancelBackupOperations(): void {
+		for (const [, disposable] of this.pendingBackupOperations) {
+			dispose(disposable);
+		}
+
+		this.pendingBackupOperations.clear();
 	}
 
 	protected abstract onBeforeShutdown(reason: ShutdownReason): boolean | Promise<boolean>;

src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts

@@ -50,6 +50,14 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 
 	protected onBeforeShutdown(reason: ShutdownReason): boolean | Promise<boolean> {
 
+		// Important: we are about to shutdown and handle dirty working copies
+		// and backups. We do not want any pending backup ops to interfer with
+		// this because there is a risk of a backup being scheduled after we have
+		// acknowledged to shutdown and then might end up with partial backups
+		// written to disk, or even empty backups or deletes after writes.
+		// (https://github.com/microsoft/vscode/issues/138055)
+		this.cancelBackupOperations();
+
 		// Dirty working copies need treatment on shutdown
 		const dirtyWorkingCopies = this.workingCopyService.dirtyWorkingCopies;
 		if (dirtyWorkingCopies.length) {
@@ -88,12 +96,13 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 
 	private async handleDirtyBeforeShutdown(dirtyWorkingCopies: readonly IWorkingCopy[], reason: ShutdownReason): Promise<boolean> {
 
-		// Trigger backup if configured
+		// Trigger backup if configured and enabled for shutdown reason
 		let backups: IWorkingCopy[] = [];
 		let backupError: Error | undefined = undefined;
-		if (this.filesConfigurationService.isHotExitEnabled) {
+		const backup = await this.shouldBackupBeforeShutdown(reason);
+		if (backup) {
 			try {
-				const backupResult = await this.backupBeforeShutdown(dirtyWorkingCopies, reason);
+				const backupResult = await this.backupBeforeShutdown(dirtyWorkingCopies);
 				backups = backupResult.backups;
 				backupError = backupResult.error;
 
@@ -137,6 +146,51 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 		}
 	}
 
+	private async shouldBackupBeforeShutdown(reason: ShutdownReason): Promise<boolean> {
+		let backup: boolean | undefined;
+		if (!this.filesConfigurationService.isHotExitEnabled) {
+			backup = false; // never backup when hot exit is disabled via settings
+		} else if (this.environmentService.isExtensionDevelopment) {
+			backup = true; // always backup closing extension development window without asking to speed up debugging
+		} else {
+
+			// When quit is requested skip the confirm callback and attempt to backup all workspaces.
+			// When quit is not requested the confirm callback should be shown when the window being
+			// closed is the only VS Code window open, except for on Mac where hot exit is only
+			// ever activated when quit is requested.
+
+			switch (reason) {
+				case ShutdownReason.CLOSE:
+					if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) {
+						backup = true; // backup if a folder is open and onExitAndWindowClose is configured
+					} else if (await this.nativeHostService.getWindowCount() > 1 || isMacintosh) {
+						backup = false; // do not backup if a window is closed that does not cause quitting of the application
+					} else {
+						backup = true; // backup if last window is closed on win/linux where the application quits right after
+					}
+					break;
+
+				case ShutdownReason.QUIT:
+					backup = true; // backup because next start we restore all backups
+					break;
+
+				case ShutdownReason.RELOAD:
+					backup = true; // backup because after window reload, backups restore
+					break;
+
+				case ShutdownReason.LOAD:
+					if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) {
+						backup = true; // backup if a folder is open and onExitAndWindowClose is configured
+					} else {
+						backup = false; // do not backup because we are switching contexts
+					}
+					break;
+			}
+		}
+
+		return backup;
+	}
+
 	private showErrorDialog(msg: string, workingCopies: readonly IWorkingCopy[], error?: Error): void {
 		const dirtyWorkingCopies = workingCopies.filter(workingCopy => workingCopy.isDirty());
 
@@ -150,54 +204,7 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 		this.logService.error(error ? `[backup tracker] ${msg}: ${error}` : `[backup tracker] ${msg}`);
 	}
 
-	private async backupBeforeShutdown(dirtyWorkingCopies: readonly IWorkingCopy[], reason: ShutdownReason): Promise<{ backups: IWorkingCopy[], error?: Error }> {
-
-		// When quit is requested skip the confirm callback and attempt to backup all workspaces.
-		// When quit is not requested the confirm callback should be shown when the window being
-		// closed is the only VS Code window open, except for on Mac where hot exit is only
-		// ever activated when quit is requested.
-
-		let doBackup: boolean | undefined;
-		if (this.environmentService.isExtensionDevelopment) {
-			doBackup = true; // always backup closing extension development window without asking to speed up debugging
-		} else {
-			switch (reason) {
-				case ShutdownReason.CLOSE:
-					if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) {
-						doBackup = true; // backup if a folder is open and onExitAndWindowClose is configured
-					} else if (await this.nativeHostService.getWindowCount() > 1 || isMacintosh) {
-						doBackup = false; // do not backup if a window is closed that does not cause quitting of the application
-					} else {
-						doBackup = true; // backup if last window is closed on win/linux where the application quits right after
-					}
-					break;
-
-				case ShutdownReason.QUIT:
-					doBackup = true; // backup because next start we restore all backups
-					break;
-
-				case ShutdownReason.RELOAD:
-					doBackup = true; // backup because after window reload, backups restore
-					break;
-
-				case ShutdownReason.LOAD:
-					if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) {
-						doBackup = true; // backup if a folder is open and onExitAndWindowClose is configured
-					} else {
-						doBackup = false; // do not backup because we are switching contexts
-					}
-					break;
-			}
-		}
-
-		if (!doBackup) {
-			return { backups: [] };
-		}
-
-		return this.doBackupBeforeShutdown(dirtyWorkingCopies);
-	}
-
-	private async doBackupBeforeShutdown(dirtyWorkingCopies: readonly IWorkingCopy[]): Promise<{ backups: IWorkingCopy[], error?: Error }> {
+	private async backupBeforeShutdown(dirtyWorkingCopies: readonly IWorkingCopy[]): Promise<{ backups: IWorkingCopy[], error?: Error }> {
 		const backups: IWorkingCopy[] = [];
 		let error: Error | undefined = undefined;
 
@@ -206,9 +213,9 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 			// Perform a backup of all dirty working copies unless a backup already exists
 			try {
 				await Promises.settled(dirtyWorkingCopies.map(async workingCopy => {
-					const contentVersion = this.getContentVersion(workingCopy);
 
 					// Backup exists
+					const contentVersion = this.getContentVersion(workingCopy);
 					if (this.workingCopyBackupService.hasBackupSync(workingCopy, contentVersion)) {
 						backups.push(workingCopy);
 					}
@@ -216,7 +223,14 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 					// Backup does not exist
 					else {
 						const backup = await workingCopy.backup(token);
+						if (token.isCancellationRequested) {
+							return;
+						}
+
 						await this.workingCopyBackupService.backup(workingCopy, backup.content, contentVersion, backup.meta, token);
+						if (token.isCancellationRequested) {
+							return;
+						}
 
 						backups.push(workingCopy);
 					}

src/vs/workbench/services/workingCopy/test/browser/workingCopyBackupTracker.test.ts

@@ -17,7 +17,6 @@ import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/wo
 import { IWorkingCopyBackup } from 'vs/workbench/services/workingCopy/common/workingCopy';
 import { ILogService } from 'vs/platform/log/common/log';
 import { ILifecycleService, LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
-import { WorkingCopyBackupTracker } from 'vs/workbench/services/workingCopy/common/workingCopyBackupTracker';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { UntitledTextEditorInput } from 'vs/workbench/services/untitled/common/untitledTextEditorInput';
 import { createEditorPart, InMemoryTestWorkingCopyBackupService, registerTestResourceEditor, TestServiceAccessor, toTypedWorkingCopyId, toUntypedWorkingCopyId, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
@@ -65,6 +64,8 @@ suite('WorkingCopyBackupTracker (browser)', function () {
 			return 10; // Reduce timeout for tests
 		}
 
+		get pendingBackupOperationCount(): number { return this.pendingBackupOperations.size; }
+
 		getUnrestoredBackups() {
 			return this.unrestoredBackups;
 		}
@@ -85,7 +86,7 @@ suite('WorkingCopyBackupTracker (browser)', function () {
 		}
 	}
 
-	async function createTracker(): Promise<{ accessor: TestServiceAccessor, part: EditorPart, tracker: WorkingCopyBackupTracker, workingCopyBackupService: InMemoryTestWorkingCopyBackupService, instantiationService: IInstantiationService, cleanup: () => void }> {
+	async function createTracker(): Promise<{ accessor: TestServiceAccessor, part: EditorPart, tracker: TestWorkingCopyBackupTracker, workingCopyBackupService: InMemoryTestWorkingCopyBackupService, instantiationService: IInstantiationService, cleanup: () => void }> {
 		const disposables = new DisposableStore();
 
 		const workingCopyBackupService = new InMemoryTestWorkingCopyBackupService();
@@ -142,7 +143,7 @@ suite('WorkingCopyBackupTracker (browser)', function () {
 	});
 
 	test('Track backups (custom)', async function () {
-		const { accessor, cleanup, workingCopyBackupService } = await createTracker();
+		const { accessor, tracker, cleanup, workingCopyBackupService } = await createTracker();
 
 		class TestBackupWorkingCopy extends TestWorkingCopy {
 
@@ -166,15 +167,18 @@ suite('WorkingCopyBackupTracker (browser)', function () {
 
 		// Normal
 		customWorkingCopy.setDirty(true);
+		assert.strictEqual(tracker.pendingBackupOperationCount, 1);
 		await workingCopyBackupService.joinBackupResource();
 		assert.strictEqual(workingCopyBackupService.hasBackupSync(customWorkingCopy), true);
 
 		customWorkingCopy.setDirty(false);
 		customWorkingCopy.setDirty(true);
+		assert.strictEqual(tracker.pendingBackupOperationCount, 1);
 		await workingCopyBackupService.joinBackupResource();
 		assert.strictEqual(workingCopyBackupService.hasBackupSync(customWorkingCopy), true);
 
 		customWorkingCopy.setDirty(false);
+		assert.strictEqual(tracker.pendingBackupOperationCount, 1);
 		await workingCopyBackupService.joinDiscardBackup();
 		assert.strictEqual(workingCopyBackupService.hasBackupSync(customWorkingCopy), false);
 
@@ -182,6 +186,7 @@ suite('WorkingCopyBackupTracker (browser)', function () {
 		customWorkingCopy.setDirty(true);
 		await timeout(0);
 		customWorkingCopy.setDirty(false);
+		assert.strictEqual(tracker.pendingBackupOperationCount, 1);
 		await workingCopyBackupService.joinDiscardBackup();
 		assert.strictEqual(workingCopyBackupService.hasBackupSync(customWorkingCopy), false);
 

src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyBackupTracker.test.ts

@@ -75,10 +75,12 @@ flakySuite('WorkingCopyBackupTracker (native)', function () {
 			return super.whenReady;
 		}
 
+		get pendingBackupOperationCount(): number { return this.pendingBackupOperations.size; }
+
 		override dispose() {
 			super.dispose();
 
-			for (const [_, disposable] of this.pendingBackups) {
+			for (const [_, disposable] of this.pendingBackupOperations) {
 				disposable.dispose();
 			}
 		}
@@ -355,6 +357,27 @@ flakySuite('WorkingCopyBackupTracker (native)', function () {
 		await cleanup();
 	});
 
+	test('onWillShutdown - pending backup operations canceled', async function () {
+		const { accessor, tracker, cleanup } = await createTracker();
+
+		const resource = toResource.call(this, '/path/index.txt');
+		await accessor.editorService.openEditor({ resource, options: { pinned: true } });
+
+		const model = accessor.textFileService.files.get(resource);
+
+		await model?.resolve();
+		model?.textEditorModel?.setValue('foo');
+		assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
+		assert.strictEqual(tracker.pendingBackupOperationCount, 1);
+
+		const event = new TestBeforeShutdownEvent();
+		accessor.lifecycleService.fireBeforeShutdown(event);
+
+		assert.strictEqual(tracker.pendingBackupOperationCount, 0);
+
+		await cleanup();
+	});
+
 	suite('Hot Exit', () => {
 		suite('"onExit" setting', () => {
 			test('should hot exit on non-Mac (reason: CLOSE, windows: single, workspace)', function () {

test/smoke/src/areas/workbench/data-migration.test.ts

@@ -15,7 +15,7 @@ export function setup(opts: ParsedArgs) {
 
 		afterSuite(opts, () => app);
 
-		it(`verifies opened editors are restored`, async function () {
+		it('verifies opened editors are restored', async function () {
 			app = await startApp(opts, this.defaultOptions);
 
 			// Open 3 editors and pin 2 of them
@@ -38,7 +38,15 @@ export function setup(opts: ParsedArgs) {
 			app = undefined;
 		});
 
-		it(`verifies that 'hot exit' works for dirty files`, async function () {
+		it('verifies that "hot exit" works for dirty files (without delay)', function () {
+			return testHotExit.call(this, undefined);
+		});
+
+		it('verifies that "hot exit" works for dirty files (with delay)', function () {
+			return testHotExit.call(this, 2000);
+		});
+
+		async function testHotExit(restartDelay: number | undefined) {
 			app = await startApp(opts, this.defaultOptions);
 
 			await app.workbench.editors.newUntitledFile();
@@ -54,19 +62,27 @@ export function setup(opts: ParsedArgs) {
 			await app.workbench.editor.waitForTypeInEditor(readmeMd, textToType);
 			await app.workbench.editors.waitForTab(readmeMd, true);
 
+			if (typeof restartDelay === 'number') {
+				// this is an OK use of a timeout in a smoke test
+				// we want to simulate a user having typed into
+				// the editor and pausing for a moment before
+				// terminating
+				await timeout(restartDelay);
+			}
+
 			await app.restart();
 
 			await app.workbench.editors.waitForTab(readmeMd, true);
 			await app.workbench.quickaccess.openFile(readmeMd);
-			await app.workbench.editor.waitForEditorContents(readmeMd, c => c.indexOf(textToType) > -1);
+			await app.workbench.editor.waitForEditorContents(readmeMd, contents => contents.indexOf(textToType) > -1);
 
 			await app.workbench.editors.waitForTab(untitled, true);
 			await app.workbench.quickaccess.openFile(untitled, textToTypeInUntitled);
-			await app.workbench.editor.waitForEditorContents(untitled, c => c.indexOf(textToTypeInUntitled) > -1);
+			await app.workbench.editor.waitForEditorContents(untitled, contents => contents.indexOf(textToTypeInUntitled) > -1);
 
 			await app.stop();
 			app = undefined;
-		});
+		}
 	});
 
 	describe('Data Migration (stable -> insiders)', () => {
@@ -76,7 +92,7 @@ export function setup(opts: ParsedArgs) {
 
 		afterSuite(opts, () => insidersApp ?? stableApp, async () => stableApp?.stop());
 
-		it(`verifies opened editors are restored`, async function () {
+		it('verifies opened editors are restored', async function () {
 			const stableCodePath = opts['stable-build'];
 			if (!stableCodePath || opts.remote) {
 				this.skip();
@@ -126,7 +142,15 @@ export function setup(opts: ParsedArgs) {
 			insidersApp = undefined;
 		});
 
-		it(`verifies that 'hot exit' works for dirty files`, async function () {
+		it('verifies that "hot exit" works for dirty files (without delay)', async function () {
+			return testHotExit.call(this, undefined);
+		});
+
+		it('verifies that "hot exit" works for dirty files (with delay)', async function () {
+			return testHotExit.call(this, 2000);
+		});
+
+		async function testHotExit(restartDelay: number | undefined) {
 			const stableCodePath = opts['stable-build'];
 			if (!stableCodePath || opts.remote) {
 				this.skip();
@@ -155,7 +179,13 @@ export function setup(opts: ParsedArgs) {
 			await stableApp.workbench.editor.waitForTypeInEditor(readmeMd, textToType);
 			await stableApp.workbench.editors.waitForTab(readmeMd, true);
 
-			await timeout(2000); // TODO@bpasero https://github.com/microsoft/vscode/issues/138055
+			if (typeof restartDelay === 'number') {
+				// this is an OK use of a timeout in a smoke test
+				// we want to simulate a user having typed into
+				// the editor and pausing for a moment before
+				// terminating
+				await timeout(restartDelay);
+			}
 
 			await stableApp.stop();
 			stableApp = undefined;
@@ -168,14 +198,14 @@ export function setup(opts: ParsedArgs) {
 
 			await insidersApp.workbench.editors.waitForTab(readmeMd, true);
 			await insidersApp.workbench.quickaccess.openFile(readmeMd);
-			await insidersApp.workbench.editor.waitForEditorContents(readmeMd, c => c.indexOf(textToType) > -1);
+			await insidersApp.workbench.editor.waitForEditorContents(readmeMd, contents => contents.indexOf(textToType) > -1);
 
 			await insidersApp.workbench.editors.waitForTab(untitled, true);
 			await insidersApp.workbench.quickaccess.openFile(untitled, textToTypeInUntitled);
-			await insidersApp.workbench.editor.waitForEditorContents(untitled, c => c.indexOf(textToTypeInUntitled) > -1);
+			await insidersApp.workbench.editor.waitForEditorContents(untitled, contents => contents.indexOf(textToTypeInUntitled) > -1);
 
 			await insidersApp.stop();
 			insidersApp = undefined;
-		});
+		}
 	});
 }