diff --git a/app/client/modules/repositories/tabs/info.tsx b/app/client/modules/repositories/tabs/info.tsx index a2aeee9a..56973628 100644 --- a/app/client/modules/repositories/tabs/info.tsx +++ b/app/client/modules/repositories/tabs/info.tsx @@ -302,6 +302,7 @@ export const RepositoryInfoTabContent = ({ repository }: Props) => { Cancel diff --git a/app/server/core/__tests__/repository-mutex.test.ts b/app/server/core/__tests__/repository-mutex.test.ts index ade8307f..eee37d40 100644 --- a/app/server/core/__tests__/repository-mutex.test.ts +++ b/app/server/core/__tests__/repository-mutex.test.ts @@ -94,4 +94,32 @@ describe("RepositoryMutex", () => { expect(releaseEx3).toBeDefined(); releaseEx3(); }); + + test("should remove signal listener and not release unacquired lock on abort", async () => { + const repoId = "cleanup-test"; + const controller = new AbortController(); + const signal = controller.signal; + + let removed = false; + const originalRemove = signal.removeEventListener.bind(signal); + signal.removeEventListener = (type: string, listener: any, options?: any) => { + if (type === "abort") removed = true; + return originalRemove(type, listener, options); + }; + + // Hold the lock so the next one has to wait + const release = await repoMutex.acquireExclusive(repoId, "holder"); + + const abortedAcquisition = repoMutex.acquireShared(repoId, "waiter", signal); + + // Trigger abort while it's waiting + controller.abort(); + + expect(abortedAcquisition).rejects.toThrow(); + expect(removed).toBe(true); + expect(repoMutex.isLocked(repoId)).toBe(true); + + release(); + expect(repoMutex.isLocked(repoId)).toBe(false); + }); }); diff --git a/app/server/core/repository-mutex.ts b/app/server/core/repository-mutex.ts index 663bd1d3..ccf62733 100644 --- a/app/server/core/repository-mutex.ts +++ b/app/server/core/repository-mutex.ts @@ -70,31 +70,36 @@ class RepositoryMutex { ); let onAbort: () => void = () => {}; - const lockId = await new Promise((resolve, reject) => { - const waiter = { type: "shared" as const, operation, resolve }; - state.waitQueue.push(waiter); + let lockId: string | undefined; + try { + lockId = await new Promise((resolve, reject) => { + const waiter = { type: "shared" as const, operation, resolve }; + state.waitQueue.push(waiter); - if (signal) { - onAbort = () => { - const index = state.waitQueue.indexOf(waiter); - if (index !== -1) { - state.waitQueue.splice(index, 1); - this.cleanupStateIfEmpty(repositoryId); - reject(signal.reason || new Error("Operation aborted")); - } - }; - signal.addEventListener("abort", onAbort); - } - }); - - signal?.removeEventListener("abort", onAbort); + if (signal) { + onAbort = () => { + const index = state.waitQueue.indexOf(waiter); + if (index !== -1) { + state.waitQueue.splice(index, 1); + this.cleanupStateIfEmpty(repositoryId); + reject(signal.reason || new Error("Operation aborted")); + } + }; + signal.addEventListener("abort", onAbort); + } + }); + } finally { + signal?.removeEventListener("abort", onAbort); + } if (signal?.aborted) { - this.releaseShared(repositoryId, lockId); + if (lockId) { + this.releaseShared(repositoryId, lockId); + } throw signal.reason || new Error("Operation aborted"); } - return () => this.releaseShared(repositoryId, lockId); + return () => this.releaseShared(repositoryId, lockId!); } async acquireExclusive(repositoryId: string, operation: string, signal?: AbortSignal): Promise<() => void> { @@ -119,32 +124,37 @@ class RepositoryMutex { ); let onAbort: () => void = () => {}; - const lockId = await new Promise((resolve, reject) => { - const waiter = { type: "exclusive" as const, operation, resolve }; - state.waitQueue.push(waiter); + let lockId: string | undefined; + try { + lockId = await new Promise((resolve, reject) => { + const waiter = { type: "exclusive" as const, operation, resolve }; + state.waitQueue.push(waiter); - if (signal) { - onAbort = () => { - const index = state.waitQueue.indexOf(waiter); - if (index !== -1) { - state.waitQueue.splice(index, 1); - this.cleanupStateIfEmpty(repositoryId); - reject(signal.reason || new Error("Operation aborted")); - } - }; - signal.addEventListener("abort", onAbort); - } - }); - - signal?.removeEventListener("abort", onAbort); + if (signal) { + onAbort = () => { + const index = state.waitQueue.indexOf(waiter); + if (index !== -1) { + state.waitQueue.splice(index, 1); + this.cleanupStateIfEmpty(repositoryId); + reject(signal.reason || new Error("Operation aborted")); + } + }; + signal.addEventListener("abort", onAbort); + } + }); + } finally { + signal?.removeEventListener("abort", onAbort); + } if (signal?.aborted) { - this.releaseExclusive(repositoryId, lockId); + if (lockId) { + this.releaseExclusive(repositoryId, lockId); + } throw signal.reason || new Error("Operation aborted"); } logger.debug(`[Mutex] Acquired exclusive lock for repo ${repositoryId}: ${operation} (${lockId})`); - return () => this.releaseExclusive(repositoryId, lockId); + return () => this.releaseExclusive(repositoryId, lockId!); } private releaseShared(repositoryId: string, lockId: string): void {