From b989a6ab0568196e6520c9333287bf30ac4690d1 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 13 Feb 2020 16:45:49 +0100 Subject: [PATCH] Investigate to cancel a running save participant when saving again (#90590) * Investigate to cancel a running save participant when saving again (fix #90509) * keep token source --- .../api/browser/mainThreadSaveParticipant.ts | 6 ++-- .../textfile/common/saveSequenzializer.ts | 9 ++++-- .../textfile/common/textFileEditorModel.ts | 13 ++++++-- .../services/textfile/common/textfiles.ts | 3 +- .../test/browser/textFileEditorModel.test.ts | 30 +++++++++++++++++++ .../test/common/saveSequenzializer.test.ts | 10 +++++++ 6 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts b/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts index 3cd807c4cce..288a349b183 100644 --- a/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts +++ b/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts @@ -365,9 +365,9 @@ export class SaveParticipant implements ISaveParticipant { this._saveParticipants.dispose(); } - async participate(model: IResolvedTextFileEditorModel, env: { reason: SaveReason; }): Promise { + async participate(model: IResolvedTextFileEditorModel, context: { reason: SaveReason; }, token: CancellationToken): Promise { - const cts = new CancellationTokenSource(); + const cts = new CancellationTokenSource(token); return this._progressService.withProgress({ title: localize('saveParticipants', "Running Save Participants for '{0}'", this._labelService.getUriLabel(model.resource, { relative: true })), @@ -383,7 +383,7 @@ export class SaveParticipant implements ISaveParticipant { break; } try { - const promise = p.participate(model, env, progress, cts.token); + const promise = p.participate(model, context, progress, cts.token); await raceCancellation(promise, cts.token); } catch (err) { this._logService.warn(err); diff --git a/src/vs/workbench/services/textfile/common/saveSequenzializer.ts b/src/vs/workbench/services/textfile/common/saveSequenzializer.ts index 2b7cc0c8282..76513ef1e16 100644 --- a/src/vs/workbench/services/textfile/common/saveSequenzializer.ts +++ b/src/vs/workbench/services/textfile/common/saveSequenzializer.ts @@ -5,6 +5,7 @@ interface IPendingSave { versionId: number; + cancel: () => void; promise: Promise; } @@ -35,8 +36,12 @@ export class SaveSequentializer { return this._pendingSave ? this._pendingSave.promise : undefined; } - setPending(versionId: number, promise: Promise): Promise { - this._pendingSave = { versionId, promise }; + cancelPending(): void { + this._pendingSave?.cancel(); + } + + setPending(versionId: number, promise: Promise, onCancel?: () => void, ): Promise { + this._pendingSave = { versionId, cancel: () => onCancel?.(), promise }; promise.then(() => this.donePending(versionId), () => this.donePending(versionId)); diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index 52ef8b8e949..40b3b72bbd5 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -24,6 +24,7 @@ import { IWorkingCopyService, IWorkingCopyBackup } from 'vs/workbench/services/w import { IFilesConfigurationService } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService'; import { SaveSequentializer } from 'vs/workbench/services/textfile/common/saveSequenzializer'; import { ILabelService } from 'vs/platform/label/common/label'; +import { CancellationTokenSource } from 'vs/base/common/cancellation'; interface IBackupMetaData { mtime: number; @@ -601,6 +602,13 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil if (this.saveSequentializer.hasPendingSave()) { this.logService.trace(`[text file model] doSave(${versionId}) - exit - because busy saving`, this.resource.toString()); + // Indicate to the save sequentializer that we want to + // cancel the pending operation so that ours can run + // before the pending one finishes. + // Currently this will try to cancel pending save + // participants but never a pending save. + this.saveSequentializer.cancelPending(); + // Register this as the next upcoming save and return return this.saveSequentializer.setNext(() => this.doSave(options)); } @@ -616,6 +624,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil // In addition we update our version right after in case it changed because of a model change // // Save participants can also be skipped through API. + const saveParticipantCancellation = new CancellationTokenSource(); let saveParticipantPromise: Promise = Promise.resolve(versionId); if (this.isResolved() && this.textFileService.saveParticipant && !options.skipSaveParticipants) { const onCompleteOrError = () => { @@ -625,7 +634,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil }; this.ignoreDirtyOnModelContentChange = true; - saveParticipantPromise = this.textFileService.saveParticipant.participate(this, { reason: options.reason }).then(onCompleteOrError, onCompleteOrError); + saveParticipantPromise = this.textFileService.saveParticipant.participate(this, { reason: options.reason }, saveParticipantCancellation.token).then(onCompleteOrError, onCompleteOrError); } // mark the save participant as current pending save operation @@ -678,7 +687,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil etag: (options.ignoreModifiedSince || !this.filesConfigurationService.preventSaveConflicts(lastResolvedFileStat.resource, this.getMode())) ? ETAG_DISABLED : lastResolvedFileStat.etag, writeElevated: options.writeElevated }).then(stat => this.handleSaveSuccess(stat, versionId, options), error => this.handleSaveError(error, versionId, options))); - })); + }), () => saveParticipantCancellation.cancel()); } private handleSaveSuccess(stat: IFileStatWithMetadata, versionId: number, options: ITextFileSaveOptions): void { diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts index f211c5939fd..c3c1f68d02c 100644 --- a/src/vs/workbench/services/textfile/common/textfiles.ts +++ b/src/vs/workbench/services/textfile/common/textfiles.ts @@ -16,6 +16,7 @@ import { isUndefinedOrNull } from 'vs/base/common/types'; import { isNative } from 'vs/base/common/platform'; import { IWorkingCopy } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { IUntitledTextEditorModelManager } from 'vs/workbench/services/untitled/common/untitledTextEditorService'; +import { CancellationToken } from 'vs/base/common/cancellation'; export const ITextFileService = createDecorator('textFileService'); @@ -230,7 +231,7 @@ export interface ISaveParticipant { /** * Participate in a save of a model. Allows to change the model before it is being saved to disk. */ - participate(model: IResolvedTextFileEditorModel, env: { reason: SaveReason }): Promise; + participate(model: IResolvedTextFileEditorModel, context: { reason: SaveReason }, token: CancellationToken): Promise; } /** diff --git a/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts b/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts index d6bc866bf8f..61bddf39309 100644 --- a/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts +++ b/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts @@ -556,4 +556,34 @@ suite('Files - TextFileEditorModel', () => { await model.save(); model.dispose(); }); + + test('Save Participant, participant cancelled when saved again', async function () { + const model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined); + + let participations: boolean[] = []; + + accessor.textFileService.saveParticipant = { + participate: (model) => { + return timeout(10).then(() => { + participations.push(true); + }); + } + }; + + await model.load(); + + model.textEditorModel!.setValue('foo'); + const p1 = model.save(); + + model.textEditorModel!.setValue('foo 1'); + const p2 = model.save(); + + model.textEditorModel!.setValue('foo 2'); + await model.save(); + + await p1; + await p2; + assert.equal(participations.length, 1); + model.dispose(); + }); }); diff --git a/src/vs/workbench/services/textfile/test/common/saveSequenzializer.test.ts b/src/vs/workbench/services/textfile/test/common/saveSequenzializer.test.ts index c85a38bd8b3..097fea645ab 100644 --- a/src/vs/workbench/services/textfile/test/common/saveSequenzializer.test.ts +++ b/src/vs/workbench/services/textfile/test/common/saveSequenzializer.test.ts @@ -87,4 +87,14 @@ suite('Files - SaveSequentializer', () => { assert.ok(!secondDone); assert.ok(thirdDone); }); + + test('SaveSequentializer - cancel pending', async function () { + const sequentializer = new SaveSequentializer(); + + let pendingCancelled = false; + sequentializer.setPending(1, timeout(1), () => pendingCancelled = true); + sequentializer.cancelPending(); + + assert.ok(pendingCancelled); + }); }); -- GitLab