Skip to content

Commit f9a0f4e

Browse files
author
Benjamin Pasero
committed
backup - make shutdown more explicit
1 parent bfee5ac commit f9a0f4e

8 files changed

Lines changed: 59 additions & 43 deletions

File tree

‎src/vs/base/common/async.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,9 @@ export class Queue<T> extends Limiter<T> {
481481
* A helper to organize queues per resource. The ResourceQueue makes sure to manage queues per resource
482482
* by disposing them once the queue is empty.
483483
*/
484-
export class ResourceQueue {
485-
private queues: Map<string, Queue<void>> = new Map();
484+
export class ResourceQueue implements IDisposable {
485+
486+
private readonly queues = new Map<string, Queue<void>>();
486487

487488
queueFor(resource: URI): Queue<void> {
488489
const key = resource.toString();
@@ -498,6 +499,14 @@ export class ResourceQueue {
498499

499500
return this.queues.get(key)!;
500501
}
502+
503+
dispose(): void {
504+
for (const [, queue] of this.queues) {
505+
queue.dispose();
506+
}
507+
508+
this.queues.clear();
509+
}
501510
}
502511

503512
export class TimeoutTimer implements IDisposable {

‎src/vs/workbench/contrib/backup/browser/backupTracker.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export class BrowserBackupTracker extends BackupTracker implements IWorkbenchCon
2525

2626
protected onBeforeShutdown(reason: ShutdownReason): boolean | Promise<boolean> {
2727

28-
2928
// Web: we cannot perform long running in the shutdown phase
3029
// As such we need to check sync if there are any dirty working
3130
// copies that have not been backed up yet and then prevent the

‎src/vs/workbench/contrib/backup/electron-browser/backupTracker.ts‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
5454
return this.handleDirtyBeforeShutdown(remainingDirtyWorkingCopies, reason);
5555
}
5656

57-
return this.noVeto({ cleanUpBackups: false }); // no veto and no backup cleanup (since there are no dirty working copies)
57+
return this.noVeto({ dicardAllBackups: false }); // no veto and no backup cleanup (since there are no dirty working copies)
5858
});
5959
}
6060

@@ -63,7 +63,7 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
6363
}
6464

6565
// No dirty working copies: no veto
66-
return this.noVeto({ cleanUpBackups: true });
66+
return this.noVeto({ dicardAllBackups: true });
6767
}
6868

6969
private handleDirtyBeforeShutdown(workingCopies: IWorkingCopy[], reason: ShutdownReason): boolean | Promise<boolean> {
@@ -72,7 +72,7 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
7272
if (this.filesConfigurationService.isHotExitEnabled) {
7373
return this.backupBeforeShutdown(workingCopies, reason).then(didBackup => {
7474
if (didBackup) {
75-
return this.noVeto({ cleanUpBackups: false }); // no veto and no backup cleanup (since backup was successful)
75+
return this.noVeto({ dicardAllBackups: false }); // no veto and no backup cleanup (since backup was successful)
7676
}
7777

7878
// since a backup did not happen, we have to confirm for the dirty working copies now
@@ -151,7 +151,7 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
151151
return true; // veto if any save failed
152152
}
153153

154-
return this.noVeto({ cleanUpBackups: true });
154+
return this.noVeto({ dicardAllBackups: true });
155155
}
156156

157157
// Don't Save
@@ -161,7 +161,7 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
161161
// see https://github.com/Microsoft/vscode/issues/29572
162162
await this.doRevertAll(dirtyWorkingCopies, { soft: true } /* soft revert is good enough on shutdown */);
163163

164-
return this.noVeto({ cleanUpBackups: true });
164+
return this.noVeto({ dicardAllBackups: true });
165165
}
166166

167167
// Cancel
@@ -186,19 +186,17 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
186186
return Promise.all(workingCopies.map(workingCopy => workingCopy.revert(options)));
187187
}
188188

189-
private noVeto(options: { cleanUpBackups: boolean }): boolean | Promise<boolean> {
190-
if (!options.cleanUpBackups) {
191-
return false;
192-
}
189+
private noVeto(options: { dicardAllBackups: boolean }): boolean | Promise<boolean> {
190+
let dicardAllBackups = options.dicardAllBackups;
193191

194192
if (this.lifecycleService.phase < LifecyclePhase.Restored) {
195-
return false; // if editors have not restored, we are not up to speed with backups and thus should not clean them
193+
dicardAllBackups = false; // if editors have not restored, we are not up to speed with backups and thus should not discard them
196194
}
197195

198196
if (this.environmentService.isExtensionDevelopment) {
199-
return false; // extension development does not track any backups
197+
dicardAllBackups = false; // extension development does not track any backups
200198
}
201199

202-
return this.backupFileService.discardBackups().then(() => false, () => false);
200+
return this.backupFileService.shutdown({ dicardAllBackups }).then(() => false, () => false);
203201
}
204202
}

‎src/vs/workbench/services/backup/common/backup.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ export interface IBackupFileService {
6666
discardBackup(resource: URI): Promise<void>;
6767

6868
/**
69-
* Discards all backups associated with the current workspace and prevents further backups from
70-
* being made.
69+
* Shutdown the service and optionally discard any backups.
7170
*/
72-
discardBackups(): Promise<void>;
71+
shutdown(options?: { dicardAllBackups: boolean }): Promise<void>;
7372
}

‎src/vs/workbench/services/backup/common/backupFileService.ts‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export interface IBackupFilesModel {
2828
get(): URI[];
2929
remove(resource: URI): void;
3030
count(): number;
31+
3132
clear(): void;
3233
}
3334

@@ -37,7 +38,8 @@ interface IBackupCacheEntry {
3738
}
3839

