Add Predis timeout

src/Lock/BasicLockInformationProvider.php

@@ -22,8 +22,8 @@ public function getLockInformation()
         $hostname = gethostname();
 
         $params = array();
-        $params[] = $pid;
-        $params[] = $hostname;
+        $params['pid'] = $pid;
+        $params['hostname'] = $hostname;
 
         return $params;
     }

src/Lock/PredisRedisLock.php

@@ -16,15 +16,20 @@
  *
  * @author Kamil Dziedzic <[email protected]>
  */
-class PredisRedisLock extends LockAbstract
+class PredisRedisLock extends LockAbstract implements LockExpirationInterface
 {
     /**
      * Predis connection
      *
-     * @var
+     * @var Predis\Client
      */
     protected $client;
 
+    /**
+     * @var int Expiration time of the lock in seconds
+     */
+    protected $expiration = 0;
+
     /**
      * @param $client Predis\Client
      */
@@ -35,18 +40,71 @@ public function __construct($client)
         $this->client = $client;
     }
 
+    /**
+     * @param int $expiration Expiration time of the lock in seconds
+     */
+    public function setExpiration($expiration)
+    {
+        $this->expiration = $expiration;
+    }
+
     /**
      * @param  string $name
      * @param  bool   $blocking
      * @return bool
      */
     protected function getLock($name, $blocking)
     {
-        if (!$this->client->setnx($name, serialize($this->getLockInformation()))) {
-            return false;
+        /**
+         * Perform the process recommended by Redis for acquiring a lock, from here: https://redis.io/commands/setnx
+         * We are "C4" in this example...
+         *
+         * 1. C4 sends SETNX lock.foo in order to acquire the lock (sets the value if it does not already exist).
+         * 2. The crashed client C3 still holds it, so Redis will reply with 0 to C4.
+         * 3. C4 sends GET lock.foo to check if the lock expired.
+         *    If it is not, it will sleep for some time and retry from the start.
+         * 4. Instead, if the lock is expired because the Unix time at lock.foo is older than the current Unix time,
+         *    C4 tries to perform:
+         *    GETSET lock.foo <current Unix timestamp + lock timeout + 1>
+         *    Because of the GETSET semantic, C4 can check if the old value stored at key is still an expired timestamp
+         *    If it is, the lock was acquired.
+         * 5. If another client, for instance C5, was faster than C4 and acquired the lock with the GETSET operation,
+         *    the C4 GETSET operation will return a non expired timestamp.
+         *    C4 will simply restart from the first step. Note that even if C4 wrote they key and set the expiry time
+         *    a few seconds in the future this is not a problem. C5's timeout will just be a few seconds later.
+         */
+
+        $lockValue = $this->getLockInformation();
+        if ($this->expiration) {
+            // Add expiration timestamp to value stored in Redis.
+            $lockValue['expires'] = time() + $this->expiration;
         }
+        $lockValue = serialize($lockValue);
 
-        return true;
+        if ($this->client->setnx($name, $lockValue)) {
+            return true;
+        }
+
+        // Check if the existing lock has an expiry time. If it does and it has expired, delete the lock.
+        if ($existingValue = $this->client->get($name)) {
+            $existingValue = unserialize($existingValue);
+            if (!empty($existingValue['expires']) && $existingValue['expires'] <= time()) {
+                // The existing lock has expired. We can delete it and take over.
+                $newExistingValue = unserialize($this->client->getset($name, $lockValue));
+
+                // GETSET atomically sets key to value and returns the old value that was stored at key.
+                // If the old value from getset does not still contain an expired timestamp
+                // another probably acquired the lock in the meantime.
+                if ($newExistingValue['expires'] > time()) {
+                    return false;
+                }
+
+                // Got it!
+                return true;
+            }
+        }
+
+        return false;
     }
 
     /**
@@ -57,7 +115,7 @@ protected function getLock($name, $blocking)
      */
     public function releaseLock($name)
     {
-        if (isset($this->locks[$name]) && $this->client->del($name)) {
+        if (isset($this->locks[$name]) && $this->client->del(array($name))) {
             unset($this->locks[$name]);
 
             return true;
@@ -76,4 +134,20 @@ public function isLocked($name)
     {
         return null !== $this->client->get($name);
     }
+
+    /**
+     * Forget a lock without releasing it
+     *
+     * @param  string $name name of lock
+     * @return bool
+     */
+    public function clearLock($name)
+    {
+        if (!isset($this->locks[$name])) {
+            return false;
+        }
+
+        unset($this->locks[$name]);
+        return true;
+    }
 }

src/Lock/ResolvedHostnameLockInformationProvider.php

@@ -21,7 +21,7 @@ class ResolvedHostnameLockInformationProvider extends BasicLockInformationProvid
     public function getLockInformation()
     {
         $params = parent::getLockInformation();
-        $params[] = gethostbyname(gethostname());
+        $params['hostIp'] = gethostbyname(gethostname());
 
         return $params;
     }

tests/Lock/PredisRedisLockTest.php

@@ -0,0 +1,76 @@
+<?php
+
+namespace NinjaMutex\Tests\Lock;
+
+use NinjaMutex\Lock\PredisRedisLock;
+use NinjaMutex\Mutex;
+use NinjaMutex\Tests\Mock\MockPredisClient;
+
+class PredisRedisLockTest extends \NinjaMutex\Tests\AbstractTest
+{
+    protected function createPredisClient()
+    {
+        return new MockPredisClient();
+    }
+
+    protected function createLock($predisClient)
+    {
+        return new PredisRedisLock($predisClient);
+    }
+
+    public function testAcquireLock()
+    {
+        $predis = $this->createPredisClient();
+        $lock = $this->createLock($predis);
+        $mutex = new Mutex('very-critical-stuff', $lock);
+        $this->assertTrue($mutex->acquireLock());
+    }
+
+    public function testAcquireLockFails()
+    {
+        $predis = $this->createPredisClient();
+
+        // Acquire lock in 1st instance - should succeed
+        $lock = $this->createLock($predis);
+        $mutex = new Mutex('very-critical-stuff', $lock);
+        $this->assertTrue($mutex->acquireLock());
+
+        // Acquire lock in 2nd instance - should fail instantly because 0 timeout
+        $lock2 = $this->createLock($predis);
+        $mutex2 = new Mutex('very-critical-stuff', $lock2);
+        $this->assertFalse($mutex2->acquireLock(0));
+    }
+
+    public function testAcquireLockSucceedsAfterReleased()
+    {
+        $predis = $this->createPredisClient();
+
+        // Acquire lock in 1st instance - should succeed
+        $lock = $this->createLock($predis);
+        $mutex = new Mutex('very-critical-stuff', $lock);
+        $this->assertTrue($mutex->acquireLock());
+
+        $this->assertTrue($mutex->releaseLock());
+
+        // Acquire lock in 2nd instance - should succeed because 1st lock had been released
+        $lock2 = $this->createLock($predis);
+        $mutex2 = new Mutex('very-critical-stuff', $lock2);
+        $this->assertTrue($mutex2->acquireLock(0));
+    }
+
+    public function testAcquireLockSucceedsAfterTimeout()
+    {
+        $predis = $this->createPredisClient();
+
+        // Acquire lock in 1st instance - should succeed
+        $lock = $this->createLock($predis);
+        $lock->setExpiration(2);
+        $mutex = new Mutex('very-critical-stuff', $lock);
+        $this->assertTrue($mutex->acquireLock());
+
+        // Acquire lock in 2nd instance - should succeed after 2 seconds
+        $lock2 = $this->createLock($predis);
+        $mutex2 = new Mutex('very-critical-stuff', $lock2);
+        $this->assertTrue($mutex2->acquireLock());
+    }
+}

tests/Mock/MockPredisClient.php

@@ -70,16 +70,18 @@ public function get($key)
     }
 
     /**
-     * @param string $key
+     * @param string[] $keys
      * @return bool
      */
-    public function del($key)
+    public function del(array $keys)
     {
         if (!$this->available) {
             return false;
         }
 
-        unset(self::$data[$key]);
+        foreach ($keys as $key) {
+            unset(self::$data[$key]);
+        }
 
         return true;
     }
@@ -91,4 +93,43 @@ public function setAvailable($available)
     {
         $this->available = (bool)$available;
     }
+
+    /**
+     * @param      $key
+     * @param      $value
+     * @param null $expireResolution
+     * @param null $expireTTL
+     * @param null $flag
+     *
+     * @return bool
+     */
+    public function set($key, $value, $expireResolution = null, $expireTTL = null, $flag = null)
+    {
+        if (!$this->available) {
+            return false;
+        }
+
+        self::$data[$key] = (string) $value;
+
+        return true;
+    }
+
+    /**
+     * @param $key
+     * @param $value
+     *
+     * @return string|null
+     */
+    public function getset($key, $value)
+    {
+        if (!$this->available) {
+            return false;
+        }
+
+        $oldValue = $this->get($key);
+
+        $this->set($key, $value);
+
+        return $oldValue;
+    }
 }