From 3d8067e947fe35ab523e62163735ee1ac64c2f08 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 23 Feb 2025 10:41:59 +0000 Subject: [PATCH] Stop using global file locks --- .../src/sync-operations/document-lock.ts | 48 ------------- ...nt-lock.test.ts => document-locks.test.ts} | 69 ++++++++++--------- .../src/sync-operations/document-locks.ts | 50 ++++++++++++++ 3 files changed, 85 insertions(+), 82 deletions(-) delete mode 100644 frontend/sync-client/src/sync-operations/document-lock.ts rename frontend/sync-client/src/sync-operations/{document-lock.test.ts => document-locks.test.ts} (50%) create mode 100644 frontend/sync-client/src/sync-operations/document-locks.ts diff --git a/frontend/sync-client/src/sync-operations/document-lock.ts b/frontend/sync-client/src/sync-operations/document-lock.ts deleted file mode 100644 index 28d97f35..00000000 --- a/frontend/sync-client/src/sync-operations/document-lock.ts +++ /dev/null @@ -1,48 +0,0 @@ -import type { RelativePath } from "../persistence/database"; - -const locked = new Set(); -const waiters = new Map void)[]>(); - -export function tryLockDocument(relativePath: RelativePath): boolean { - if (locked.has(relativePath)) { - return false; - } - - locked.add(relativePath); - return true; -} - -export async function waitForDocumentLock( - relativePath: RelativePath -): Promise { - if (tryLockDocument(relativePath)) { - return Promise.resolve(); - } - - return new Promise((resolve) => { - let waiting = waiters.get(relativePath); - if (!waiting) { - waiting = []; - waiters.set(relativePath, waiting); - } - - waiting.push(resolve); - }); -} - -export function unlockDocument(relativePath: RelativePath): void { - if (!locked.has(relativePath)) { - throw new Error( - `Document ${relativePath} is not locked, cannot unlock` - ); - } - - // Remove the first element to ensure FIFO unblocking order - const nextWaiting = waiters.get(relativePath)?.shift(); - - if (nextWaiting) { - nextWaiting(); - } else { - locked.delete(relativePath); - } -} diff --git a/frontend/sync-client/src/sync-operations/document-lock.test.ts b/frontend/sync-client/src/sync-operations/document-locks.test.ts similarity index 50% rename from frontend/sync-client/src/sync-operations/document-lock.test.ts rename to frontend/sync-client/src/sync-operations/document-locks.test.ts index 2bc05e69..ce661d02 100644 --- a/frontend/sync-client/src/sync-operations/document-lock.test.ts +++ b/frontend/sync-client/src/sync-operations/document-locks.test.ts @@ -1,89 +1,90 @@ -import { RelativePath } from "../persistence/database"; -import { - tryLockDocument, - waitForDocumentLock, - unlockDocument -} from "./document-lock"; +import type { RelativePath } from "../persistence/database"; +import { DocumentLocks } from "./document-locks"; -describe("Document Lock Operations", () => { +describe("Document lock", () => { const testPath: RelativePath = "test/document/path"; + let locks = new DocumentLocks(); beforeEach(() => { - // Reset the state before each test - (global as any).locked = new Set(); - (global as any).waiters = new Map void)[]>(); + locks = new DocumentLocks(); }); test("should lock a document successfully", () => { - const result = tryLockDocument(testPath); + const result = locks.tryLockDocument(testPath); expect(result).toBe(true); }); test("should not lock a document that is already locked", () => { - tryLockDocument(testPath); - const result = tryLockDocument(testPath); + locks.tryLockDocument(testPath); + const result = locks.tryLockDocument(testPath); expect(result).toBe(false); }); test("should unlock a locked document", () => { - tryLockDocument(testPath); - unlockDocument(testPath); - const result = tryLockDocument(testPath); + locks.tryLockDocument(testPath); + locks.unlockDocument(testPath); + const result = locks.tryLockDocument(testPath); expect(result).toBe(true); - unlockDocument(testPath); + locks.unlockDocument(testPath); }); test("should throw an error when unlocking a document that is not locked", () => { expect(() => { - unlockDocument(testPath); + locks.unlockDocument(testPath); }).toThrow(`Document ${testPath} is not locked, cannot unlock`); }); test("should wait for a document lock and resolve when unlocked", async () => { - tryLockDocument(testPath); + locks.tryLockDocument(testPath); let resolved = false; - const waitPromise = waitForDocumentLock(testPath).then(() => { + const waitPromise = locks.waitForDocumentLock(testPath).then(() => { resolved = true; }); - unlockDocument(testPath); + locks.unlockDocument(testPath); await waitPromise; expect(resolved).toBe(true); }); test("should resolve multiple waiters in FIFO order", async () => { - tryLockDocument(testPath); + locks.tryLockDocument(testPath); let firstResolved = false; let secondResolved = false; let thirdResolved = false; - const firstWaitPromise = waitForDocumentLock(testPath).then(() => { - firstResolved = true; - }); + const firstWaitPromise = locks + .waitForDocumentLock(testPath) + .then(() => { + firstResolved = true; + }); - const secondWaitPromise = waitForDocumentLock(testPath).then(() => { - secondResolved = true; - }); + const secondWaitPromise = locks + .waitForDocumentLock(testPath) + .then(() => { + secondResolved = true; + }); - const thirdWaitPromise = waitForDocumentLock(testPath).then(() => { - thirdResolved = true; - }); + const thirdWaitPromise = locks + .waitForDocumentLock(testPath) + .then(() => { + thirdResolved = true; + }); - unlockDocument(testPath); + locks.unlockDocument(testPath); await firstWaitPromise; expect(firstResolved).toBe(true); expect(secondResolved).toBe(false); expect(thirdResolved).toBe(false); - unlockDocument(testPath); + locks.unlockDocument(testPath); await secondWaitPromise; expect(secondResolved).toBe(true); expect(thirdResolved).toBe(false); - unlockDocument(testPath); + locks.unlockDocument(testPath); await thirdWaitPromise; expect(thirdResolved).toBe(true); }); diff --git a/frontend/sync-client/src/sync-operations/document-locks.ts b/frontend/sync-client/src/sync-operations/document-locks.ts new file mode 100644 index 00000000..f1831f82 --- /dev/null +++ b/frontend/sync-client/src/sync-operations/document-locks.ts @@ -0,0 +1,50 @@ +import type { RelativePath } from "../persistence/database"; + +export class DocumentLocks { + private readonly locked = new Set(); + private readonly waiters = new Map void)[]>(); + + public tryLockDocument(relativePath: RelativePath): boolean { + if (this.locked.has(relativePath)) { + return false; + } + + this.locked.add(relativePath); + return true; + } + + public async waitForDocumentLock( + relativePath: RelativePath + ): Promise { + if (this.tryLockDocument(relativePath)) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + let waiting = this.waiters.get(relativePath); + if (!waiting) { + waiting = []; + this.waiters.set(relativePath, waiting); + } + + waiting.push(resolve); + }); + } + + public unlockDocument(relativePath: RelativePath): void { + if (!this.locked.has(relativePath)) { + throw new Error( + `Document ${relativePath} is not locked, cannot unlock` + ); + } + + // Remove the first element to ensure FIFO unblocking order + const nextWaiting = this.waiters.get(relativePath)?.shift(); + + if (nextWaiting) { + nextWaiting(); + } else { + this.locked.delete(relativePath); + } + } +}