Infinite loop in mutex dtor when backend is unavailable

src/NinjaMutex/Mutex.php

@@ -10,6 +10,7 @@
 namespace NinjaMutex;
 
 use NinjaMutex\Lock\LockInterface;
+use NinjaMutex\MutexException;
 
 /**
  * Mutex
@@ -18,13 +19,6 @@
  */
 class Mutex
 {
-    /**
-     * Is mutex acquired?
-     *
-     * @var bool
-     */
-    protected $acquired = false;
-
     /**
      * Lock implementor
      *
@@ -62,9 +56,10 @@ public function __construct($name, LockInterface $lockImplementor)
      */
     public function acquireLock($timeout = null)
     {
-        if ($this->counter > 0 || $this->lockImplementor->acquireLock($this->name, $timeout)) {
+        if ($this->counter > 0 ||
+            $this->lockImplementor->acquireLock($this->name, $timeout)) {
             $this->counter++;
-            return $this->acquired = true;
+            return true;
         }
 
         return false;
@@ -75,23 +70,27 @@ public function acquireLock($timeout = null)
      */
     public function releaseLock()
     {
-        if ($this->acquired) {
+        if ($this->counter > 0) {
             $this->counter--;
-            if ($this->counter > 0) {
+            if ($this->counter > 0 ||
+                $this->lockImplementor->releaseLock($this->name)) {
                 return true;
             }
-
-            return !($this->acquired = !$this->lockImplementor->releaseLock($this->name));
+            $this->counter++;
         }
-
         return false;
     }
 
     public function __destruct()
     {
         // If we acquired lock then we should release it
-        while ($this->acquired) {
-            $this->releaseLock();
+        while ($this->counter > 0) {
+            if (!$this->releaseLock()) {
+                throw new MutexException(sprintf(
+                    'Cannot release lock: %s',
+                    $this->name
+                ));
+            }
         }
     }
 
@@ -102,7 +101,7 @@ public function __destruct()
      */
     public function isAcquired()
     {
-        return $this->acquired;
+        return $this->counter > 0;
     }
 
     /**

tests/NinjaMutex/Mock/MockPredisClient.php

@@ -23,6 +23,12 @@ class MockPredisClient extends Predis\Client
      */
     protected static $data = array();
 
+    /**
+     * Whether the service is available
+     * @var boolean
+     */
+    private $available = true;
+
     public function __construct()
     {
     }
@@ -34,6 +40,9 @@ public function __construct()
      */
     public function setnx($key, $value)
     {
+        if (!$this->available) {
+            return false;
+        }
         if (null === $this->get($key)) {
             self::$data[$key] = (string)$value;
             return true;
@@ -48,6 +57,9 @@ public function setnx($key, $value)
      */
     public function get($key)
     {
+        if (!$this->available) {
+            return false;
+        }
         if (!isset(self::$data[$key])) {
             return null;
         }
@@ -61,7 +73,18 @@ public function get($key)
      */
     public function del($key)
     {
+        if (!$this->available) {
+            return false;
+        }
         unset(self::$data[$key]);
         return true;
     }
+
+    /**
+     * @param boolean $b
+     */
+    public function setAvailable($b = true)
+    {
+        $this->available = (bool) $b;
+    }
 }

tests/NinjaMutex/MutexTest.php

@@ -12,6 +12,9 @@
 use NinjaMutex\AbstractTest;
 use NinjaMutex\Lock\LockAbstract;
 use NinjaMutex\Lock\LockInterface;
+use NinjaMutex\Mock\MockPredisClient;
+use NinjaMutex\Lock\PredisRedisLock;
+use NinjaMutex\MutexException;
 use NinjaMutex\Mutex;
 
 /**
@@ -188,4 +191,24 @@ public function testIfMutexIsReusableAfterSeveralAcquireReleaseCycles(LockInterf
         // cleanup
         $secondMutex->releaseLock();
     }
+
+    /**
+     * @expectedException NinjaMutex\MutexException
+     */
+    public function testIfMutexDestructorThrowsWhenBackendIsUnavailable() {
+        $predisMock = new MockPredisClient();
+        $lockImplementor = new PredisRedisLock($predisMock);
+        $mutex = new Mutex('forfiter', $lockImplementor);
+
+        $this->assertFalse($mutex->isAcquired());
+        $mutex->acquireLock();
+        $this->assertTrue($mutex->isAcquired());
+        $mutex->acquireLock();
+        $this->assertTrue($mutex->isAcquired());
+
+        // make backend unavailable
+        $predisMock->setAvailable(false);
+        // explicit dtor call, should throw MutexException
+        $mutex->__destruct();
+    }
 }