Check for locked after unlocked

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

@@ -157,4 +157,36 @@ 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 = [];
+        const searchPref = 'search.searchOnTypeDebouncePeriod'
+        const channelPref = 'output.maxChannelHistory'
+        const hoverPref = 'workbench.hover.delay';
+        let searchDebounce;
+        let channelHistory;
+        let hoverDelay;
+        /** @type import ('@theia/core/src/browser/preferences/preference-service').PreferenceChanges | undefined */
+        let event;
+        const toDispose = preferenceService.onPreferencesChanged(e => event = e);
+        while (settings++ < 50) {
+            searchDebounce = 100 + Math.floor(Math.random() * 500);
+            channelHistory = 200 + Math.floor(Math.random() * 800);
+            hoverDelay = 250 + Math.floor(Math.random() * 2_500);
+            promises.push(
+                preferenceService.set(searchPref, searchDebounce),
+                preferenceService.set(channelPref, channelHistory),
+                preferenceService.set(hoverPref, hoverDelay)
+            );
+        }
+        const results = await Promise.allSettled(promises);
+        const expectedValues = { [searchPref]: searchDebounce, [channelPref]: channelHistory, [hoverPref]: hoverDelay };
+        const actualValues = { [searchPref]: preferenceService.get(searchPref), [channelPref]: preferenceService.get(channelPref), [hoverPref]: preferenceService.get(hoverPref), }
+        const eventValues = event && Object.keys(event).reduce((accu, key) => { accu[key] = event[key].newValue; return accu; }, {});
+        toDispose.dispose();
+        assert(results.every(setting => setting.status === 'fulfilled'), 'All promises should have resolved rather than rejected.');
+        assert.deepEqual(actualValues, eventValues, 'The event should reflect the current state of the service.');
+        assert.deepEqual(expectedValues, actualValues, 'The service state should reflect the most recent setting');
+    });
 });

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

@@ -115,6 +115,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
         if (!this.transaction?.open) {
             const current = this.transaction;
             this.transaction = this.transactionFactory(this);
+            this.transaction.waitFor(current?.result);
             this.transaction.onWillConclude(({ status, waitUntil }) => {
                 if (status) {
                     waitUntil((async () => {
@@ -124,7 +125,6 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
                 }
             });
             this.toDispose.push(this.transaction);
-            await current?.result;
         }
         return this.transaction.enqueueAction(key, path, value);
     }

packages/preferences/src/browser/preference-transaction-manager.ts

@@ -82,6 +82,16 @@ export abstract class Transaction<Arguments extends unknown[], Result = unknown,
         }
     }
 
+    async waitFor(delay?: Promise<unknown>, disposeIfRejected?: boolean): Promise<void> {
+        try {
+            await this.queue.runExclusive(() => delay);
+        } catch {
+            if (disposeIfRejected) {
+                this.dispose();
+            }
+        }
+    }
+
     /**
      * @returns a promise reflecting the result of performing an action. Typically the promise will not resolve until the whole transaction is complete.
      */
@@ -92,7 +102,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 +111,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 {