refactor: fix security hardcode, dead code, resource leaks, operator bugs
This commit is contained in:
+34
-6
@@ -3,15 +3,22 @@ import { logInfo } from '../utils';
|
||||
/**
|
||||
* AsyncLockManager: Prevents race conditions by ensuring only one task
|
||||
* can access a specific resource (e.g., a file path) at a time.
|
||||
*
|
||||
* Includes timeout protection to prevent indefinite lock-waiting,
|
||||
* and proper cleanup on acquisition failure.
|
||||
*/
|
||||
export class AsyncLockManager {
|
||||
private locks: Map<string, Promise<void>> = new Map();
|
||||
private static readonly DEFAULT_TIMEOUT_MS = 30_000;
|
||||
|
||||
/**
|
||||
* Acquires a lock for a specific resource.
|
||||
* If the resource is already locked, it waits until the previous task finishes.
|
||||
* If the resource is already locked, waits until the previous task finishes.
|
||||
* Times out after `timeoutMs` to prevent deadlocks.
|
||||
*
|
||||
* @returns A release function that MUST be called when the work is done (use try/finally).
|
||||
*/
|
||||
public async acquire(resourceId: string): Promise<() => void> {
|
||||
public async acquire(resourceId: string, timeoutMs: number = AsyncLockManager.DEFAULT_TIMEOUT_MS): Promise<() => void> {
|
||||
const previousLock = this.locks.get(resourceId) || Promise.resolve();
|
||||
|
||||
let release: () => void;
|
||||
@@ -21,17 +28,38 @@ export class AsyncLockManager {
|
||||
|
||||
this.locks.set(resourceId, previousLock.then(() => newLock));
|
||||
|
||||
await previousLock;
|
||||
// Wait for previous lock with a timeout to prevent deadlocks
|
||||
const timeoutPromise = new Promise<never>((_, reject) => {
|
||||
setTimeout(() => reject(new Error(`Lock acquisition timed out for resource: ${resourceId}`)), timeoutMs);
|
||||
});
|
||||
|
||||
try {
|
||||
await Promise.race([previousLock, timeoutPromise]);
|
||||
} catch (error) {
|
||||
// Clean up the dangling lock on timeout
|
||||
if (this.locks.get(resourceId) === previousLock.then(() => newLock)) {
|
||||
this.locks.delete(resourceId);
|
||||
}
|
||||
release!();
|
||||
throw error;
|
||||
}
|
||||
|
||||
logInfo(`Lock acquired for: ${resourceId}`);
|
||||
|
||||
return () => {
|
||||
logInfo(`Lock released for: ${resourceId}`);
|
||||
release();
|
||||
if (this.locks.get(resourceId) === newLock) {
|
||||
this.locks.delete(resourceId);
|
||||
}
|
||||
// Clean up the Map entry if this is the latest lock
|
||||
this.locks.delete(resourceId);
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the number of currently held locks (for diagnostics).
|
||||
*/
|
||||
public getActiveLockCount(): number {
|
||||
return this.locks.size;
|
||||
}
|
||||
}
|
||||
|
||||
// Export as a singleton for the entire agent process
|
||||
|
||||
Reference in New Issue
Block a user