fix(lock): implement Redlock single-instance pattern in LockManagerService

Libs/Utils/ICacheService.php

@@ -96,6 +96,16 @@ public function setSingleValue($key, $value, $ttl = 0);
 	*/
 	public function addSingleValue($key, $value, $ttl = 0);
 
+	/**
+	 * Atomically compare-and-delete: DEL the key only when its current value
+	 * equals $expectedValue. Implementations MUST use an atomic operation
+	 * (Lua EVAL or equivalent) — never a separate GET + conditional DEL.
+	 * @param string $key
+	 * @param string $expectedValue
+	 * @return bool  true iff the key existed, matched, and was deleted
+	 */
+	public function deleteIfValueMatches(string $key, string $expectedValue): bool;
+
 	/**
 	* Set time to live to a given key
 	* @param $key

app/Services/Model/Imp/SummitOrderService.php

@@ -827,7 +827,7 @@ public function run(array $formerState): array
 
                 $this->lock_service->lock('promocode.' . $promo_code->getId() . '.usage.lock', function () use ($promo_code, $qty, $owner_email) {
                     $promo_code->addUsage($owner_email, $qty);
-                });
+                }, 30);
 
             });
             // mark a done
@@ -865,7 +865,7 @@ public function undo()
 
                 $this->lock_service->lock('promocode.' . $promo_code->getId() . '.usage.lock', function () use ($promo_code, $info, $owner_email) {
                     $promo_code->removeUsage(intval($info['qty']), $owner_email);
-                });
+                }, 30);
 
             });
         }
@@ -950,7 +950,7 @@ public function run(array $formerState): array
 
                 $this->lock_service->lock('ticket_type.' . $ticket_type->getId() . '.sell.lock', function () use ($ticket_type, $reservations) {
                     $ticket_type->sell($reservations[$ticket_type->getId()]);
-                });
+                }, 30);
 
             }
         });
@@ -967,7 +967,7 @@ public function undo()
                 if (is_null($ticket_type)) return;
                 $this->lock_service->lock('ticket_type.' . $ticket_type->getId() . '.sell.lock', function () use ($ticket_type, $qty) {
                     $ticket_type->restore($qty);
-                });
+                }, 30);
             });
         }
     }
@@ -1536,7 +1536,7 @@ public function run(array $formerState): array
             if (empty($promo_code_val)) throw new ValidationException("Promo code is required.");
 
             $type_id = $ticket_dto['type_id'];
-            $order = $this->lock_service->lock('ticket_type.' . $type_id . 'promo_code.' . $promo_code_val . '.sell.lock',
+            $order = $this->lock_service->lock('ticket_type.' . $type_id . '.promo_code.' . $promo_code_val . '.sell.lock',
                 function () use ($promo_code_val, $type_id) {
 
                     $attendee_email = $this->owner->getEmail();
@@ -1658,7 +1658,7 @@ function () use ($promo_code_val, $type_id) {
 
 
                     return $order;
-                });
+                }, 30);
             return ['order' => $order];
         });
     }

app/Services/Utils/ILockManagerService.php

@@ -24,20 +24,21 @@ interface ILockManagerService
      * @param string $name
      * @param int $lifetime
      * @throws UnacquiredLockException
-     * @return mixed
+     * @return string ownership token — must be passed to releaseLock
      */
-    public function acquireLock(string $name,int $lifetime = self::DefaultLifetime);
+    public function acquireLock(string $name, int $lifetime = self::DefaultLifetime): string;
+
     /**
-     * @param  string $name
-     * @return mixed
+     * @param string $name
+     * @param string $token ownership token returned by acquireLock
      */
-    public function releaseLock(string $name);
+    public function releaseLock(string $name, string $token): void;
 
     /**
      * @param string $name
      * @param Closure $callback
      * @param int $lifetime
      * @return mixed
      */
-    public function lock(string $name, Closure $callback, int $lifetime = self::DefaultLifetime);
+    public function lock(string $name, Closure $callback, int $lifetime = self::DefaultLifetime): mixed;
 }

app/Services/Utils/LockManagerService.php

