Dataloss when closing Code before it fully restored with dirty files (fix #123636)

This commit is contained in:
Benjamin Pasero 2021-05-12 07:40:20 +02:00
parent 08b559fd96
commit 1e8d96dcc1
No known key found for this signature in database
GPG key ID: E6380CC4C8219E65
7 changed files with 44 additions and 45 deletions

View file

@ -211,11 +211,6 @@ export class EditorPart extends Part implements IEditorGroupsService, IEditorGro
private whenRestoredResolve: (() => void) | undefined;
readonly whenRestored = new Promise<void>(resolve => (this.whenRestoredResolve = resolve));
private restored = false;
isRestored(): boolean {
return this.restored;
}
get hasRestorableState(): boolean {
return !!this.workspaceMemento[EditorPart.EDITOR_PART_UI_STATE_STORAGE_KEY];
@ -845,7 +840,6 @@ export class EditorPart extends Part implements IEditorGroupsService, IEditorGro
// Signal restored
Promises.settled(this.groups.map(group => group.whenRestored)).finally(() => {
this.restored = true;
this.whenRestoredResolve?.();
});

View file

@ -217,11 +217,6 @@ export interface IEditorGroupsService {
*/
readonly whenRestored: Promise<void>;
/**
* Will return `true` as soon as `whenRestored` is resolved.
*/
isRestored(): boolean;
/**
* Find out if the editor group service has UI state to restore
* from a previous session.

View file

@ -347,8 +347,6 @@ suite('EditorGroupsService', () => {
await part.whenReady;
await part.whenRestored;
assert.strictEqual(part.isRestored(), true);
});
test('options', async () => {

View file

@ -210,6 +210,9 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
protected readonly unrestoredBackups = new Set<IWorkingCopyIdentifier>();
private readonly whenReady = this.resolveBackupsToRestore();
private _isReady = false;
protected get isReady(): boolean { return this._isReady; }
private async resolveBackupsToRestore(): Promise<void> {
// Wait for resolving backups until we are restored to reduce startup pressure
@ -219,6 +222,8 @@ export abstract class WorkingCopyBackupTracker extends Disposable {
for (const backup of await this.workingCopyBackupService.getBackups()) {
this.unrestoredBackups.add(backup);
}
this._isReady = true;
}
protected async restoreBackups(handler: IWorkingCopyEditorHandler): Promise<void> {

View file

@ -8,7 +8,7 @@ import { IWorkingCopyBackupService } from 'vs/workbench/services/workingCopy/com
import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
import { IFilesConfigurationService, AutoSaveMode } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
import { IWorkingCopy, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopy';
import { IWorkingCopy, IWorkingCopyIdentifier, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopy';
import { ILifecycleService, ShutdownReason } from 'vs/workbench/services/lifecycle/common/lifecycle';
import { ConfirmResult, IFileDialogService, IDialogService, getFileNamesMessage } from 'vs/platform/dialogs/common/dialogs';
import Severity from 'vs/base/common/severity';
@ -24,7 +24,6 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/progress';
import { Promises, raceCancellation } from 'vs/base/common/async';
import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
import { IWorkingCopyEditorService } from 'vs/workbench/services/workingCopy/common/workingCopyEditorService';
export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker implements IWorkbenchContribution {
@ -41,7 +40,6 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
@ILogService logService: ILogService,
@IEnvironmentService private readonly environmentService: IEnvironmentService,
@IProgressService private readonly progressService: IProgressService,
@IEditorGroupsService private readonly editorGroupService: IEditorGroupsService,
@IWorkingCopyEditorService workingCopyEditorService: IWorkingCopyEditorService,
@IEditorService editorService: IEditorService
) {
@ -328,40 +326,51 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
}, () => raceCancellation(promiseFactory(cts.token), cts.token), () => cts.dispose(true));
}
private async noVeto(backupsToDiscard: IWorkingCopy[]): Promise<boolean> {
private async noVeto(backupsToDiscard: IWorkingCopyIdentifier[]): Promise<boolean> {
// If we have proceeded enough that editors and dirty state
// has restored, we make sure that the dirty working copies
// that have been handled by the user get discarded.
if (this.editorGroupService.isRestored()) {
try {
await Promises.settled(backupsToDiscard.map(workingCopy => this.workingCopyBackupService.discardBackup(workingCopy)));
} catch (error) {
this.logService.error(`[backup tracker] error discarding backups: ${error}`);
}
}
// Discard backups from working copies the
// user either saved or reverted
await this.discardBackupsBeforeShutdown(backupsToDiscard);
return false; // no veto (no dirty)
}
private async onBeforeShutdownWithoutDirty(): Promise<boolean> {
// If we have proceeded enough that editors and dirty state
// has restored, we make sure that no backups lure around
// given we have no known dirty working copy. This helps
// to clean up stale backups as for example reported in
// https://github.com/microsoft/vscode/issues/92962
//
// However, we never want to discard backups that we know
// were not restored in the session.
if (this.editorGroupService.isRestored()) {
try {
await this.workingCopyBackupService.discardBackups({ except: Array.from(this.unrestoredBackups) });
} catch (error) {
this.logService.error(`[backup tracker] error discarding backups: ${error}`);
}
}
// Discard all backups except those that
// were not restored
await this.discardBackupsBeforeShutdown({ except: Array.from(this.unrestoredBackups) });
return false; // no veto (no dirty)
}
private discardBackupsBeforeShutdown(backupsToDiscard: IWorkingCopyIdentifier[]): Promise<void>;
private discardBackupsBeforeShutdown(backupsToKeep: { except: IWorkingCopyIdentifier[] }): Promise<void>;
private async discardBackupsBeforeShutdown(arg1: IWorkingCopyIdentifier[] | { except: IWorkingCopyIdentifier[] }): Promise<void> {
// We never discard any backups before we are ready
// and have resolved all backups that exist. This
// is important to not loose backups that have not
// been handled.
if (!this.isReady) {
return;
}
// When we shutdown either with no dirty working copies left
// or with some handled, we start to discard these backups
// to free them up. This helps to get rid of stale backups
// as reported in https://github.com/microsoft/vscode/issues/92962
//
// However, we never want to discard backups that we know
// were not restored in the session.
try {
if (Array.isArray(arg1)) {
await Promises.settled(arg1.map(workingCopy => this.workingCopyBackupService.discardBackup(workingCopy)));
} else {
await this.workingCopyBackupService.discardBackups(arg1);
}
} catch (error) {
this.logService.error(`[backup tracker] error discarding backups: ${error}`);
}
}
}

View file

@ -63,10 +63,9 @@ flakySuite('WorkingCopyBackupTracker (native)', function () {
@IEditorService editorService: IEditorService,
@IEnvironmentService environmentService: IEnvironmentService,
@IProgressService progressService: IProgressService,
@IEditorGroupsService editorGroupService: IEditorGroupsService,
@IWorkingCopyEditorService workingCopyEditorService: IWorkingCopyEditorService
) {
super(workingCopyBackupService, filesConfigurationService, workingCopyService, lifecycleService, fileDialogService, dialogService, contextService, nativeHostService, logService, environmentService, progressService, editorGroupService, workingCopyEditorService, editorService);
super(workingCopyBackupService, filesConfigurationService, workingCopyService, lifecycleService, fileDialogService, dialogService, contextService, nativeHostService, logService, environmentService, progressService, workingCopyEditorService, editorService);
}
protected override getBackupScheduleDelay(): number {

View file

@ -656,7 +656,6 @@ export class TestEditorGroupsService implements IEditorGroupsService {
get activeGroup(): IEditorGroup { return this.groups[0]; }
get count(): number { return this.groups.length; }
isRestored(): boolean { return true; }
getGroups(_order?: GroupsOrder): readonly IEditorGroup[] { return this.groups; }
getGroup(identifier: number): IEditorGroup | undefined { return this.groups.find(group => group.id === identifier); }
getLabel(_identifier: number): string { return 'Group 1'; }