diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 57618af6bda..14112cc8a90 100644 --- a/framework/CHANGELOG.md +++ b/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 diff --git a/framework/mutex/FileMutex.php b/framework/mutex/FileMutex.php index d06a1f5076f..5dc71a088cb 100644 --- a/framework/mutex/FileMutex.php +++ b/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; + }); } /** diff --git a/framework/mutex/PgsqlMutex.php b/framework/mutex/PgsqlMutex.php index e61972ee6b8..a7207cd746c 100644 --- a/framework/mutex/PgsqlMutex.php +++ b/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(); + }); }); } diff --git a/framework/mutex/RetryAcquireTrait.php b/framework/mutex/RetryAcquireTrait.php new file mode 100644 index 00000000000..35465a0175f --- /dev/null +++ b/framework/mutex/RetryAcquireTrait.php @@ -0,0 +1,40 @@ + + * @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; + } +} diff --git a/tests/framework/mutex/MutexTestTrait.php b/tests/framework/mutex/MutexTestTrait.php index 9faeb830ba7..3fa77d360a5 100644 --- a/tests/framework/mutex/MutexTestTrait.php +++ b/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' diff --git a/tests/framework/mutex/RetryAcquireTraitTest.php b/tests/framework/mutex/RetryAcquireTraitTest.php new file mode 100644 index 00000000000..865e81d281c --- /dev/null +++ b/tests/framework/mutex/RetryAcquireTraitTest.php @@ -0,0 +1,49 @@ + + */ +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(), + ]); + } +} diff --git a/tests/framework/mutex/mocks/DumbMutex.php b/tests/framework/mutex/mocks/DumbMutex.php new file mode 100644 index 00000000000..c279bb45085 --- /dev/null +++ b/tests/framework/mutex/mocks/DumbMutex.php @@ -0,0 +1,55 @@ + + */ +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; + } +}