@@ -22,9 +22,10 @@
  */
 final class LockManagerService implements ILockManagerService {
 
-    const MaxRetries = 3;
+    const MaxRetries        = 3;
     const BackOffMultiplier = 2.0;
-    const BackOffBaseInterval = 100000; // 1 ms
+    const BackOffBaseInterval = 100000; // microseconds
+
     /**
      * @var ICacheService
      */
@@ -41,73 +42,76 @@ public function __construct(ICacheService $cache_service){
     /**
      * @param string $name
      * @param int $lifetime
-     * @return LockManagerService
+     * @return string ownership token — pass to releaseLock
      * @throws UnacquiredLockException
      */
-    public function acquireLock(string $name, int $lifetime = 3600):LockManagerService
+    public function acquireLock(string $name, int $lifetime = 3600): string
     {
-        Log::debug(sprintf("LockManagerService::acquireLock name %s lifetime %s",$name, $lifetime));
-        $attempt  = 0 ;
+        Log::debug(sprintf("LockManagerService::acquireLock name %s lifetime %s", $name, $lifetime));
+        if ($lifetime <= 0) {
+            throw new \InvalidArgumentException("Lock lifetime must be greater than zero seconds.");
+        }
+        $token   = bin2hex(random_bytes(16));
+        $attempt = 0;
         do {
-            $time = time() + $lifetime + 1;
-            $success = $this->cache_service->addSingleValue($name, $time, $time);
-            if($success) return $this;
-            $wait_interval = self::BackOffBaseInterval * ( self::BackOffMultiplier ^ $attempt );
-            Log::debug(sprintf("LockManagerService::acquireLock name %s retrying in %s microseconds (%s).", $name, $wait_interval, $attempt));
+            $success = $this->cache_service->addSingleValue($name, $token, $lifetime);
+            if ($success) {
+                return $token;
+            }
+            $wait_interval = (int)(self::BackOffBaseInterval * (self::BackOffMultiplier ** $attempt));
+            Log::debug(sprintf("LockManagerService::acquireLock name %s retrying in %s µs (attempt %s)", $name, $wait_interval, $attempt));
             usleep($wait_interval);
-            if($attempt >= (self::MaxRetries - 1 )) {
-                // only one time we could use this handle
+            if ($attempt >= (self::MaxRetries - 1)) {
                 Log::error(sprintf("LockManagerService::acquireLock name %s lifetime %s ERROR MAX RETRIES attempt %s", $name, $lifetime, $attempt));
                 throw new UnacquiredLockException(sprintf("lock name %s", $name));
             }
             ++$attempt;
-        } while(1);
+        } while (1);
     }
 
     /**
      * @param string $name
-     * @return $this
+     * @param string $token ownership token returned by acquireLock
      */
-    public function releaseLock(string $name):LockManagerService
+    public function releaseLock(string $name, string $token): void
     {
-        Log::debug(sprintf("LockManagerService::releaseLock name %s",$name));
-        $this->cache_service->delete($name);
-        return $this;
+        Log::debug(sprintf("LockManagerService::releaseLock name %s", $name));
+        $this->cache_service->deleteIfValueMatches($name, $token);
     }
 
     /**
      * @param string $name
      * @param Closure $callback
      * @param int $lifetime
-     * @return null
+     * @return mixed
      * @throws UnacquiredLockException
      * @throws Exception
      */
-    public function lock(string $name, Closure $callback, int $lifetime = 3600)
+    public function lock(string $name, Closure $callback, int $lifetime = 3600): mixed
     {
+        $token  = null;
         $result = null;
         Log::debug(sprintf("LockManagerService::lock name %s lifetime %s", $name, $lifetime));
 
-        try
-        {
-            $this->acquireLock($name, $lifetime);
+        try {
+            $token  = $this->acquireLock($name, $lifetime);
             Log::debug(sprintf("LockManagerService::lock name %s calling callback", $name));
             $result = $callback($this);
         }
-        catch(UnacquiredLockException $ex)
-        {
+        catch(UnacquiredLockException $ex) {
             Log::warning($ex);
             throw $ex;
         }
-        catch(Exception $ex)
-        {
+        catch(Exception $ex) {
             Log::error($ex);
             throw $ex;
         }
         finally {
-            $this->releaseLock($name);
+            if ($token !== null) {
+                $this->releaseLock($name, $token);
+            }
         }
         return $result;
     }
 
-}
+}

app/Services/Utils/RedisCacheService.php

@@ -239,8 +239,7 @@ public function storeHash($name, array $values, $ttl = 0)
     public function incCounter($counter_name, $ttl = 0)
     {
         return $this->retryOnConnectionError(function ($conn) use ($counter_name, $ttl) {
-            if ($conn->setnx($counter_name, 1)) {
-                if ($ttl > 0) $conn->expire($counter_name, (int)$ttl);
+            if ($conn->set($counter_name, 1, ['EX' => (int)$ttl, 'NX' => true]) !== null) {
                 return 1;
             }
             return (int)$conn->incr($counter_name);
@@ -306,12 +305,11 @@ public function setSingleValue($key, $value, $ttl = 0)
     public function addSingleValue($key, $value, $ttl = 0)
     {
         return $this->retryOnConnectionError(function ($conn) use ($key, $value, $ttl) {
-            $res = $conn->setnx($key, $value);
-            if ($res && $ttl > 0) {
-                $conn->expire($key, $ttl);
+            if ($ttl > 0) {
+                return $conn->set($key, $value, 'EX', (int)$ttl, 'NX') !== null;
             }
-            return $res;
-        });
+            return $conn->set($key, $value, 'NX') !== null;
+        }, false);
     }
 
     public function setKeyExpiration($key, $ttl)
@@ -331,7 +329,21 @@ public function ttl($key)
             return (int)$conn->ttl($key);
         }, 0);
     }
-
+
+    public function deleteIfValueMatches(string $key, string $expectedValue): bool
+    {
+        $lua = <<<'LUA'
+if redis.call('get', KEYS[1]) == ARGV[1] then
+    return redis.call('del', KEYS[1])
+else
+    return 0
+end
+LUA;
+        return $this->retryOnConnectionError(function ($conn) use ($lua, $key, $expectedValue) {
+            return (int)$conn->eval($lua, 1, $key, $expectedValue) === 1;
+        }, false);
+    }
+
     /**
      * @param string $cache_region_key
      * @return void

tests/Integration/RedisCacheServiceAddSingleValueTest.php

@@ -0,0 +1,99 @@
+<?php namespace Tests\Integration;
+/**
+ * Copyright 2026 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+use Illuminate\Support\Facades\Redis;
+use PHPUnit\Framework\Attributes\Group;
+use services\utils\RedisCacheService;
+use Tests\CreatesApplication;
+use Tests\TestCase;
+
+/**
+ * Integration tests for RedisCacheService::addSingleValue.
+ *
+ * These tests require a live Redis instance and verify two properties that
+ * mocks cannot exercise:
+ *
+ *  1. Driver compatibility — the variadic SET NX EX form works with the
+ *     configured Predis/PhpRedis driver.  If the driver is switched to
+ *     PhpRedis, set() returns false on an NX-miss (not null), which would
+ *     silently break the `!== null` check; this test catches that regression.
+ *
+ *  2. Atomicity — key and TTL are written in a single command; there is no
+ *     window where the key exists without a TTL.  Verified by reading TTL
+ *     immediately after addSingleValue returns.
+ *
+ */
+#[Group("integration")]
+final class RedisCacheServiceAddSingleValueTest extends TestCase
+{
+    use CreatesApplication;
+
+    private const TEST_KEY = 'test:add_single_value:lock';
+    private const TTL      = 30;
+
+    private RedisCacheService $service;
+    private mixed             $redis;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->redis   = Redis::connection();
+        $this->service = new RedisCacheService();
+        // Start clean regardless of any leftover from a previous failed run.
+        $this->redis->del(self::TEST_KEY);
+    }
+
+    protected function tearDown(): void
+    {
+        $this->redis->del(self::TEST_KEY);
+        parent::tearDown();
+    }
+
+    /**
+     * First call must succeed and leave a TTL on the key.
+     * Second call on the same key must return false (NX semantics).
+     */
+    public function testAddSingleValueSetsKeyWithTtlAndNxSemanticsHold(): void
+    {
+        $token = bin2hex(random_bytes(16));
+
+        $acquired = $this->service->addSingleValue(self::TEST_KEY, $token, self::TTL);
+        $this->assertTrue($acquired, 'first addSingleValue must return true');
+
+        // Atomicity: TTL must already be set — no gap between key write and expire.
+        $ttl = (int)$this->redis->ttl(self::TEST_KEY);
+        $this->assertGreaterThanOrEqual(1, $ttl, 'key must have a positive TTL immediately after addSingleValue');
+        $this->assertLessThanOrEqual(self::TTL, $ttl, 'TTL must not exceed the requested lifetime');
+
+        // NX semantics: a second call while the key still exists must fail.
+        $again = $this->service->addSingleValue(self::TEST_KEY, bin2hex(random_bytes(16)), self::TTL);
+        $this->assertFalse($again, 'addSingleValue must return false when key already exists (NX)');
+    }
+
+    /**
+     * After the key is deleted the lock can be re-acquired, confirming the
+     * return-value contract holds across both the true and false branches.
+     */
+    public function testAddSingleValueReturnsTrueAfterKeyIsDeleted(): void
+    {
+        $token = bin2hex(random_bytes(16));
+
+        $this->assertTrue($this->service->addSingleValue(self::TEST_KEY, $token, self::TTL));
+        $this->redis->del(self::TEST_KEY);
+        $this->assertTrue(
+            $this->service->addSingleValue(self::TEST_KEY, bin2hex(random_bytes(16)), self::TTL),
+            'addSingleValue must return true once the key has been removed'
+        );
+    }
+}