From 2ed8b09eadd87b14d298c072dd56df3decc6888a Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sat, 27 Oct 2018 18:32:04 +0200 Subject: [PATCH 1/8] Increase frequency of lock acquire tries when $timeout is specified. --- framework/mutex/FileMutex.php | 22 +++++----------- framework/mutex/RetryAcquireTrait.php | 38 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 framework/mutex/RetryAcquireTrait.php 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/RetryAcquireTrait.php b/framework/mutex/RetryAcquireTrait.php new file mode 100644 index 00000000000..086d21e8e41 --- /dev/null +++ b/framework/mutex/RetryAcquireTrait.php @@ -0,0 +1,38 @@ + + * @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. + */ + public $retryFrequency = 50; + + + private function retryAcquire($timeout, Closure $callback) + { + $start = microtime(true); + do { + if ($callback()) { + return true; + } + } while (microtime(true) - $start < $timeout); + + return false; + } +} From 73fdf35b59a9712ad6d895f09c12000a7abcfbb4 Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sat, 27 Oct 2018 22:21:09 +0200 Subject: [PATCH 2/8] Add tests. --- tests/framework/mutex/MutexTestTrait.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/framework/mutex/MutexTestTrait.php b/tests/framework/mutex/MutexTestTrait.php index 9faeb830ba7..45ef97abccb 100644 --- a/tests/framework/mutex/MutexTestTrait.php +++ b/tests/framework/mutex/MutexTestTrait.php @@ -34,6 +34,19 @@ public function testMutexAcquire($mutexName) $this->assertTrue($mutex->release($mutexName)); } + public function testTimeout() { + $mutex = $this->createMutex(); + $mutexName = __FUNCTION__; + + $this->assertTrue($mutex->acquire($mutexName)); + $microtime = microtime(true); + $this->assertFalse($mutex->acquire($mutexName, 1)); + $diff = microtime(true) - $microtime; + $this->assertTrue($diff >= 1 && $diff < 2); + $this->assertTrue($mutex->release($mutexName)); + $this->assertFalse($mutex->release($mutexName)); + } + /** * @dataProvider mutexDataProvider() * From eccb15506da0e7e06a22a5bbe6cf576add23c6d6 Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sat, 27 Oct 2018 22:30:53 +0200 Subject: [PATCH 3/8] Support timeout in PgsqlMutex. --- framework/mutex/PgsqlMutex.php | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) 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(); + }); }); } From 4db331cd7b55a728bad46c20b7fd7590e800d017 Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sat, 27 Oct 2018 22:50:03 +0200 Subject: [PATCH 4/8] Fix RetryAcquireTrait - add missing delay between tries. --- framework/mutex/RetryAcquireTrait.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/framework/mutex/RetryAcquireTrait.php b/framework/mutex/RetryAcquireTrait.php index 086d21e8e41..0ee1b8ffe5b 100644 --- a/framework/mutex/RetryAcquireTrait.php +++ b/framework/mutex/RetryAcquireTrait.php @@ -21,7 +21,7 @@ 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. */ - public $retryFrequency = 50; + public $retryDelay = 50; private function retryAcquire($timeout, Closure $callback) @@ -31,6 +31,7 @@ private function retryAcquire($timeout, Closure $callback) if ($callback()) { return true; } + usleep($this->retryDelay * 1000); } while (microtime(true) - $start < $timeout); return false; From 305239767489be5d0aea346bd5725b8a96415d9a Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sun, 28 Oct 2018 00:25:05 +0200 Subject: [PATCH 5/8] Fix tests. --- tests/framework/mutex/MutexTestTrait.php | 28 +++++++++++++----------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/framework/mutex/MutexTestTrait.php b/tests/framework/mutex/MutexTestTrait.php index 45ef97abccb..3fa77d360a5 100644 --- a/tests/framework/mutex/MutexTestTrait.php +++ b/tests/framework/mutex/MutexTestTrait.php @@ -34,19 +34,6 @@ public function testMutexAcquire($mutexName) $this->assertTrue($mutex->release($mutexName)); } - public function testTimeout() { - $mutex = $this->createMutex(); - $mutexName = __FUNCTION__; - - $this->assertTrue($mutex->acquire($mutexName)); - $microtime = microtime(true); - $this->assertFalse($mutex->acquire($mutexName, 1)); - $diff = microtime(true) - $microtime; - $this->assertTrue($diff >= 1 && $diff < 2); - $this->assertTrue($mutex->release($mutexName)); - $this->assertFalse($mutex->release($mutexName)); - } - /** * @dataProvider mutexDataProvider() * @@ -65,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' From f9ed822355394b44289d7136718ebf6c169cb55e Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sun, 28 Oct 2018 00:50:43 +0200 Subject: [PATCH 6/8] Add tests for `RetryAcquireTrait::retryAcquire()`. --- .../framework/mutex/RetryAcquireTraitTest.php | 48 ++++++++++++++++ tests/framework/mutex/mocks/DumbMutex.php | 55 +++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 tests/framework/mutex/RetryAcquireTraitTest.php create mode 100644 tests/framework/mutex/mocks/DumbMutex.php diff --git a/tests/framework/mutex/RetryAcquireTraitTest.php b/tests/framework/mutex/RetryAcquireTraitTest.php new file mode 100644 index 00000000000..74ddb32d2a0 --- /dev/null +++ b/tests/framework/mutex/RetryAcquireTraitTest.php @@ -0,0 +1,48 @@ + + */ +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; + } +} From 20b7c23d0c5c7da6526a3500441aa049be64f1be Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sun, 28 Oct 2018 00:52:57 +0200 Subject: [PATCH 7/8] Fix CS. --- tests/framework/mutex/RetryAcquireTraitTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/framework/mutex/RetryAcquireTraitTest.php b/tests/framework/mutex/RetryAcquireTraitTest.php index 74ddb32d2a0..865e81d281c 100644 --- a/tests/framework/mutex/RetryAcquireTraitTest.php +++ b/tests/framework/mutex/RetryAcquireTraitTest.php @@ -24,7 +24,8 @@ class RetryAcquireTraitTest extends TestCase /** * @throws InvalidConfigException */ - public function testRetryAcquire() { + public function testRetryAcquire() + { $mutexName = __FUNCTION__; $mutexOne = $this->createMutex(); $mutexTwo = $this->createMutex(); From 94017a3da507c840553c461417b236e8f0c41a95 Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sun, 28 Oct 2018 01:15:22 +0200 Subject: [PATCH 8/8] Changelog line. Add `@since` annotation. --- framework/CHANGELOG.md | 4 +++- framework/mutex/RetryAcquireTrait.php | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) 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/RetryAcquireTrait.php b/framework/mutex/RetryAcquireTrait.php index 0ee1b8ffe5b..35465a0175f 100644 --- a/framework/mutex/RetryAcquireTrait.php +++ b/framework/mutex/RetryAcquireTrait.php @@ -20,6 +20,7 @@ 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;