Check for locked after unlocked

eclipse-theia · Mar 15, 2022 · 97be6e1 · 97be6e1
1 parent 66a7b8c
commit 97be6e1
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git a/examples/api-tests/src/preferences.spec.js b/examples/api-tests/src/preferences.spec.js
@@ -157,4 +157,27 @@ describe('Preferences', function () {
         const prefs = await getPreferences();
         shouldBeUndefined(prefs[override], override);
     });
+
+    it('Handles many synchronous settings of preferences gracefully', async function () {
+        let settings = 0;
+        const promises = [];
+        let fontSize;
+        let tabSize;
+        let hoverDelay;
+        while (settings++ < 100) {
+            fontSize = 8 + Math.floor(Math.random() * 12);
+            tabSize = 1 + Math.floor(Math.random() * 12);
+            hoverDelay = 250 + Math.floor(Math.random() * 2_500);
+            promises.push(
+                preferenceService.set('editor.fontSize', fontSize),
+                preferenceService.set('editor.tabSize', tabSize),
+                preferenceService.set('editor.hover.delay', hoverDelay)
+            );
+        }
+        const results = await Promise.allSettled(promises);
+        assert(results.every(setting => setting.status === 'fulfilled'), 'All promises should have resolved rather than rejected.');
+        assert.equal(fontSize, preferenceService.get('editor.fontSize'), 'The last setting should have taken effect.');
+        assert.equal(tabSize, preferenceService.get('editor.tabSize'), 'The last setting should have taken effect.');
+        assert.equal(hoverDelay, preferenceService.get('editor.hover.delay'), 'The last setting should have taken effect');
+    })
 });
diff --git a/packages/preferences/src/browser/preference-transaction-manager.ts b/packages/preferences/src/browser/preference-transaction-manager.ts
@@ -92,7 +92,7 @@ export abstract class Transaction<Arguments extends unknown[], Result = unknown,
                 release = await this.queue.acquire();
                 if (!this.inUse) {
                     this.inUse = true;
-                    this.queue.waitForUnlock().then(() => this.dispose());
+                    this.disposeWhenDone();
                 }
                 return this.act(...args);
             } catch (e) {
@@ -101,15 +101,25 @@ export abstract class Transaction<Arguments extends unknown[], Result = unknown,
                 }
                 return false;
             } finally {
-                if (release) {
-                    release();
-                }
+                release?.();
             }
         } else {
             throw new Error('Transaction used after disposal.');
         }
     }
 
+    protected disposeWhenDone(): void {
+        // Due to properties of the micro task system, it's possible for something to have been equeued between
+        // the resolution of the waitForUnlock() promise and the the time this code runs, so we have to check.
+        this.queue.waitForUnlock().then(() => {
+            if (!this.queue.isLocked()) {
+                this.dispose();
+            } else {
+                this.disposeWhenDone();
+            }
+        });
+    }
+
     protected async conclude(): Promise<void> {
         if (this._open) {
             try {