From a662fe4d2cdd794bc698290de0c9a1bb674d39d1 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Tue, 19 May 2026 17:08:36 +0200 Subject: [PATCH] fix: clean up promoted repository lock on queued abort --- .../core/__tests__/repository-mutex.test.ts | 39 ++++++++++++++++++- app/server/core/repository-mutex.ts | 3 +- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/app/server/core/__tests__/repository-mutex.test.ts b/app/server/core/__tests__/repository-mutex.test.ts index 1d1fd679..6eba0172 100644 --- a/app/server/core/__tests__/repository-mutex.test.ts +++ b/app/server/core/__tests__/repository-mutex.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import { eq } from "drizzle-orm"; import { db } from "~/server/db/db"; import { repositoryLocksTable, repositoryLockWaitersTable } from "~/server/db/schema"; @@ -338,6 +338,43 @@ describe("RepositoryMutex", () => { expect(repoMutex.isLocked(repoB)).toBe(false); }); + test("should not leave a promoted shared lock behind if that waiter aborts before observing acquisition", async () => { + vi.useFakeTimers(); + const repoId = "shared-abort-after-promotion"; + const releaseExclusive = await repoMutex.acquireExclusive(repoId, "holder"); + const firstSharedPromise = repoMutex.acquireShared(repoId, "shared-1"); + + try { + await vi.advanceTimersByTimeAsync(100); + + const controller = new AbortController(); + const secondSharedPromise = repoMutex.acquireShared(repoId, "shared-2", controller.signal); + + await vi.advanceTimersByTimeAsync(140); + releaseExclusive(); + + // The first shared waiter wakes first and promotes both shared waiters. + await vi.advanceTimersByTimeAsync(10); + const releaseShared1 = await firstSharedPromise; + + controller.abort(new Error("abort after promotion")); + await expect(secondSharedPromise).rejects.toThrow("abort after promotion"); + + releaseShared1(); + + const remainingLocks = await db.query.repositoryLocksTable.findMany({ + where: { repositoryId: { eq: repoId } }, + orderBy: { operation: "asc" }, + }); + + expect(remainingLocks).toEqual([]); + } finally { + await db.delete(repositoryLockWaitersTable).where(eq(repositoryLockWaitersTable.repositoryId, repoId)); + await db.delete(repositoryLocksTable).where(eq(repositoryLocksTable.repositoryId, repoId)); + vi.useRealTimers(); + } + }); + test("should safely handle multiple calls to the release function", async () => { const repoId = "idempotent-release"; diff --git a/app/server/core/repository-mutex.ts b/app/server/core/repository-mutex.ts index 1d434260..61b30944 100644 --- a/app/server/core/repository-mutex.ts +++ b/app/server/core/repository-mutex.ts @@ -314,6 +314,7 @@ class RepositoryMutex { } catch (error) { this.stopHeartbeat(waiterId); this.deleteWaiter(waiterId); + this.release({ id: waiterId }); throw error; } } @@ -412,7 +413,7 @@ class RepositoryMutex { }; } - private release(lock: AcquiredLock) { + private release(lock: Pick) { const releasedLock = db.transaction((tx) => { const row = tx.query.repositoryLocksTable .findFirst({ where: { AND: [{ id: { eq: lock.id } }, { ownerId: { eq: this.ownerId } }] } })