diff --git a/js/js-resources.gresource.xml b/js/js-resources.gresource.xml index e5e6167f150a7309db226abe9aca834ca1bb1391..85a53c8f6b1c6bbdfd277f3cf3e0889aab411674 100644 --- a/js/js-resources.gresource.xml +++ b/js/js-resources.gresource.xml @@ -14,6 +14,7 @@ extensions/extension.js extensions/sharedInternals.js + misc/asyncMutex.js misc/animationUtils.js misc/breakManager.js misc/brightnessManager.js diff --git a/js/misc/asyncMutex.js b/js/misc/asyncMutex.js new file mode 100644 index 0000000000000000000000000000000000000000..0ae52dad0e9e345466ee9d3e88c9e564ec1b2b8f --- /dev/null +++ b/js/misc/asyncMutex.js @@ -0,0 +1,29 @@ +export class AsyncMutex { + #isHold = false; + #pendingResolvers = []; + + hold() { + if (this.#isHold) { + const {promise, resolve} = Promise.withResolvers(); + this.#pendingResolvers.push(resolve); + return promise; + } + this.#isHold = true; + return Promise.resolve(); + } + + release() { + if (!this.#isHold) + return; + if (this.#pendingResolvers.length === 0) { + this.#isHold = false; + return; + } + const resolve = this.#pendingResolvers.shift(); + resolve(); + } + + get pendingResolves() { + return this.#pendingResolvers.length; + } +} diff --git a/js/ui/extensionSystem.js b/js/ui/extensionSystem.js index 1dc94b40b65ea39129034d3370ce92646fdeff3f..986c6e12df03816354f6e7fb1b79d6cba991cf1c 100644 --- a/js/ui/extensionSystem.js +++ b/js/ui/extensionSystem.js @@ -5,6 +5,7 @@ import St from 'gi://St'; import Shell from 'gi://Shell'; import * as Signals from '../misc/signals.js'; +import * as AsyncMutex from '../misc/asyncMutex.js'; import * as Config from '../misc/config.js'; import * as ExtensionDownloader from './extensionDownloader.js'; import {formatError} from '../misc/errorUtils.js'; @@ -22,6 +23,8 @@ const EXTENSION_DISABLE_VERSION_CHECK_KEY = 'disable-extension-version-validatio const UPDATE_CHECK_TIMEOUT = 24 * 60 * 60; // 1 day in seconds +const enableExtensionMutex = new AsyncMutex.AsyncMutex(); + function stateToString(state) { return Object.keys(ExtensionState).find(k => ExtensionState[k] === state); } @@ -562,6 +565,8 @@ export class ExtensionManager extends Signals.EventEmitter { } async _onEnabledExtensionsChanged() { + await enableExtensionMutex.hold(); + const newEnabledExtensions = this._getEnabledExtensions(); for (const extension of this._extensions.values()) { @@ -595,6 +600,8 @@ export class ExtensionManager extends Signals.EventEmitter { } this._enabledExtensions = newEnabledExtensions; + + enableExtensionMutex.release(); } _onSettingsWritableChanged() { diff --git a/tests/meson.build b/tests/meson.build index b9b4f3112638cd43b5ac54e092ece570e92f49a9..dcd4351e1fcbeffa27d993836fcedab15dc344be 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -29,6 +29,7 @@ unit_testenv.append('GI_TYPELIB_PATH', shell_typelib_path, separator: ':') unit_testenv.append('GI_TYPELIB_PATH', st_typelib_path, separator: ':') unit_tests = { + 'asyncMutex': {}, 'breakManager': {}, 'errorUtils': {}, 'extensionUtils': {}, diff --git a/tests/unit/asyncMutex.js b/tests/unit/asyncMutex.js new file mode 100755 index 0000000000000000000000000000000000000000..18e93194c0864464a266ade37ad6f057a221f656 --- /dev/null +++ b/tests/unit/asyncMutex.js @@ -0,0 +1,223 @@ +import GLib from 'gi://GLib'; + +import * as Mutex from 'resource:///org/gnome/shell/misc/asyncMutex.js'; + +/** + * A helper that asynchronously waits an specified amount + * of time in milliseconds. + * + * @param {Int} ms Time, in milliseconds, to wait before resolving the promise. + * @returns A promise that will be resolved after the specified time. + */ +function waitMs(ms) { + return new Promise((resolve, _reject) => { + setTimeout(resolve, ms); + }); +} + +const STATE_UNINITIALIZED = -1; +const STATE_HOLD = 0; +const STATE_RUN = 1; +const STATE_RELEASE = 2; +const STATE_DONE = 3; + +class baseTest { + #counterWithMutex = 0; + #mutex; + #loop; + #promiseList = []; + #messages = []; + #workercounter = 0; + + constructor() { + this.#mutex = new Mutex.AsyncMutex(); + this.#loop = new GLib.MainLoop(null, false); + } + + _disableMutex() { + this.#mutex = null; + } + + async _workerWithMutex(index) { + this.#messages.push({'state': STATE_HOLD, 'id': index}); + if (this.#mutex) + await this.#mutex.hold(); + this.#messages.push({'state': STATE_RUN, 'id': index}); + const value = this.#counterWithMutex; + await waitMs(200); + this.#messages.push({'state': STATE_RELEASE, 'id': index}); + this.#counterWithMutex = value + 1; + if (this.#mutex) + this.#mutex.release(); + this.#messages.push({'state': STATE_DONE, 'id': index}); + } + + _appendWorker() { + this.#promiseList.push(this._workerWithMutex(this.#workercounter++)); + } + + _runLoop() { + Promise.all(this.#promiseList).then(() => { + this.#loop.quit(); + }); + this.#loop.run(); + } + + get counter() { + return this.#counterWithMutex; + } + + #checkWorkerCompletedFine(workerId) { + // Ensures that the specified worker passes in order by all the states. + let lastState = STATE_UNINITIALIZED; + for (const msg of this.#messages) { + if (msg.id !== workerId) + continue; + if (msg.state !== (lastState + 1)) + return false; + lastState = msg.state; + } + return lastState === STATE_DONE; + } + + checkWorkersCompletedFine() { + // Ensures that all the workers pass in order by all the states. + for (let i = 0; i < this.#workercounter; i++) { + if (!this.#checkWorkerCompletedFine(i)) + return false; + } + return true; + } + + checkNoRaceConditions() { + // Ensures that no worker has STATE_RUN or STATE_RELEASE between the STATE_HOLD and the + // STATE_RELEASE of other worker, thus ensuring that the mutex works as expected. + let currentWorker = -1; + const onHold = []; + for (const msg of this.#messages) { + switch (msg.state) { + case STATE_HOLD: + if (currentWorker === -1) + currentWorker = msg.id; + else + onHold.push(msg.id); + break; + case STATE_RUN: + if (currentWorker !== -1) { + // Ensure that the worker that enters RUN state is the one that owns the mutex + expect(msg.id).toBe(currentWorker); + } else { + // the mutex is released and a new worker has taken ownership of it + currentWorker = msg.id; + + // Ensure that the worker did a HOLD before + const idx = onHold.indexOf(msg.id); + expect(idx).not.toBe(-1); + + // remove the id from the onHold list + onHold.splice(onHold.indexOf(msg.id), 1); + } + break; + case STATE_RELEASE: + // Ensure that the worker that releases the mutex is the one that owns it + expect(currentWorker).toBe(msg.id); + currentWorker = -1; + break; + } + } + return true; + } +} + + +/* This test confirms that the race condition happens */ +class mutexTestRaceCondition extends baseTest { + run() { + this._disableMutex(); + + this._appendWorker(); + this._appendWorker(); + this._appendWorker(); + this._appendWorker(); + + this._runLoop(); + + expect(this.checkWorkersCompletedFine()).toBe(true); + expect(this.counter).not.toBe(4); + } +} + + +/* This test confirms that the asyncMutex works when called once */ +class mutexTestAsyncMutex extends baseTest { + run() { + this._appendWorker(); + this._runLoop(); + + expect(this.checkWorkersCompletedFine()).toBe(true); + expect(this.counter).toBe(1); + } +} + +/* This test confirms that the asyncMutex fixes the race condition */ +class mutexTestAsyncMutexFix extends baseTest { + run() { + this._appendWorker(); + this._appendWorker(); + this._appendWorker(); + this._appendWorker(); + + this._runLoop(); + + expect(this.checkWorkersCompletedFine()).toBe(true); + expect(this.counter).toBe(4); + expect(this.checkNoRaceConditions()).toBe(true); + } +} + +/* This test confirms that several asyncMutex can be used + symultaneously */ +class mutexTestMultipleUse { + run() { + const mutexA = new Mutex.AsyncMutex(); + const mutexB = new Mutex.AsyncMutex(); + const mutexC = new Mutex.AsyncMutex(); + + mutexA.hold(); + expect(mutexA.pendingResolves).toBe(0); + + mutexB.hold(); + expect(mutexB.pendingResolves).toBe(0); + + mutexA.hold(); + expect(mutexA.pendingResolves).toBe(1); + + mutexB.hold(); + expect(mutexB.pendingResolves).toBe(1); + + mutexC.hold(); + expect(mutexC.pendingResolves).toBe(0); + + mutexC.hold(); + expect(mutexC.pendingResolves).toBe(1); + } +} + +describe('AsyncMutex Test', () => { + it('Confirm that the race condition happens', () => { + const test = new mutexTestRaceCondition(); + test.run(); + }); + it('Test that AsyncMutex works with one worker', () => { + const test = new mutexTestAsyncMutex(); + test.run(); + }); + it('Test that AsyncMutex works with several concurrent workers', () => { + const test = new mutexTestAsyncMutexFix(); + test.run(); + }); + it('Test that several AsyncMutex can be used at the same time without interference', () => { + const test = new mutexTestMultipleUse(); + test.run(); + }); +});