Fix release on expired locks

Author: phil-r

src/index.ts

@@ -313,12 +313,7 @@ export default class Redlock extends EventEmitter {
         settings
       );
 
-      // Add 2 milliseconds to the drift to account for Redis expires precision,
-      // which is 1 ms, plus the configured allowable drift factor.
-      const drift =
-        Math.round(
-          (settings?.driftFactor ?? this.settings.driftFactor) * duration
-        ) + 2;
+      const drift = this.calculateDrift(duration, settings);
 
       return new Lock(
         this,
@@ -351,6 +346,12 @@ export default class Redlock extends EventEmitter {
     lock: Lock,
     settings?: Partial<Settings>
   ): Promise<ExecutionResult> {
+    // Check if lock already expired and quit early
+    if (lock.expiration < Date.now()) {
+      const attempts: Promise<ExecutionStats>[] = [];
+      return Promise.resolve({ attempts });
+    }
+
     // Immediately invalidate the lock.
     lock.expiration = 0;
 
@@ -363,6 +364,22 @@ export default class Redlock extends EventEmitter {
     );
   }
 
+  /**
+   * This method calculates drift for the provided `duration`.
+   */
+  public calculateDrift(
+    duration: number,
+    settings?: Partial<Settings>
+  ): number {
+    // Add 2 milliseconds to the drift to account for Redis expires precision,
+    // which is 1 ms, plus the configured allowable drift factor.
+    return (
+      Math.round(
+        (settings?.driftFactor ?? this.settings.driftFactor) * duration
+      ) + 2
+    );
+  }
+
   /**
    * This method extends a valid lock by the provided `duration`.
    */
@@ -392,12 +409,7 @@ export default class Redlock extends EventEmitter {
     // Invalidate the existing lock.
     existing.expiration = 0;
 
-    // Add 2 milliseconds to the drift to account for Redis expires precision,
-    // which is 1 ms, plus the configured allowable drift factor.
-    const drift =
-      Math.round(
-        (settings?.driftFactor ?? this.settings.driftFactor) * duration
-      ) + 2;
+    const drift = this.calculateDrift(duration, settings);
 
     const replacement = new Lock(
       this,

src/multi.test.ts

@@ -3,6 +3,10 @@ import test, { ExecutionContext } from "ava";
 import Redis, { Redis as Client, Cluster } from "ioredis";
 import Redlock, { ExecutionError, ResourceLockedError } from "./index.js";
 
+async function wait(ms: number): Promise<number> {
+  return new Promise((resolve) => setTimeout(() => resolve(ms), ms));
+}
+
 async function fail(
   t: ExecutionContext<unknown>,
   error: unknown
@@ -174,6 +178,28 @@ function run(
     }
   });
 
+  test(`${namespace} - releasing expired lock doesn't fail`, async (t) => {
+    try {
+      const redlock = new Redlock([redisA, redisB, redisC]);
+
+      const duration = 1000;
+
+      // Acquire a lock.
+      const lock = await redlock.acquire(["{redlock}a"], duration);
+
+      // Wait for duration + drift to be sure that lock has expired
+      await wait(duration + redlock.calculateDrift(duration));
+
+      // Release the lock.
+      await lock.release();
+      t.is(await redisA.get("{redlock}a"), null, "The lock was not released");
+      t.is(await redisB.get("{redlock}a"), null, "The lock was not released");
+      t.is(await redisC.get("{redlock}a"), null, "The lock was not released");
+    } catch (error) {
+      fail(t, error);
+    }
+  });
+
   test(`${namespace} - succeeds when a minority of clients fail`, async (t) => {
     try {
       const redlock = new Redlock([redisA, redisB, redisC]);

src/single.test.ts

@@ -3,6 +3,10 @@ import test, { ExecutionContext } from "ava";
 import Redis, { Redis as Client, Cluster } from "ioredis";
 import Redlock, { ExecutionError, ResourceLockedError } from "./index.js";
 
+async function wait(ms: number): Promise<number> {
+  return new Promise((resolve) => setTimeout(() => resolve(ms), ms));
+}
+
 async function fail(
   t: ExecutionContext<unknown>,
   error: unknown
@@ -135,6 +139,26 @@ function run(namespace: string, redis: Client | Cluster): void {
     }
   });
 
+  test(`${namespace} - releasing expired lock doesn't fail`, async (t) => {
+    try {
+      const redlock = new Redlock([redis]);
+
+      const duration = 1000;
+
+      // Acquire a lock.
+      const lock = await redlock.acquire(["{redlock}a"], duration);
+
+      // Wait for duration + drift to be sure that lock has expired
+      await wait(duration + redlock.calculateDrift(duration));
+
+      // Release the lock.
+      await lock.release();
+      t.is(await redis.get("{redlock}a"), null, "The lock was not released");
+    } catch (error) {
+      fail(t, error);
+    }
+  });
+
   test(`${namespace} - acquires, extends, and releases a multi-resource lock`, async (t) => {
     try {
       const redlock = new Redlock([redis]);
@@ -199,6 +223,30 @@ function run(namespace: string, redis: Client | Cluster): void {
     }
   });
 
+  test(`${namespace} - releasing expired multi-resource lock doesn't fail`, async (t) => {
+    try {
+      const redlock = new Redlock([redis]);
+
+      const duration = 1000;
+
+      // Acquire a lock.
+      const lock = await redlock.acquire(
+        ["{redlock}a1", "{redlock}a2"],
+        duration
+      );
+
+      // Wait for duration + drift to be sure that lock has expired
+      await wait(duration + redlock.calculateDrift(duration));
+
+      // Release the lock.
+      await lock.release();
+      t.is(await redis.get("{redlock}a1"), null, "The lock was not released");
+      t.is(await redis.get("{redlock}a2"), null, "The lock was not released");
+    } catch (error) {
+      fail(t, error);
+    }
+  });
+
   test(`${namespace} - locks fail when redis is unreachable`, async (t) => {
     try {
       const redis = new Redis({