3940
export class BackupFilesModel implements IBackupFilesModel {
40-
private cache: ResourceMap<IBackupCacheEntry> = new ResourceMap();
41+
42+
private readonly cache = new ResourceMap<IBackupCacheEntry>();
4143

4244
constructor(private fileService: IFileService) { }
4345

@@ -160,8 +162,8 @@ export class BackupFileService implements IBackupFileService {
160162
return this.impl.discardBackup(resource);
161163
}
162164

163-
discardBackups(): Promise<void> {
164-
return this.impl.discardBackups();
165+
shutdown(options?: { dicardAllBackups: boolean }): Promise<void> {
166+
return this.impl.shutdown(options);
165167
}
166168

167169
getBackups(): Promise<URI[]> {
@@ -187,8 +189,8 @@ class BackupFileServiceImpl implements IBackupFileService {
187189

188190
private backupWorkspacePath!: URI;
189191

190-
private isShuttingDown: boolean;
191-
private ioOperationQueues: ResourceQueue; // queue IO operations to ensure write order
192+
private readonly ioOperationQueues = new ResourceQueue(); // queue IO operations to ensure write order
193+
private isShutdown = false; // a flag to set indicating that we no longer backup any resources
192194

193195
private ready!: Promise<IBackupFilesModel>;
194196
private model!: IBackupFilesModel;
@@ -198,9 +200,6 @@ class BackupFileServiceImpl implements IBackupFileService {
198200
private readonly hashPath: (resource: URI) => string,
199201
@IFileService private readonly fileService: IFileService
200202
) {
201-
this.isShuttingDown = false;
202-
this.ioOperationQueues = new ResourceQueue();
203-
204203
this.initialize(backupWorkspaceResource);
205204
}
206205

@@ -229,7 +228,7 @@ class BackupFileServiceImpl implements IBackupFileService {
229228
}
230229

231230
async backup<T extends object>(resource: URI, content?: ITextSnapshot, versionId?: number, meta?: T): Promise<void> {
232-
if (this.isShuttingDown) {
231+
if (this.isShutdown) {
233232
return;
234233
}
235234

@@ -265,27 +264,39 @@ class BackupFileServiceImpl implements IBackupFileService {
265264
}
266265

267266
async discardBackup(resource: URI): Promise<void> {
267+
if (this.isShutdown) {
268+
return;
269+
}
270+
268271
const model = await this.ready;
269272
const backupResource = this.toBackupResource(resource);
270273

271274
return this.ioOperationQueues.queueFor(backupResource).queue(async () => {
272-
await this.doDiscardResource(backupResource);
275+
await this.deleteIgnoreFileNotFound(backupResource);
273276

274277
model.remove(backupResource);
275278
});
276279
}
277280

278-
async discardBackups(): Promise<void> {
279-
this.isShuttingDown = true;
281+
async shutdown(options?: { dicardAllBackups: boolean }): Promise<void> {
280282

281-
const model = await this.ready;
283+
// Signal that no more backups should happen from this point
284+
this.isShutdown = true;
285+
this.ioOperationQueues.dispose();
282286

283-
await this.doDiscardResource(this.backupWorkspacePath);
287+
// Optionally discard backups
288+
if (options?.dicardAllBackups) {
284289

285-
model.clear();
290+
// Clear model
291+
const model = await this.ready;
292+
model.clear();
293+
294+
// Delete the backup home for this workspace
295+
await this.deleteIgnoreFileNotFound(this.backupWorkspacePath);
296+
}
286297
}
287298

288-
private async doDiscardResource(resource: URI): Promise<void> {
299+
private async deleteIgnoreFileNotFound(resource: URI): Promise<void> {
289300
try {
290301
await this.fileService.del(resource, { recursive: true });
291302
} catch (error) {
@@ -436,7 +447,7 @@ export class InMemoryBackupFileService implements IBackupFileService {
436447
this.backups.delete(this.toBackupResource(resource).toString());
437448
}
438449

439-
async discardBackups(): Promise<void> {
450+
async shutdown(options?: { dicardAllBackups: boolean }): Promise<void> {
440451
this.backups.clear();
441452
}
442453

‎src/vs/workbench/services/backup/test/electron-browser/backupFileService.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ export class NodeTestBackupFileService extends BackupFileService {
101101

102102
didDiscardAllWorkspaceBackups: boolean;
103103

104-
discardBackups(): Promise<void> {
105-
this.didDiscardAllWorkspaceBackups = true;
104+
shutdown(options?: { dicardAllBackups: boolean }): Promise<void> {
105+
this.didDiscardAllWorkspaceBackups = !!options?.dicardAllBackups;
106106

107-
return super.discardBackups();
107+
return super.shutdown(options);
108108
}
109109
}
110110

@@ -289,7 +289,7 @@ suite('BackupFileService', () => {
289289
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
290290
await service.backup(barFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
291291
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 2);
292-
await service.discardBackups();
292+
await service.shutdown({ dicardAllBackups: true });
293293
assert.equal(fs.existsSync(fooBackupPath), false);
294294
assert.equal(fs.existsSync(barBackupPath), false);
295295
assert.equal(fs.existsSync(path.join(workspaceBackupPath, 'file')), false);
@@ -298,13 +298,13 @@ suite('BackupFileService', () => {
298298
test('untitled file', async () => {
299299
await service.backup(untitledFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
300300
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1);
301-
await service.discardBackups();
301+
await service.shutdown({ dicardAllBackups: true });
302302
assert.equal(fs.existsSync(untitledBackupPath), false);
303303
assert.equal(fs.existsSync(path.join(workspaceBackupPath, 'untitled')), false);
304304
});
305305

306306
test('should disable further backups', async () => {
307-
await service.discardBackups();
307+
await service.shutdown({ dicardAllBackups: true });
308308
await service.backup(untitledFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
309309
assert.equal(fs.existsSync(workspaceBackupPath), false);
310310
});

‎src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
4444
private readonly mapResourceToDisposeListener = new ResourceMap<IDisposable>();
4545
private readonly mapResourceToPendingModelLoaders = new ResourceMap<Promise<ITextFileEditorModel>>();
4646

47-
private readonly modelLoadQueue = new ResourceQueue();
47+
private readonly modelLoadQueue = this._register(new ResourceQueue());
4848

4949
constructor(
5050
@ILifecycleService private readonly lifecycleService: ILifecycleService,

‎src/vs/workbench/test/workbenchTestServices.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,7 @@ export class TestBackupFileService implements IBackupFileService {
11981198
return Promise.resolve();
11991199
}
12001200

1201-
discardBackups(): Promise<void> {
1201+
shutdown(options?: { dicardAllBackups: boolean }): Promise<void> {
12021202
return Promise.resolve();
12031203
}
12041204
}

0 commit comments

Comments
 (0)