add locking queue

examples/api-tests/src/saveable.spec.js

@@ -554,7 +554,7 @@ describe('Saveable', function () {
         }
     });
 
-    it('saves preference file when open and not dirty', async function () {
+    it.only('saves preference file when open and not dirty', async function () {
         const prefName = 'editor.copyWithSyntaxHighlighting';
         const prefTest = await setUpPrefsTest([prefName]);
         if (!prefTest) { return; }
@@ -567,27 +567,31 @@ describe('Saveable', function () {
         assert.equal(1, record.calls, 'save should have been called one time.');
     });
 
-    it('saves once when many edits are made (editor open)', async function () {
-        const prefNames = ['editor.copyWithSyntaxHighlighting', 'debug.inlineValues'];
+    it.only('saves once when many edits are made (editor open)', async function () {
+        const incrementablePreference = 'diffEditor.maxComputationTime';
+        const booleanPreference = 'editor.copyWithSyntaxHighlighting';
+        const prefNames = [incrementablePreference, booleanPreference];
         const prefTest = await setUpPrefsTest(prefNames);
         if (!prefTest) { return; }
 
         const { record, initialValues, editorWidget } = prefTest;
+        const targetScope = rootUri.toString();
 
         /** @type {Promise<void>[]} */
         const attempts = [];
+        let booleanSwap = initialValues[0];
         while (attempts.length < 250) {
-            prefNames.forEach((prefName, index) => {
-                const value = attempts.length % 2 === 0 ? !initialValues[index] : initialValues[index];
-                attempts.push(preferences.set(prefName, value, undefined, rootUri.toString()));
-            });
+            booleanSwap = !booleanSwap;
+            attempts.push(preferences.set(booleanPreference, booleanSwap, undefined, targetScope));
+            attempts.push(preferences.set(incrementablePreference, attempts.length, undefined, targetScope));
         }
         await Promise.all(attempts);
         assert.isFalse(Saveable.isDirty(editorWidget), "editor should not be dirty if it wasn't dirty before");
         assert.equal(1, record.calls, 'save should have been called one time.');
+        assert.equal(attempts.length - 1, preferences.get(incrementablePreference, undefined, targetScope), 'The final setting should be in effect.');
     });
 
-    it('saves once when many edits are made (editor closed)', async function () {
+    it.only('saves once when many edits are made (editor closed)', async function () {
         const prefNames = ['editor.copyWithSyntaxHighlighting', 'debug.inlineValues'];
         const prefTest = await setUpPrefsTest(prefNames);
         if (!prefTest) { return; }
@@ -607,7 +611,7 @@ describe('Saveable', function () {
         assert.equal(1, record.calls, 'save should have been called one time.');
     });
 
-    it('displays the toast once no matter how many edits are queued', async function () {
+    it.only('displays the toast once no matter how many edits are queued', async function () {
         const prefNames = ['editor.copyWithSyntaxHighlighting', 'debug.inlineValues'];
         const prefTest = await setUpPrefsTest(prefNames);
         if (!prefTest) { return; }

packages/preferences/src/browser/abstract-resource-preference-provider.ts

@@ -30,13 +30,55 @@ import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model
 import { MonacoWorkspace } from '@theia/monaco/lib/browser/monaco-workspace';
 import { Deferred } from '@theia/core/lib/common/promise-util';
 import { EditorManager } from '@theia/editor/lib/browser';
+import { Emitter } from '@theia/core/lib/common';
 
 interface EnqueuedPreferenceChange {
     key: string,
     value: any,
     editResolver: Deferred<boolean | Promise<boolean>>,
 }
 
+class LockQueue {
+    protected readonly queue: Deferred<void>[] = [];
+    protected busy: boolean = false;
+    protected readonly onFreeEmitter = new Emitter<void>();
+    readonly onFree = this.onFreeEmitter.event;
+
+    acquire(): Promise<void> {
+        const wasBusy = this.busy;
+        this.busy = true;
+        if (wasBusy) {
+            const addedToQueue = new Deferred<void>();
+            this.queue.push(addedToQueue);
+            return addedToQueue.promise;
+        }
+        return Promise.resolve();
+    }
+
+    release(): void {
+        const next = this.queue.shift();
+        if (next) {
+            next.resolve();
+        } else {
+            this.busy = false;
+            this.onFreeEmitter.fire();
+        }
+    }
+
+    waitUntilFree(): Promise<void> {
+        if (!this.busy) {
+            return Promise.resolve();
+        }
+
+        return new Promise(resolve => {
+            const toDisposeOnFree = this.onFree(() => {
+                resolve();
+                toDisposeOnFree.dispose();
+            });
+        });
+    }
+}
+
 @injectable()
 export abstract class AbstractResourcePreferenceProvider extends PreferenceProvider {
 
@@ -45,7 +87,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
     protected editQueue: EnqueuedPreferenceChange[] = [];
     protected readonly loading = new Deferred();
     protected dirtyEditorHandled = new Deferred();
-    protected pendingTransaction = new Deferred();
+    protected fileEditLock = new LockQueue();
 
     @inject(PreferenceService) protected readonly preferenceService: PreferenceService;
     @inject(MessageService) protected readonly messageService: MessageService;
@@ -69,7 +111,6 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
         this.toDispose.push(Disposable.create(() => this.loading.reject(new Error(`preference provider for '${uri}' was disposed`))));
         this._ready.resolve();
         this.dirtyEditorHandled.resolve();
-        this.pendingTransaction.resolve();
 
         const reference = await this.textModelService.createModelReference(uri);
         if (this.toDispose.disposed) {
@@ -125,25 +166,29 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
 
     async setPreference(key: string, value: any, resourceUri?: string): Promise<boolean> {
         await this.loading.promise;
-        // Wait for save of previous transaction
-        await this.pendingTransaction.promise;
         if (!this.model) {
             return false;
         }
         if (!this.contains(resourceUri)) {
             return false;
         }
+
+        // Wait for save of previous transaction to prevent false positives on model.dirty when a progrommatic save is  in progress.
+        await this.fileEditLock.waitUntilFree();
         const isFirstEditInQueue = !this.editQueue.length;
         const mustHandleDirty = isFirstEditInQueue && this.model.dirty;
+        const editResolver = new Deferred<boolean | Promise<boolean>>();
+        this.editQueue.push({ key, value, editResolver });
+
         if (mustHandleDirty) {
             this.dirtyEditorHandled = new Deferred();
             this.handleDirtyEditor();
         }
-        const editResolver = new Deferred<boolean | Promise<boolean>>();
-        this.editQueue.push({ key, value, editResolver });
+
         if (isFirstEditInQueue) {
             this.doSetPreferences();
         }
+
         return editResolver.promise;
     }
 
@@ -158,7 +203,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
         let success = true;
         // Defer new actions until transaction complete.
         // Prevents the dirty-editor toast from appearing if save cycle in progress.
-        this.pendingTransaction = new Deferred();
+        await this.fileEditLock.acquire();
         try {
             if (this.model?.dirty) {
                 await this.model.save();
@@ -168,7 +213,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
         } catch {
             success = false;
         }
-        this.pendingTransaction.resolve();
+        this.fileEditLock.release();
         localPendingChanges.resolve(success);
     }