diff --git a/src/fileStorage/sagas.ts b/src/fileStorage/sagas.ts index 424979ec..c4db64aa 100644 --- a/src/fileStorage/sagas.ts +++ b/src/fileStorage/sagas.ts @@ -10,7 +10,7 @@ import { take, takeEvery, } from 'typed-redux-saga/macro'; -import { defined, ensureError } from '../utils'; +import { acquireLock, defined, ensureError } from '../utils'; import { sha256Digest } from '../utils/crypto'; import { createCountFunc } from '../utils/iter'; import { @@ -77,36 +77,12 @@ function* handleOpen( ): Generator { try { const fd = nextFd(); - let lockWaiter: Promise; - const close = yield* call( - () => - new Promise<(() => void) | void>((resolve, reject) => { - lockWaiter = navigator.locks - .request( - lockNameForPath(action.path), - { - ifAvailable: true, - mode: action.mode === 'w' ? 'exclusive' : 'shared', - }, - (lock) => { - if (lock === null) { - resolve(); - return; - } - - // capture a promise new resolve function that will be used - // to release the lock later - return new Promise((resolve2) => - resolve(resolve2), - ); - }, - ) - .catch(reject); - }), + const releaseLock = yield* call(() => + acquireLock(lockNameForPath(action.path), action.mode !== 'w'), ); - if (!close) { + if (!releaseLock) { throw new Error(`file '${action.path}' is already in use`); } @@ -162,10 +138,9 @@ function* handleOpen( isCloseExplicitlyRequested = true; } finally { openFds.delete(fd); - close(); // this ensures that the lock is released before we send the action - yield* call(() => lockWaiter); + yield* call(() => releaseLock()); // Post fileStorageDidClose only if fileStorageClose was received. // If the task is canceled or fails, we don't want this extra action. diff --git a/src/utils/index.test.ts b/src/utils/index.test.ts index fb80543b..2f1b7dc7 100644 --- a/src/utils/index.test.ts +++ b/src/utils/index.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2022 The Pybricks Authors -import { assert, defined, ensureError, hex, maybe, timestamp } from '.'; +import { acquireLock, assert, defined, ensureError, hex, maybe, timestamp } from '.'; test('assert', () => { const assertTrue = jest.fn(() => assert(true, 'should not throw')); @@ -62,3 +62,93 @@ describe('timestamp', () => { expect(timestamp()).not.toMatch(/[A-Za-z]/); }); }); + +describe('acquireLock', () => { + it.each([true, false])('should acquire lock when shared is %o', async () => { + const releaseLock = await acquireLock('test'); + try { + expect(releaseLock).toBeDefined(); + } finally { + await releaseLock?.(); + } + }); + + it.each([true, false])( + 'should fail to acquire exclusive second lock when first lock shared is %o', + async (shared) => { + const releaseLock = await acquireLock('test', shared); + try { + const releaseLock2 = await acquireLock('test'); + try { + expect(releaseLock2).toBeUndefined(); + } finally { + await releaseLock2?.(); + } + } finally { + await releaseLock?.(); + } + }, + ); + + it('should acquire shared second lock when first lock is shared', async () => { + const releaseLock = await acquireLock('test', true); + try { + const releaseLock2 = await acquireLock('test', true); + try { + expect(releaseLock2).toBeDefined(); + } finally { + await releaseLock2?.(); + } + } finally { + await releaseLock?.(); + } + }); + + it('should fail to acquire shared second lock when first lock is exclusive', async () => { + const releaseLock = await acquireLock('test'); + try { + const releaseLock2 = await acquireLock('test', true); + try { + expect(releaseLock2).toBeUndefined(); + } finally { + await releaseLock2?.(); + } + } finally { + await releaseLock?.(); + } + }); + + it('should acquire second lock when first lock is released', async () => { + const releaseLock = await acquireLock('test'); + try { + expect(releaseLock).toBeDefined(); + } finally { + await releaseLock?.(); + } + + const releaseLock2 = await acquireLock('test'); + try { + expect(releaseLock2).toBeDefined(); + } finally { + await releaseLock2?.(); + } + }); + + it('should fail to acquire second lock when first lock is released but release is not awaited', async () => { + const releaseLock = await acquireLock('test'); + try { + expect(releaseLock).toBeDefined(); + // not awaited! + releaseLock?.(); + + const releaseLock2 = await acquireLock('test'); + try { + expect(releaseLock2).toBeUndefined(); + } finally { + await releaseLock2?.(); + } + } finally { + await releaseLock?.(); + } + }); +}); diff --git a/src/utils/index.ts b/src/utils/index.ts index c1d8094e..1b154e1a 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020-2021 The Pybricks Authors +// Copyright (c) 2020-2022 The Pybricks Authors /** * Asserts that an assumption is true. This is used to detect programmer errors @@ -75,3 +75,58 @@ export function timestamp(): string { .replaceAll(':', '-') .replace(/\..*$/, ''); } + +/** + * Helper function to wrap navigator.locks in a promise so that it can be used + * in code where using it natively doesn't work well (e.g. in sagas). Care must + * be taken so that all code paths (including exceptions) release the lock. + * + * To release the lock, await the returned release function. When the release + * function resolves, the lock will no longer be held. + * + * @param name The name of the lock. + * @param shared If true, the lock will be share (e.g. for reading), otherwise + * the lock will be exclusive (e.g. for writing). + * @returns A release function if the lock was acquired or nothing if the lock + * was already held exclusively by someone else. + */ +export async function acquireLock( + name: string, + shared?: boolean, +): Promise<(() => Promise) | void> { + let lockWaiter: Promise; + + const release = await new Promise<(() => void) | void>((resolve, reject) => { + lockWaiter = navigator.locks + .request( + name, + { + ifAvailable: true, + mode: shared ? 'shared' : 'exclusive', + }, + (lock) => { + // if the locks is already held, lock will be null here + if (lock === null) { + resolve(); + return; + } + + // Now we own the lock and it will be held until the returned + // promise is resolved. + return new Promise((resolve2) => resolve(resolve2)); + }, + ) + .catch(reject); + }); + + if (!release) { + return; + } + + return async () => { + // trigger the release + release(); + // then wait until the release is complete and the lock is no longer held + await lockWaiter; + }; +}