From 4ed08c907b089995403475f1e946dd442b29d34f Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Sun, 20 May 2018 21:00:47 +0200 Subject: [PATCH 01/10] Add debugging calls to Spork\ProcessManager calls, so exceptions become visible --- tests/mutex/MutexConcurrencyTest.php | 4 ++++ tests/mutex/MutexTest.php | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/mutex/MutexConcurrencyTest.php b/tests/mutex/MutexConcurrencyTest.php index ebbde3c8..b48d371d 100644 --- a/tests/mutex/MutexConcurrencyTest.php +++ b/tests/mutex/MutexConcurrencyTest.php @@ -57,9 +57,13 @@ private function getPDO($dsn, $user) private function fork($concurrency, callable $code) { $manager = new ProcessManager(); + $manager->setDebug(true); + for ($i = 0; $i < $concurrency; $i++) { $manager->fork($code); } + + $manager->check(); } /** diff --git a/tests/mutex/MutexTest.php b/tests/mutex/MutexTest.php index e10c4d83..0ef11fad 100644 --- a/tests/mutex/MutexTest.php +++ b/tests/mutex/MutexTest.php @@ -155,6 +155,8 @@ public function testRelease(callable $mutexFactory) public function testLiveness(callable $mutexFactory) { $manager = new ProcessManager(); + $manager->setDebug(true); + $manager->fork(function () use ($mutexFactory) { $mutex = call_user_func($mutexFactory); $mutex->synchronized(function () { @@ -168,6 +170,8 @@ public function testLiveness(callable $mutexFactory) $mutex = call_user_func($mutexFactory); $mutex->synchronized(function () { }); + + $manager->check(); } /** From 6f5b86b71c5e6d56dc03e90c223ae30d72a3e510 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 09:38:01 +0200 Subject: [PATCH 02/10] Add MySQL named locks --- classes/mutex/MySQLMutex.php | 75 ++++++++++++++++++++++++++++ classes/util/Loop.php | 6 +-- tests/mutex/MutexConcurrencyTest.php | 9 ++++ tests/mutex/MutexTest.php | 9 ++++ 4 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 classes/mutex/MySQLMutex.php diff --git a/classes/mutex/MySQLMutex.php b/classes/mutex/MySQLMutex.php new file mode 100644 index 00000000..0b3833b6 --- /dev/null +++ b/classes/mutex/MySQLMutex.php @@ -0,0 +1,75 @@ +pdo = $PDO; + + if (\strlen($name) > 64) { + throw new \InvalidArgumentException("The maximum length of the lock name is 64 characters."); + } + + $this->name = $name; + $this->timeout = $timeout; + } + + /** + * @throws LockAcquireException + */ + public function lock() + { + $statement = $this->pdo->prepare("SELECT GET_LOCK(?,?)"); + + $statement->execute([ + $this->name, + $this->timeout, + ]); + + $statement->setFetchMode(\PDO::FETCH_NUM); + $row = $statement->fetch(); + + if ($row[0] == 1) { + /* + * Returns 1 if the lock was obtained successfully. + */ + return; + } + + if ($row[0] === null) { + /* + * NULL if an error occurred (such as running out of memory or the thread was killed with mysqladmin kill). + */ + throw new LockAcquireException("An error occurred while acquiring the lock"); + } + + throw new TimeoutException("The attempt timed out (for example, because another client has previously locked the name)"); + } + + public function unlock() + { + $statement = $this->pdo->prepare("DO RELEASE_LOCK(?)"); + $statement->execute([ + $this->name + ]); + } +} \ No newline at end of file diff --git a/classes/util/Loop.php b/classes/util/Loop.php index 667f3452..f5d0dc45 100644 --- a/classes/util/Loop.php +++ b/classes/util/Loop.php @@ -93,12 +93,12 @@ public function execute(callable $code) throw new TimeoutException("Timeout of $this->timeout seconds exceeded."); } - $usleep = min($usecRemaining, \random_int($min, $max)); + $usleep = \min($usecRemaining, \random_int($min, $max)); - usleep($usleep); + \usleep($usleep); } - if (microtime(true) >= $timeout) { + if (\microtime(true) >= $timeout) { throw new TimeoutException("Timeout of $this->timeout seconds exceeded."); } diff --git a/tests/mutex/MutexConcurrencyTest.php b/tests/mutex/MutexConcurrencyTest.php index b48d371d..b137ab3e 100644 --- a/tests/mutex/MutexConcurrencyTest.php +++ b/tests/mutex/MutexConcurrencyTest.php @@ -263,6 +263,15 @@ function ($uri) { return new PHPRedisMutex($apis, "test", $timeout); }]; } + + if (getenv("MYSQL_DSN")) { + $cases["MySQLMutex"] = [function($timeout = 3) { + $pdo = new \PDO(getenv("MYSQL_DSN"), getenv("MYSQL_USER")); + $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); + + return new MySQLMutex($pdo, "test", $timeout); + }]; + } return $cases; } diff --git a/tests/mutex/MutexTest.php b/tests/mutex/MutexTest.php index 0ef11fad..1b8ba929 100644 --- a/tests/mutex/MutexTest.php +++ b/tests/mutex/MutexTest.php @@ -109,6 +109,15 @@ function ($uri) { }]; } + if (getenv("MYSQL_DSN")) { + $cases["MySQLMutex"] = [function() { + $pdo = new \PDO(getenv("MYSQL_DSN"), getenv("MYSQL_USER")); + $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); + + return new MySQLMutex($pdo, "test", self::TIMEOUT); + }]; + } + return $cases; } From 3b90f4afc26cb41b4825447680dfed9d002d10c0 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 09:43:48 +0200 Subject: [PATCH 03/10] Allow any version for redis, so we can support "develop" version on PHP nightly --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index d793c603..29c00247 100644 --- a/composer.json +++ b/composer.json @@ -28,7 +28,7 @@ }, "require-dev": { "ext-memcached": "*", - "ext-redis": "^2.2.4|^3.0|^4.0", + "ext-redis": "*", "ext-pdo_mysql": "*", "ext-pdo_sqlite": "*", "kriswallsmith/spork": "^0.3", From 25c62b7962e0aff2f78703e5a3cb6699900fdab1 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 09:50:30 +0200 Subject: [PATCH 04/10] Fix CS issues --- classes/mutex/MySQLMutex.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/mutex/MySQLMutex.php b/classes/mutex/MySQLMutex.php index 0b3833b6..0ed9e4d5 100644 --- a/classes/mutex/MySQLMutex.php +++ b/classes/mutex/MySQLMutex.php @@ -62,7 +62,7 @@ public function lock() throw new LockAcquireException("An error occurred while acquiring the lock"); } - throw new TimeoutException("The attempt timed out (for example, because another client has previously locked the name)"); + throw new TimeoutException("Timeout when acquiring lock."); } public function unlock() @@ -72,4 +72,4 @@ public function unlock() $this->name ]); } -} \ No newline at end of file +} From fbeeaec475beca4fe389da6d8f2464cdcb11f9f1 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 09:57:47 +0200 Subject: [PATCH 05/10] Remove some PHP version constraints that no longer apply --- classes/util/Loop.php | 2 +- tests/mutex/MutexTest.php | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/classes/util/Loop.php b/classes/util/Loop.php index f5d0dc45..d3ca558b 100644 --- a/classes/util/Loop.php +++ b/classes/util/Loop.php @@ -84,7 +84,7 @@ public function execute(callable $code) /* * Calculate max time remaining, don't sleep any longer than that. */ - $usecRemaining = intval(($timeout - microtime(true)) * 1e6); + $usecRemaining = \intval(($timeout - \microtime(true)) * 1e6); if ($usecRemaining <= 0) { /* diff --git a/tests/mutex/MutexTest.php b/tests/mutex/MutexTest.php index 1b8ba929..0513ca66 100644 --- a/tests/mutex/MutexTest.php +++ b/tests/mutex/MutexTest.php @@ -159,7 +159,6 @@ public function testRelease(callable $mutexFactory) * @param callable $mutexFactory The Mutex factory. * @test * @dataProvider provideMutexFactories - * @requires PHP 7.0 */ public function testLiveness(callable $mutexFactory) { @@ -190,7 +189,6 @@ public function testLiveness(callable $mutexFactory) * @test * @dataProvider provideMutexFactories * @expectedException \DomainException - * @requires PHP 5.6 */ public function testSynchronizedPassesExceptionThrough(callable $mutexFactory) { From 44a121651a5240078630abe441474809ca28ac63 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 10:00:22 +0200 Subject: [PATCH 06/10] Fix typo in comment --- classes/util/Loop.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/util/Loop.php b/classes/util/Loop.php index d3ca558b..b7610da8 100644 --- a/classes/util/Loop.php +++ b/classes/util/Loop.php @@ -50,7 +50,7 @@ public function end() } /** - * Repeats executing a code until it was succesful. + * Repeats executing a code until it was successful. * * The code has to be designed in a way that it can be repeated without any * side effects. When execution was successful it should notify that event From cbef652ae15e35d8c03f8fba0e5159d87cf6edf1 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 10:02:24 +0200 Subject: [PATCH 07/10] Remove prefix of microtime and usleep, is used by PHPMock --- classes/util/Loop.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/classes/util/Loop.php b/classes/util/Loop.php index b7610da8..db33adae 100644 --- a/classes/util/Loop.php +++ b/classes/util/Loop.php @@ -84,7 +84,7 @@ public function execute(callable $code) /* * Calculate max time remaining, don't sleep any longer than that. */ - $usecRemaining = \intval(($timeout - \microtime(true)) * 1e6); + $usecRemaining = \intval(($timeout - microtime(true)) * 1e6); if ($usecRemaining <= 0) { /* @@ -95,10 +95,10 @@ public function execute(callable $code) $usleep = \min($usecRemaining, \random_int($min, $max)); - \usleep($usleep); + usleep($usleep); } - if (\microtime(true) >= $timeout) { + if (microtime(true) >= $timeout) { throw new TimeoutException("Timeout of $this->timeout seconds exceeded."); } From 8af4de6548e987a035f8f21100a1cd65700e951c Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 12:48:32 +0200 Subject: [PATCH 08/10] Some small improvements to various tests --- composer.json | 1 + tests/mutex/MutexConcurrencyTest.php | 25 +++++++++++++------------ tests/mutex/MutexTest.php | 2 +- tests/mutex/PHPRedisMutexTest.php | 5 ++++- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/composer.json b/composer.json index 29c00247..fdc3fedb 100644 --- a/composer.json +++ b/composer.json @@ -29,6 +29,7 @@ "require-dev": { "ext-memcached": "*", "ext-redis": "*", + "ext-pcntl": "*", "ext-pdo_mysql": "*", "ext-pdo_sqlite": "*", "kriswallsmith/spork": "^0.3", diff --git a/tests/mutex/MutexConcurrencyTest.php b/tests/mutex/MutexConcurrencyTest.php index b137ab3e..3b9e32a2 100644 --- a/tests/mutex/MutexConcurrencyTest.php +++ b/tests/mutex/MutexConcurrencyTest.php @@ -25,12 +25,11 @@ */ class MutexConcurrencyTest extends \PHPUnit_Framework_TestCase { - /** * @var \PDO The pdo instance. */ private $pdo; - + /** * Gets a PDO instance. * @@ -103,21 +102,22 @@ public function provideTestHighContention() { $cases = array_map(function (array $mutexFactory) { $file = tmpfile(); - fputs($file, pack("i", 0)); - fflush($file); + fwrite($file, pack("i", 0)); return [ function ($increment) use ($file) { - fseek($file, 0); + rewind($file); + flock($file, LOCK_EX); $data = fread($file, 4); $counter = unpack("i", $data)[1]; $counter += $increment; - fseek($file, 0); + rewind($file); fwrite($file, pack("i", $counter)); - fflush($file); - + + flock($file, LOCK_UN); + return $counter; }, $mutexFactory[0] @@ -126,15 +126,16 @@ function ($increment) use ($file) { $addPDO = function ($dsn, $user, $vendor) use (&$cases) { $pdo = $this->getPDO($dsn, $user); - $pdo->beginTransaction(); - + $options = ["mysql" => "engine=InnoDB"]; $option = isset($options[$vendor]) ? $options[$vendor] : ""; $pdo->exec("CREATE TABLE IF NOT EXISTS counter(id INT PRIMARY KEY, counter INT) $option"); - + + $pdo->beginTransaction(); $pdo->exec("DELETE FROM counter"); $pdo->exec("INSERT INTO counter VALUES (1, 0)"); $pdo->commit(); + $this->pdo = null; $cases[$vendor] = [ @@ -265,7 +266,7 @@ function ($uri) { } if (getenv("MYSQL_DSN")) { - $cases["MySQLMutex"] = [function($timeout = 3) { + $cases["MySQLMutex"] = [function ($timeout = 3) { $pdo = new \PDO(getenv("MYSQL_DSN"), getenv("MYSQL_USER")); $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); diff --git a/tests/mutex/MutexTest.php b/tests/mutex/MutexTest.php index 0513ca66..eae8a714 100644 --- a/tests/mutex/MutexTest.php +++ b/tests/mutex/MutexTest.php @@ -110,7 +110,7 @@ function ($uri) { } if (getenv("MYSQL_DSN")) { - $cases["MySQLMutex"] = [function() { + $cases["MySQLMutex"] = [function () { $pdo = new \PDO(getenv("MYSQL_DSN"), getenv("MYSQL_USER")); $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); diff --git a/tests/mutex/PHPRedisMutexTest.php b/tests/mutex/PHPRedisMutexTest.php index 7e2ed56e..3fd31a19 100644 --- a/tests/mutex/PHPRedisMutexTest.php +++ b/tests/mutex/PHPRedisMutexTest.php @@ -3,7 +3,6 @@ namespace malkusch\lock\mutex; use Redis; -use RedisException; /** * Tests for PHPRedisMutex. @@ -93,6 +92,10 @@ public function testSyncronizedWorks($serialization) public function dpSerializationModes() { + if (!class_exists(Redis::class)) { + return []; + } + $serializers = [ [Redis::SERIALIZER_NONE], [Redis::SERIALIZER_PHP], From c29df5ebbd079d57f96a3557af9c1bb4f7bed8bb Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 13:05:58 +0200 Subject: [PATCH 09/10] Tests occasionally fail with timeout of 3s --- tests/mutex/MutexTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/mutex/MutexTest.php b/tests/mutex/MutexTest.php index eae8a714..1c378115 100644 --- a/tests/mutex/MutexTest.php +++ b/tests/mutex/MutexTest.php @@ -23,8 +23,7 @@ */ class MutexTest extends \PHPUnit_Framework_TestCase { - - const TIMEOUT = 3; + const TIMEOUT = 4; /** * Provides Mutex factories. From 941e058a53b97cf211feccc8b3fcf601d90a4365 Mon Sep 17 00:00:00 2001 From: Willem Stuursma-Ruwen Date: Mon, 21 May 2018 13:19:11 +0200 Subject: [PATCH 10/10] Test also prone to random failures --- tests/mutex/MemcachedMutexTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mutex/MemcachedMutexTest.php b/tests/mutex/MemcachedMutexTest.php index e6b1455e..985b0ca5 100644 --- a/tests/mutex/MemcachedMutexTest.php +++ b/tests/mutex/MemcachedMutexTest.php @@ -37,7 +37,7 @@ public function testFailAcquireLock() { $mutex = new MemcachedMutex("testFailAcquireLock", $this->memcached, 1); - $this->memcached->add(MemcachedMutex::PREFIX."testFailAcquireLock", true, 2); + $this->memcached->add(MemcachedMutex::PREFIX."testFailAcquireLock", "xxx", 999); $mutex->synchronized(function () { $this->fail("execution is not expected");