Increase frequency of lock tries when $timeout is used in FileMutex

framework/CHANGELOG.md

@@ -76,7 +76,9 @@ Yii Framework 2 Change Log
 - Bug #16424: `yii\db\Transaction::begin()` throws now `NotSupportedException` for nested transaction and DBMS not supporting savepoints (bizley)
 - Bug #15204: `yii\helpers\BaseInflector::slug()` is not removing substrings matching provided replacement from given string anymore (bizley)
 - Bug #16101: Fixed Error Handler to clear registered meta tags, link tags, css/js scripts and files in error view (bizley)
-- Bug #16836: Fix `yii\mutex\MysqlMutex` to handle locks with names longer than 64 character (rob006)
+- Bug #16836: Fix `yii\mutex\MysqlMutex` to handle locks with names longer than 64 characters (rob006)
+- Enh #16839: Increase frequency of lock tries for `yii\mutex\FileMutex::acquireLock()` when $timeout is provided (rob006)
+- Enh #16839: Add support for `$timeout` in  `yii\mutex\PgsqlMutex::acquire()` (rob006)
 
 
 2.0.15.1 March 21, 2018

framework/mutex/FileMutex.php

@@ -41,6 +41,8 @@
  */
 class FileMutex extends Mutex
 {
+    use RetryAcquireTrait;
+
     /**
      * @var string the directory to store mutex files. You may use [path alias](guide:concept-aliases) here.
      * Defaults to the "mutex" subdirectory under the application runtime path.
@@ -99,11 +101,8 @@ public function init()
     protected function acquireLock($name, $timeout = 0)
     {
         $filePath = $this->getLockFilePath($name);
-        $waitTime = 0;
-
-        while (true) {
+        return $this->retryAcquire($timeout, function () use ($filePath, $name) {
             $file = fopen($filePath, 'w+');
-
             if ($file === false) {
                 return false;
             }
@@ -114,13 +113,7 @@ protected function acquireLock($name, $timeout = 0)
 
             if (!flock($file, LOCK_EX | LOCK_NB)) {
                 fclose($file);
-
-                if (++$waitTime > $timeout) {
-                    return false;
-                }
-
-                sleep(1);
-                continue;
+                return false;
             }
 
             // Under unix we delete the lock file before releasing the related handle. Thus it's possible that we've acquired a lock on
@@ -137,15 +130,12 @@ protected function acquireLock($name, $timeout = 0)
                 clearstatcache(true, $filePath);
                 flock($file, LOCK_UN);
                 fclose($file);
-                continue;
+                return false;
             }
 
             $this->_files[$name] = $file;
             return true;
-        }
-
-        // Should not be reached normally.
-        return false;
+        });
     }
 
     /**

framework/mutex/PgsqlMutex.php

@@ -36,6 +36,8 @@
  */
 class PgsqlMutex extends DbMutex
 {
+    use RetryAcquireTrait;
+
     /**
      * Initializes PgSQL specific mutex component implementation.
      * @throws InvalidConfigException if [[db]] is not PgSQL connection.
@@ -67,16 +69,16 @@ private function getKeysFromName($name)
      */
     protected function acquireLock($name, $timeout = 0)
     {
-        if ($timeout !== 0) {
-            throw new InvalidArgumentException('PgsqlMutex does not support timeout.');
-        }
         list($key1, $key2) = $this->getKeysFromName($name);
-        return $this->db->useMaster(function ($db) use ($key1, $key2) {
-            /** @var \yii\db\Connection $db */
-            return (bool) $db->createCommand(
-                'SELECT pg_try_advisory_lock(:key1, :key2)',
-                [':key1' => $key1, ':key2' => $key2]
-            )->queryScalar();
+
+        return $this->retryAcquire($timeout, function () use ($key1, $key2) {
+            return $this->db->useMaster(function ($db) use ($key1, $key2) {
+                /** @var \yii\db\Connection $db */
+                return (bool) $db->createCommand(
+                    'SELECT pg_try_advisory_lock(:key1, :key2)',
+                    [':key1' => $key1, ':key2' => $key2]
+                )->queryScalar();
+            });
         });
     }
 

framework/mutex/RetryAcquireTrait.php

@@ -0,0 +1,40 @@
+<?php
+/**
+ * @link http://www.yiiframework.com/
+ * @copyright Copyright (c) 2008 Yii Software LLC
+ * @license http://www.yiiframework.com/license/
+ */
+
+namespace yii\mutex;
+
+use Closure;
+
+/**
+ * Trait RetryAcquireTrait.
+ *
+ * @author Robert Korulczyk <[email protected]>
+ * @internal
+ */
+trait RetryAcquireTrait
+{
+    /**
+     * @var int Number of milliseconds between each try in [[acquire()]] until specified timeout times out.
+     * By default it is 50 milliseconds - it means that [[acquire()]] may try acquire lock up to 20 times per second.
+     * @since 2.0.16
+     */
+    public $retryDelay = 50;
+
+
+    private function retryAcquire($timeout, Closure $callback)
+    {
+        $start = microtime(true);
+        do {
+            if ($callback()) {
+                return true;
+            }
+            usleep($this->retryDelay * 1000);
+        } while (microtime(true) - $start < $timeout);
+
+        return false;
+    }
+}

tests/framework/mutex/MutexTestTrait.php

@@ -52,6 +52,21 @@ public function testThatMutexLockIsWorking($mutexName)
         $this->assertTrue($mutexTwo->acquire($mutexName));
     }
 
+    public function testTimeout()
+    {
+        $mutexName = __FUNCTION__;
+        $mutexOne = $this->createMutex();
+        $mutexTwo = $this->createMutex();
+
+        $this->assertTrue($mutexOne->acquire($mutexName));
+        $microtime = microtime(true);
+        $this->assertFalse($mutexTwo->acquire($mutexName, 1));
+        $diff = microtime(true) - $microtime;
+        $this->assertTrue($diff >= 1 && $diff < 2);
+        $this->assertTrue($mutexOne->release($mutexName));
+        $this->assertFalse($mutexTwo->release($mutexName));
+    }
+
     public static function mutexDataProvider()
     {
         $utf = <<<'UTF'

tests/framework/mutex/RetryAcquireTraitTest.php

@@ -0,0 +1,49 @@
+<?php
+/**
+ * @link http://www.yiiframework.com/
+ * @copyright Copyright (c) 2008 Yii Software LLC
+ * @license http://www.yiiframework.com/license/
+ */
+
+namespace yiiunit\framework\mutex;
+
+use Yii;
+use yii\base\InvalidConfigException;
+use yiiunit\framework\mutex\mocks\DumbMutex;
+use yiiunit\TestCase;
+
+/**
+ * Class RetryAcquireTraitTest.
+ *
+ * @group mutex
+ *
+ * @author Robert Korulczyk <[email protected]>
+ */
+class RetryAcquireTraitTest extends TestCase
+{
+    /**
+     * @throws InvalidConfigException
+     */
+    public function testRetryAcquire()
+    {
+        $mutexName = __FUNCTION__;
+        $mutexOne = $this->createMutex();
+        $mutexTwo = $this->createMutex();
+
+        $this->assertTrue($mutexOne->acquire($mutexName));
+        $this->assertFalse($mutexTwo->acquire($mutexName, 1));
+
+        $this->assertSame(20, $mutexTwo->attemptsCounter);
+    }
+
+    /**
+     * @return DumbMutex
+     * @throws InvalidConfigException
+     */
+    private function createMutex()
+    {
+        return Yii::createObject([
+            'class' => DumbMutex::className(),
+        ]);
+    }
+}

tests/framework/mutex/mocks/DumbMutex.php

@@ -0,0 +1,55 @@
+<?php
+/**
+ * @link http://www.yiiframework.com/
+ * @copyright Copyright (c) 2008 Yii Software LLC
+ * @license http://www.yiiframework.com/license/
+ */
+
+namespace yiiunit\framework\mutex\mocks;
+
+use yii\mutex\Mutex;
+use yii\mutex\RetryAcquireTrait;
+
+/**
+ * Class DumbMutex.
+ *
+ * @author Robert Korulczyk <[email protected]>
+ */
+class DumbMutex extends Mutex
+{
+    use RetryAcquireTrait;
+
+    public $attemptsCounter = 0;
+    public static $locked = false;
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function acquireLock($name, $timeout = 0)
+    {
+        return $this->retryAcquire($timeout, function () {
+            $this->attemptsCounter++;
+            if (!static::$locked) {
+                static::$locked = true;
+
+                return true;
+            }
+
+            return false;
+        });
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function releaseLock($name)
+    {
+        if (static::$locked) {
+            static::$locked = false;
+
+            return true;
+        }
+
+        return false;
+    }
+}