Simplify syncing logic
This commit is contained in:
parent
e8c57b3a37
commit
4493365076
48 changed files with 1054 additions and 918 deletions
|
|
@ -5,18 +5,20 @@ import type { RelativePath } from "../../persistence/database";
|
|||
import { Locks } from "./locks";
|
||||
import { awaitAll } from "../await-all";
|
||||
import { sleep } from "../sleep";
|
||||
import { SyncResetError } from "../../services/sync-reset-error";
|
||||
import { SyncResetError } from "../../errors/sync-reset-error";
|
||||
|
||||
describe("withLock", () => {
|
||||
const testPath: RelativePath = "test/document/path";
|
||||
const testPath2: RelativePath = "test/document/path2";
|
||||
const testPath3: RelativePath = "test/document/path3";
|
||||
|
||||
const logger = new Logger();
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/init-declarations
|
||||
let locks: Locks<RelativePath>;
|
||||
|
||||
beforeEach(() => {
|
||||
locks = new Locks<RelativePath>(logger);
|
||||
locks = new Locks<RelativePath>("locks-test", logger);
|
||||
});
|
||||
|
||||
it("should execute function with single key lock", async () => {
|
||||
|
|
@ -56,22 +58,32 @@ describe("withLock", () => {
|
|||
it("should sort multiple keys to prevent deadlocks", async () => {
|
||||
const executionOrder: string[] = [];
|
||||
|
||||
// Start two concurrent operations with keys in different orders
|
||||
const promise1 = locks.withLock([testPath2, testPath], async () => {
|
||||
executionOrder.push("operation1-start");
|
||||
await sleep(50);
|
||||
executionOrder.push("operation1-end");
|
||||
return "result1";
|
||||
});
|
||||
await locks.waitForLock(testPath);
|
||||
|
||||
const promise2 = locks.withLock([testPath, testPath2], async () => {
|
||||
executionOrder.push("operation2-start");
|
||||
await sleep(50);
|
||||
executionOrder.push("operation2-end");
|
||||
return "result2";
|
||||
});
|
||||
const promise = awaitAll([
|
||||
locks.withLock([testPath2, testPath3, testPath], async () => {
|
||||
executionOrder.push("operation1-start");
|
||||
executionOrder.push("operation1-end");
|
||||
return "result1";
|
||||
}),
|
||||
|
||||
const [result1, result2] = await awaitAll([promise1, promise2]);
|
||||
locks.withLock([testPath3, testPath, testPath2], async () => {
|
||||
executionOrder.push("operation2-start");
|
||||
executionOrder.push("operation2-end");
|
||||
return "result2";
|
||||
})
|
||||
]);
|
||||
|
||||
locks.unlock(testPath);
|
||||
|
||||
const [result1, result2] = await Promise.race([
|
||||
promise,
|
||||
new Promise<never>((_, reject) => {
|
||||
setTimeout(() => {
|
||||
reject(new Error("Deadlock detected"));
|
||||
}, 1000);
|
||||
})
|
||||
]);
|
||||
|
||||
assert.strictEqual(result1, "result1");
|
||||
assert.strictEqual(result2, "result2");
|
||||
|
|
@ -234,13 +246,14 @@ describe("withLock", () => {
|
|||
|
||||
describe("reset", () => {
|
||||
const testPath: RelativePath = "test/document/path";
|
||||
const testPath2: RelativePath = "test/document/path2";
|
||||
const logger = new Logger();
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/init-declarations
|
||||
let locks: Locks<RelativePath>;
|
||||
|
||||
beforeEach(() => {
|
||||
locks = new Locks<RelativePath>(logger);
|
||||
locks = new Locks<RelativePath>("locks-test", logger);
|
||||
});
|
||||
|
||||
it("should reject pending waiters with SyncResetError while running operation completes", async () => {
|
||||
|
|
@ -252,7 +265,7 @@ describe("reset", () => {
|
|||
await sleep(1);
|
||||
|
||||
const secondPromise = locks.withLock(testPath, async () => "second");
|
||||
void secondPromise.catch(() => {}); // eslint-disable-line @typescript-eslint/no-empty-function
|
||||
void secondPromise.catch(() => { }); // eslint-disable-line @typescript-eslint/no-empty-function
|
||||
|
||||
locks.reset();
|
||||
|
||||
|
|
@ -273,7 +286,7 @@ describe("reset", () => {
|
|||
await sleep(1);
|
||||
|
||||
const secondPromise = locks.withLock(testPath, async () => "second");
|
||||
void secondPromise.catch(() => {}); // eslint-disable-line @typescript-eslint/no-empty-function
|
||||
void secondPromise.catch(() => { }); // eslint-disable-line @typescript-eslint/no-empty-function
|
||||
|
||||
locks.reset();
|
||||
|
||||
|
|
@ -289,4 +302,38 @@ describe("reset", () => {
|
|||
const result = await locks.withLock(testPath, () => "success");
|
||||
assert.strictEqual(result, "success");
|
||||
});
|
||||
|
||||
it("should release partially acquired locks when reset interrupts multi-key acquisition", async () => {
|
||||
// Hold testPath2 so multi-key acquisition will block on it
|
||||
await locks.waitForLock(testPath2);
|
||||
|
||||
// Start multi-key lock that will acquire testPath first, then block on testPath2
|
||||
const multiKeyPromise = locks.withLock(
|
||||
[testPath, testPath2],
|
||||
async () => "multi"
|
||||
);
|
||||
void multiKeyPromise.catch(() => { }); // eslint-disable-line @typescript-eslint/no-empty-function
|
||||
|
||||
// Wait for the multi-key operation to acquire testPath and start waiting on testPath2
|
||||
await sleep(10);
|
||||
|
||||
// Reset should reject the waiting operation
|
||||
locks.reset();
|
||||
|
||||
await assert.rejects(multiKeyPromise, (err: Error) => {
|
||||
assert.ok(err instanceof SyncResetError);
|
||||
return true;
|
||||
});
|
||||
|
||||
// The key that was already acquired (testPath) should now be released
|
||||
// This would hang/timeout if the lock was leaked
|
||||
const result = await Promise.race([
|
||||
locks.withLock(testPath, () => "success"),
|
||||
sleep(100).then(() => {
|
||||
throw new Error("Lock was not released - deadlock detected");
|
||||
})
|
||||
]);
|
||||
|
||||
assert.strictEqual(result, "success");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
import { SyncResetError } from "../../services/sync-reset-error";
|
||||
import { SyncResetError } from "../../errors/sync-reset-error";
|
||||
import type { Logger } from "../../tracing/logger";
|
||||
import { awaitAll } from "../await-all";
|
||||
|
||||
/**
|
||||
* Manages exclusive locks on items to prevent concurrent modifications.
|
||||
|
|
@ -8,47 +7,50 @@ import { awaitAll } from "../await-all";
|
|||
*
|
||||
* @template T The type of the key used for locking
|
||||
*/
|
||||
/** Waiter entry with callbacks */
|
||||
interface WaiterEntry<T> {
|
||||
resolve: () => unknown;
|
||||
reject: (err: unknown) => unknown;
|
||||
}
|
||||
|
||||
export class Locks<T> {
|
||||
/** Currently locked keys */
|
||||
private readonly locked = new Set<T>();
|
||||
|
||||
/** Queue of resolve functions waiting for each key */
|
||||
private readonly waiters = new Map<
|
||||
T,
|
||||
[() => unknown, (err: unknown) => unknown][]
|
||||
>();
|
||||
/** Queue of waiters for each key */
|
||||
private readonly waiters = new Map<T, WaiterEntry<T>[]>();
|
||||
|
||||
public constructor(private readonly logger?: Logger) {}
|
||||
public constructor(private readonly name: string, private readonly logger?: Logger) { }
|
||||
|
||||
/**
|
||||
* Executes a function while holding exclusive locks on one or more keys.
|
||||
*
|
||||
* This method ensures that the provided function runs with exclusive access to the
|
||||
* specified key(s). Multiple keys are sorted to prevent deadlocks when different
|
||||
* operations request the same keys in different orders.
|
||||
*
|
||||
* @template R The return type of the function to execute
|
||||
* @param keyOrKeys A single key or array of keys to lock during function execution
|
||||
* @param fn The function to execute while holding the lock(s). Can be sync or async.
|
||||
* @returns A Promise that resolves to the return value of the executed function
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* // Lock a single key
|
||||
* const result = await locks.withLock('file1', () => {
|
||||
* // Critical section - only one operation can access 'file1' at a time
|
||||
* return processFile('file1');
|
||||
* });
|
||||
*
|
||||
* // Lock multiple keys (prevents deadlocks through consistent ordering)
|
||||
* await locks.withLock(['file1', 'file2'], async () => {
|
||||
* // Critical section - exclusive access to both files
|
||||
* await moveFile('file1', 'file2');
|
||||
* });
|
||||
* ```
|
||||
*
|
||||
* @throws Any error thrown by the provided function will be propagated after locks are released
|
||||
*/
|
||||
* Executes a function while holding exclusive locks on one or more keys.
|
||||
*
|
||||
* This method ensures that the provided function runs with exclusive access to the
|
||||
* specified key(s). Multiple keys are sorted to prevent deadlocks when different
|
||||
* operations request the same keys in different orders.
|
||||
*
|
||||
* @template R The return type of the function to execute
|
||||
* @param keyOrKeys A single key or array of keys to lock during function execution
|
||||
* @param fn The function to execute while holding the lock(s). Can be sync or async.
|
||||
* @returns A Promise that resolves to the return value of the executed function
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* // Lock a single key
|
||||
* const result = await locks.withLock('file1', () => {
|
||||
* // Critical section - only one operation can access 'file1' at a time
|
||||
* return processFile('file1');
|
||||
* });
|
||||
*
|
||||
* // Lock multiple keys (prevents deadlocks through consistent ordering)
|
||||
* await locks.withLock(['file1', 'file2'], async () => {
|
||||
* // Critical section - exclusive access to both files
|
||||
* await moveFile('file1', 'file2');
|
||||
* });
|
||||
* ```
|
||||
*
|
||||
* @throws Any error thrown by the provided function will be propagated after locks are released
|
||||
*/
|
||||
public async withLock<R>(
|
||||
keyOrKeys: T | T[],
|
||||
fn: () => R | Promise<R>
|
||||
|
|
@ -59,12 +61,17 @@ export class Locks<T> {
|
|||
const uniqueKeys = Array.from(new Set(keys));
|
||||
uniqueKeys.sort((a, b) => String(a).localeCompare(String(b))); // Ensure consistent order to prevent deadlocks
|
||||
|
||||
await awaitAll(uniqueKeys.map(async (key) => this.waitForLock(key)));
|
||||
|
||||
const lockedKeys = [];
|
||||
try {
|
||||
for (const key of uniqueKeys) {
|
||||
// Must acquire locks in-order (not concurrently) to prevent deadlocks
|
||||
await this.waitForLock(key);
|
||||
lockedKeys.push(key);
|
||||
}
|
||||
|
||||
return await fn();
|
||||
} finally {
|
||||
uniqueKeys.forEach((key) => {
|
||||
lockedKeys.forEach((key) => {
|
||||
this.unlock(key);
|
||||
});
|
||||
}
|
||||
|
|
@ -74,7 +81,7 @@ export class Locks<T> {
|
|||
// Resolve all waiting promises before clearing to prevent deadlock
|
||||
// Any operation waiting for a lock will be granted access immediately
|
||||
for (const waiting of this.waiters.values()) {
|
||||
for (const [_, reject] of waiting) {
|
||||
for (const { reject } of waiting) {
|
||||
reject(new SyncResetError());
|
||||
}
|
||||
}
|
||||
|
|
@ -82,13 +89,17 @@ export class Locks<T> {
|
|||
this.waiters.clear();
|
||||
}
|
||||
|
||||
public isLocked(key: T): boolean {
|
||||
return this.locked.has(key);
|
||||
}
|
||||
|
||||
/**
|
||||
* Attempts to acquire a lock immediately without waiting.
|
||||
* Must call `unlock()` if successful.
|
||||
*
|
||||
* @param key The key to lock
|
||||
* @returns `true` if lock acquired, `false` if already locked
|
||||
*/
|
||||
* Attempts to acquire a lock immediately without waiting.
|
||||
* Must call `unlock()` if successful.
|
||||
*
|
||||
* @param key The key to lock
|
||||
* @returns `true` if lock acquired, `false` if already locked
|
||||
*/
|
||||
public tryLock(key: T): boolean {
|
||||
if (this.locked.has(key)) {
|
||||
return false;
|
||||
|
|
@ -100,18 +111,18 @@ export class Locks<T> {
|
|||
}
|
||||
|
||||
/**
|
||||
* Waits to acquire a lock, blocking until available.
|
||||
* Operations are queued in FIFO order. Must call `unlock()` when done.
|
||||
*
|
||||
* @param key The key to wait for and lock
|
||||
* @returns Promise that resolves when lock is acquired
|
||||
*/
|
||||
* Waits to acquire a lock, blocking until available.
|
||||
* Operations are queued in FIFO order. Must call `unlock()` when done.
|
||||
*
|
||||
* @param key The key to wait for and lock
|
||||
* @returns Promise that resolves when lock is acquired
|
||||
*/
|
||||
public async waitForLock(key: T): Promise<void> {
|
||||
if (this.tryLock(key)) {
|
||||
return Promise.resolve();
|
||||
}
|
||||
|
||||
this.logger?.debug(`Waiting for lock on ${key}`);
|
||||
this.logger?.debug(`Waiting for lock '${this.name}' on '${key}'`);
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
// DefaultDict behavior
|
||||
|
|
@ -121,28 +132,36 @@ export class Locks<T> {
|
|||
this.waiters.set(key, waiting);
|
||||
}
|
||||
|
||||
waiting.push([resolve, reject]);
|
||||
waiting.push({
|
||||
resolve,
|
||||
reject,
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Releases a lock and grants access to the next waiting operation in FIFO order.
|
||||
* Removes the key from locked set if no waiters.
|
||||
*
|
||||
* @param key The key to unlock
|
||||
* @throws {Error} If key is not currently locked
|
||||
*/
|
||||
* Releases a lock and grants access to the next waiting operation in FIFO order.
|
||||
* Removes the key from locked set if no waiters.
|
||||
*
|
||||
* @param key The key to unlock
|
||||
* @throws {Error} If key is not currently locked
|
||||
*/
|
||||
public unlock(key: T): void {
|
||||
if (!this.locked.has(key)) {
|
||||
this.logger?.debug(
|
||||
`Attempted to unlock '${this.name}' on '${key}' which is not locked`
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Remove first waiter to ensure FIFO order
|
||||
const [resolveNextWaiting, _] = this.waiters.get(key)?.shift() ?? [];
|
||||
this.logger?.debug(`Releasing lock '${this.name}' on '${key}'`);
|
||||
|
||||
if (resolveNextWaiting) {
|
||||
this.logger?.debug(`Granted lock on ${key}`);
|
||||
resolveNextWaiting();
|
||||
// Remove first waiter to ensure FIFO order
|
||||
const nextWaiter = this.waiters.get(key)?.shift();
|
||||
|
||||
if (nextWaiter) {
|
||||
this.logger?.debug(`Granted lock '${this.name}' on '${key}'`);
|
||||
nextWaiter.resolve();
|
||||
} else {
|
||||
this.locked.delete(key);
|
||||
}
|
||||
|
|
@ -152,8 +171,8 @@ export class Locks<T> {
|
|||
export class Lock {
|
||||
private readonly locks: Locks<boolean>;
|
||||
|
||||
public constructor(logger?: Logger) {
|
||||
this.locks = new Locks(logger);
|
||||
public constructor(name: string, logger?: Logger) {
|
||||
this.locks = new Locks(name, logger);
|
||||
}
|
||||
|
||||
public async withLock<R>(fn: () => R | Promise<R>): Promise<R> {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue