diff --git a/src/mutex/CASMutex.php b/src/mutex/CASMutex.php index e84695d7..24dc8924 100644 --- a/src/mutex/CASMutex.php +++ b/src/mutex/CASMutex.php @@ -27,10 +27,10 @@ class CASMutex extends Mutex * * The default is 3 seconds. * - * @param int $timeout The timeout in seconds. + * @param float $timeout The timeout in seconds. * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(int $timeout = 3) + public function __construct(float $timeout = 3) { $this->loop = new Loop($timeout); } diff --git a/src/mutex/FlockMutex.php b/src/mutex/FlockMutex.php index 8fc5c0fd..500c776a 100644 --- a/src/mutex/FlockMutex.php +++ b/src/mutex/FlockMutex.php @@ -16,7 +16,7 @@ */ class FlockMutex extends LockMutex { - public const INFINITE_TIMEOUT = -1; + public const INFINITE_TIMEOUT = -1.0; /** * @internal @@ -39,12 +39,12 @@ class FlockMutex extends LockMutex private $fileHandle; /** - * @var int + * @var float */ private $timeout; /** - * @var int + * @var self::STRATEGY_* */ private $strategy; @@ -52,9 +52,9 @@ class FlockMutex extends LockMutex * Sets the file handle. * * @param resource $fileHandle The file handle. - * @param int $timeout + * @param float $timeout */ - public function __construct($fileHandle, int $timeout = self::INFINITE_TIMEOUT) + public function __construct($fileHandle, float $timeout = self::INFINITE_TIMEOUT) { if (!is_resource($fileHandle)) { throw new \InvalidArgumentException('The file handle is not a valid resource.'); @@ -65,9 +65,12 @@ public function __construct($fileHandle, int $timeout = self::INFINITE_TIMEOUT) $this->strategy = $this->determineLockingStrategy(); } - private function determineLockingStrategy() + /** + * @return self::STRATEGY_* + */ + private function determineLockingStrategy(): int { - if ($this->timeout == self::INFINITE_TIMEOUT) { + if ($this->timeout === self::INFINITE_TIMEOUT) { return self::STRATEGY_BLOCK; } @@ -94,7 +97,9 @@ private function lockBlocking(): void */ private function lockPcntl(): void { - $timebox = new PcntlTimeout($this->timeout); + $timeoutInt = (int) ceil($this->timeout); + + $timebox = new PcntlTimeout($timeoutInt); try { $timebox->timeBoxed( @@ -103,7 +108,7 @@ function (): void { } ); } catch (DeadlineException $e) { - throw TimeoutException::create($this->timeout); + throw TimeoutException::create($timeoutInt); } } diff --git a/src/mutex/MemcachedMutex.php b/src/mutex/MemcachedMutex.php index 9200c752..21518d96 100644 --- a/src/mutex/MemcachedMutex.php +++ b/src/mutex/MemcachedMutex.php @@ -24,20 +24,24 @@ class MemcachedMutex extends SpinlockMutex * * @param string $name The lock name. * @param Memcached $memcache The connected Memcached API. - * @param int $timeout The time in seconds a lock expires, default is 3. + * @param float $timeout The time in seconds a lock expires, default is 3. * * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(string $name, Memcached $memcache, int $timeout = 3) + public function __construct(string $name, Memcached $memcache, float $timeout = 3) { parent::__construct($name, $timeout); $this->memcache = $memcache; } - protected function acquire(string $key, int $expire): bool + protected function acquire(string $key, float $expire): bool { - return $this->memcache->add($key, true, $expire); + // memcached supports only integer expire + // https://github.com/memcached/memcached/wiki/Commands#standard-protocol + $expireInt = (int) ceil($expire); + + return $this->memcache->add($key, true, $expireInt); } protected function release(string $key): bool diff --git a/src/mutex/MySQLMutex.php b/src/mutex/MySQLMutex.php index f6f7cff3..128c9c18 100644 --- a/src/mutex/MySQLMutex.php +++ b/src/mutex/MySQLMutex.php @@ -20,11 +20,11 @@ class MySQLMutex extends LockMutex */ private $name; /** - * @var int + * @var float */ private $timeout; - public function __construct(\PDO $PDO, string $name, int $timeout = 0) + public function __construct(\PDO $PDO, string $name, float $timeout = 0) { $this->pdo = $PDO; @@ -43,9 +43,15 @@ public function lock(): void { $statement = $this->pdo->prepare('SELECT GET_LOCK(?,?)'); + // MySQL rounds the value to whole seconds, sadly rounds, not ceils + // TODO MariaDB supports microseconds precision since 10.1.2 version, + // but we need to detect the support reliably first + // https://github.com/MariaDB/server/commit/3e792e6cbccb5d7bf5b84b38336f8a40ad232020 + $timeoutInt = (int) ceil($this->timeout); + $statement->execute([ $this->name, - $this->timeout, + $timeoutInt, ]); $statement->setFetchMode(\PDO::FETCH_NUM); diff --git a/src/mutex/PHPRedisMutex.php b/src/mutex/PHPRedisMutex.php index c17e788c..1a858db6 100644 --- a/src/mutex/PHPRedisMutex.php +++ b/src/mutex/PHPRedisMutex.php @@ -29,12 +29,12 @@ class PHPRedisMutex extends RedisMutex * called already. * * @param array<\Redis|\RedisCluster> $redisAPIs The Redis connections. - * @param string $name The lock name. - * @param int $timeout The time in seconds a lock expires after. Default is - * 3 seconds. + * @param string $name The lock name. + * @param float $timeout The time in seconds a lock expires after. Default is + * 3 seconds. * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(array $redisAPIs, string $name, int $timeout = 3) + public function __construct(array $redisAPIs, string $name, float $timeout = 3) { parent::__construct($redisAPIs, $name, $timeout); } @@ -43,12 +43,14 @@ public function __construct(array $redisAPIs, string $name, int $timeout = 3) * @param \Redis|\RedisCluster $redisAPI The Redis or RedisCluster connection. * @throws LockAcquireException */ - protected function add($redisAPI, string $key, string $value, int $expire): bool + protected function add($redisAPI, string $key, string $value, float $expire): bool { + $expireMillis = (int) ceil($expire * 1000); + /** @var \Redis $redisAPI */ try { // Will set the key, if it doesn't exist, with a ttl of $expire seconds - return $redisAPI->set($key, $value, ['nx', 'ex' => $expire]); + return $redisAPI->set($key, $value, ['nx', 'px' => $expireMillis]); } catch (RedisException $e) { $message = sprintf( "Failed to acquire lock for key '%s'", diff --git a/src/mutex/PredisMutex.php b/src/mutex/PredisMutex.php index 1f436c08..20002880 100644 --- a/src/mutex/PredisMutex.php +++ b/src/mutex/PredisMutex.php @@ -20,12 +20,12 @@ class PredisMutex extends RedisMutex * Sets the Redis connections. * * @param ClientInterface[] $clients The Redis clients. - * @param string $name The lock name. - * @param int $timeout The time in seconds a lock expires, default is 3. + * @param string $name The lock name. + * @param float $timeout The time in seconds a lock expires, default is 3. * * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(array $clients, string $name, int $timeout = 3) + public function __construct(array $clients, string $name, float $timeout = 3) { parent::__construct($clients, $name, $timeout); } @@ -33,11 +33,13 @@ public function __construct(array $clients, string $name, int $timeout = 3) /** * @throws LockAcquireException */ - protected function add($redisAPI, string $key, string $value, int $expire): bool + protected function add($redisAPI, string $key, string $value, float $expire): bool { + $expireMillis = (int) ceil($expire * 1000); + /** @var ClientInterface $redisAPI */ try { - return $redisAPI->set($key, $value, 'EX', $expire, 'NX') !== null; + return $redisAPI->set($key, $value, 'PX', $expireMillis, 'NX') !== null; } catch (PredisException $e) { $message = sprintf( "Failed to acquire lock for key '%s'", diff --git a/src/mutex/RedisMutex.php b/src/mutex/RedisMutex.php index 3942e4a3..efebcdd8 100644 --- a/src/mutex/RedisMutex.php +++ b/src/mutex/RedisMutex.php @@ -34,11 +34,11 @@ abstract class RedisMutex extends SpinlockMutex implements LoggerAwareInterface * * @param array $redisAPIs The Redis APIs. * @param string $name The lock name. - * @param int $timeout The time in seconds a lock expires, default is 3. + * @param float $timeout The time in seconds a lock expires, default is 3. * * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(array $redisAPIs, string $name, int $timeout = 3) + public function __construct(array $redisAPIs, string $name, float $timeout = 3) { parent::__construct($name, $timeout); @@ -46,7 +46,7 @@ public function __construct(array $redisAPIs, string $name, int $timeout = 3) $this->logger = new NullLogger(); } - protected function acquire(string $key, int $expire): bool + protected function acquire(string $key, float $expire): bool { // 1. This differs from the specification to avoid an overflow on 32-Bit systems. $time = microtime(true); @@ -149,14 +149,14 @@ private function isMajority(int $count): bool /** * Sets the key only if such key doesn't exist at the server yet. * - * @param mixed $redisAPI The connected Redis API. - * @param string $key The key. - * @param string $value The value. - * @param int $expire The TTL seconds. + * @param mixed $redisAPI The connected Redis API. + * @param string $key The key. + * @param string $value The value. + * @param float $expire The TTL seconds. * * @return bool True, if the key was set. */ - abstract protected function add($redisAPI, string $key, string $value, int $expire): bool; + abstract protected function add($redisAPI, string $key, string $value, float $expire): bool; /** * @param mixed $redisAPI The connected Redis API. diff --git a/src/mutex/SpinlockMutex.php b/src/mutex/SpinlockMutex.php index 86cdcc88..6ba9a283 100644 --- a/src/mutex/SpinlockMutex.php +++ b/src/mutex/SpinlockMutex.php @@ -22,7 +22,7 @@ abstract class SpinlockMutex extends LockMutex private const PREFIX = 'lock_'; /** - * @var int The timeout in seconds a lock may live. + * @var float The timeout in seconds a lock may live. */ private $timeout; @@ -44,11 +44,11 @@ abstract class SpinlockMutex extends LockMutex /** * Sets the timeout. * - * @param int $timeout The time in seconds a lock expires, default is 3. + * @param float $timeout The time in seconds a lock expires, default is 3. * * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(string $name, int $timeout = 3) + public function __construct(string $name, float $timeout = 3) { $this->timeout = $timeout; $this->loop = new Loop($this->timeout); @@ -92,13 +92,13 @@ protected function unlock(): void /** * Tries to acquire a lock. * - * @param string $key The lock key. - * @param int $expire The timeout in seconds when a lock expires. + * @param string $key The lock key. + * @param float $expire The timeout in seconds when a lock expires. * * @throws LockAcquireException An unexpected error happened. * @return bool True, if the lock could be acquired. */ - abstract protected function acquire(string $key, int $expire): bool; + abstract protected function acquire(string $key, float $expire): bool; /** * Tries to release a lock. diff --git a/src/mutex/TransactionalMutex.php b/src/mutex/TransactionalMutex.php index 56680fbb..3325ca50 100644 --- a/src/mutex/TransactionalMutex.php +++ b/src/mutex/TransactionalMutex.php @@ -40,12 +40,12 @@ class TransactionalMutex extends Mutex * As this implementation spans a transaction over a unit of work, * PDO::ATTR_AUTOCOMMIT SHOULD not be enabled. * - * @param \PDO $pdo The PDO. - * @param int $timeout The timeout in seconds, default is 3. + * @param \PDO $pdo The PDO. + * @param float $timeout The timeout in seconds, default is 3. * * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(\PDO $pdo, int $timeout = 3) + public function __construct(\PDO $pdo, float $timeout = 3) { if ($pdo->getAttribute(\PDO::ATTR_ERRMODE) !== PDO::ERRMODE_EXCEPTION) { throw new InvalidArgumentException('The pdo must have PDO::ERRMODE_EXCEPTION set.'); diff --git a/src/util/Loop.php b/src/util/Loop.php index 91e8945f..acc719a4 100644 --- a/src/util/Loop.php +++ b/src/util/Loop.php @@ -29,7 +29,7 @@ class Loop private const MAXIMUM_WAIT_US = 5e5; // 0.50 seconds /** - * @var int The timeout in seconds. + * @var float The timeout in seconds. */ private $timeout; @@ -41,10 +41,10 @@ class Loop /** * Sets the timeout. The default is 3 seconds. * - * @param int $timeout The timeout in seconds. The default is 3 seconds. + * @param float $timeout The timeout in seconds. The default is 3 seconds. * @throws \LengthException The timeout must be greater than 0. */ - public function __construct(int $timeout = 3) + public function __construct(float $timeout = 3) { if ($timeout <= 0) { throw new LengthException(\sprintf( diff --git a/src/util/PcntlTimeout.php b/src/util/PcntlTimeout.php index dacd0744..cf3280ca 100644 --- a/src/util/PcntlTimeout.php +++ b/src/util/PcntlTimeout.php @@ -14,6 +14,8 @@ * * This class requires the pcntl module and supports the cli sapi only. * + * Only integer timeout is supported - https://github.com/php/php-src/issues/11828. + * * @internal */ final class PcntlTimeout diff --git a/tests/mutex/MutexConcurrencyTest.php b/tests/mutex/MutexConcurrencyTest.php index fb950dae..43725207 100644 --- a/tests/mutex/MutexConcurrencyTest.php +++ b/tests/mutex/MutexConcurrencyTest.php @@ -197,18 +197,18 @@ function ($timeout = 3) use ($dsn, $user, $password) { */ public function testExecutionIsSerializedWhenLocked(callable $mutexFactory) { - $timestamp = hrtime(true); + $time = \microtime(true); - $this->fork(5, function () use ($mutexFactory): void { + $this->fork(6, function () use ($mutexFactory): void { /** @var Mutex $mutex */ $mutex = $mutexFactory(); $mutex->synchronized(function (): void { - \usleep(200000); + \usleep(200 * 1000); }); }); - $delta = \hrtime(true) - $timestamp; - $this->assertGreaterThan(1e9, $delta); + $delta = \microtime(true) - $time; + $this->assertGreaterThan(1.201, $delta); } /** diff --git a/tests/mutex/PredisMutexTest.php b/tests/mutex/PredisMutexTest.php index d46e4421..5b5705a2 100644 --- a/tests/mutex/PredisMutexTest.php +++ b/tests/mutex/PredisMutexTest.php @@ -40,7 +40,7 @@ protected function setUp(): void ->setMethods(array_merge(get_class_methods(ClientInterface::class), ['set', 'eval'])) ->getMock(); - $this->mutex = new PredisMutex([$this->client], 'test'); + $this->mutex = new PredisMutex([$this->client], 'test', 2.5); $this->logger = $this->createMock(LoggerInterface::class); $this->mutex->setLogger($this->logger); @@ -53,7 +53,7 @@ public function testAddFailsToSetKey() { $this->client->expects($this->atLeastOnce()) ->method('set') - ->with('lock_test', $this->isType('string'), 'EX', 4, 'NX') + ->with('lock_test', $this->isType('string'), 'PX', 3500, 'NX') ->willReturn(null); $this->logger->expects($this->never()) @@ -75,7 +75,7 @@ public function testAddErrors() { $this->client->expects($this->atLeastOnce()) ->method('set') - ->with('lock_test', $this->isType('string'), 'EX', 4, 'NX') + ->with('lock_test', $this->isType('string'), 'PX', 3500, 'NX') ->willThrowException($this->createMock(PredisException::class)); $this->logger->expects($this->once()) @@ -95,7 +95,7 @@ public function testWorksNormally() { $this->client->expects($this->atLeastOnce()) ->method('set') - ->with('lock_test', $this->isType('string'), 'EX', 4, 'NX') + ->with('lock_test', $this->isType('string'), 'PX', 3500, 'NX') ->willReturnSelf(); $this->client->expects($this->once()) @@ -119,7 +119,7 @@ public function testEvalScriptFails() { $this->client->expects($this->atLeastOnce()) ->method('set') - ->with('lock_test', $this->isType('string'), 'EX', 4, 'NX') + ->with('lock_test', $this->isType('string'), 'PX', 3500, 'NX') ->willReturnSelf(); $this->client->expects($this->once()) diff --git a/tests/mutex/RedisMutexTest.php b/tests/mutex/RedisMutexTest.php index e5cf0aaa..53eb6183 100644 --- a/tests/mutex/RedisMutexTest.php +++ b/tests/mutex/RedisMutexTest.php @@ -36,12 +36,12 @@ protected function setUp(): void /** * Builds a testable RedisMutex mock. * - * @param int $count The amount of redis apis. - * @param int $timeout The timeout. + * @param int $count The amount of redis apis. + * @param float $timeout The timeout. * * @return MockObject|RedisMutex */ - private function buildRedisMutex(int $count, int $timeout = 1) + private function buildRedisMutex(int $count, float $timeout = 1) { $redisAPIs = array_map( function ($id): array { @@ -156,13 +156,13 @@ function () use (&$i, $available): bool { /** * Tests acquiring keys takes too long. * - * @param int $count The total count of servers. - * @param int $timeout The timeout in seconds. - * @param int $delay The delay in microseconds. + * @param int $count The total count of servers. + * @param float $timeout The timeout in seconds. + * @param float $delay The delay in seconds. * * @dataProvider provideTestTimingOut */ - public function testTimingOut(int $count, int $timeout, int $delay) + public function testTimingOut(int $count, float $timeout, float $delay) { $timeoutStr = (string) round($timeout, 6); if (strpos($timeoutStr, '.') === false) { @@ -177,7 +177,7 @@ public function testTimingOut(int $count, int $timeout, int $delay) $mutex->expects($this->exactly($count)) ->method('add') ->willReturnCallback(function () use ($delay): bool { - usleep($delay); + usleep((int) ($delay * 1e6)); return true; }); @@ -196,8 +196,8 @@ public function provideTestTimingOut() { // count, timeout, delay return [ - [1, 1, 2001000], - [2, 1, 1001000], + [1, 1.2 - 1, 1.201], + [2, 1.2 - 1, 1.401], ]; } diff --git a/tests/mutex/SpinlockMutexTest.php b/tests/mutex/SpinlockMutexTest.php index 85616ffd..87243aec 100644 --- a/tests/mutex/SpinlockMutexTest.php +++ b/tests/mutex/SpinlockMutexTest.php @@ -72,7 +72,7 @@ public function testAcquireTimesOut() public function testExecuteTooLong() { /** @var SpinlockMutex|\PHPUnit\Framework\MockObject\MockObject $mutex */ - $mutex = $this->getMockForAbstractClass(SpinlockMutex::class, ['test', 1]); + $mutex = $this->getMockForAbstractClass(SpinlockMutex::class, ['test', 0.5]); $mutex->expects($this->any()) ->method('acquire') ->willReturn(true); @@ -82,13 +82,13 @@ public function testExecuteTooLong() ->willReturn(true); $this->expectException(ExecutionOutsideLockException::class); -// $this->expectExceptionMessageRegExp( -// '/The code executed for \d+\.\d+ seconds. But the timeout is 1 ' . -// 'seconds. The last \d+\.\d+ seconds were executed outside of the lock./' -// ); + $this->expectExceptionMessageMatches( + '/The code executed for 0\.5\d+ seconds. But the timeout is 0\.5 ' . + 'seconds. The last 0\.0\d+ seconds were executed outside of the lock./' + ); $mutex->synchronized(function () { - usleep(1100 * 1000); + usleep(501 * 1000); }); } @@ -98,12 +98,12 @@ public function testExecuteTooLong() */ public function testExecuteBarelySucceeds() { - $mutex = $this->getMockForAbstractClass(SpinlockMutex::class, ['test', 1]); + $mutex = $this->getMockForAbstractClass(SpinlockMutex::class, ['test', 0.5]); $mutex->expects($this->any())->method('acquire')->willReturn(true); $mutex->expects($this->once())->method('release')->willReturn(true); $mutex->synchronized(function () { - usleep(999999); + usleep(499 * 1000); }); } @@ -127,16 +127,16 @@ public function testFailReleasingLock() */ public function testExecuteTimeoutLeavesOneSecondForKeyToExpire() { - $mutex = $this->getMockForAbstractClass(SpinlockMutex::class, ['test', 3]); + $mutex = $this->getMockForAbstractClass(SpinlockMutex::class, ['test', 0.2]); $mutex->expects($this->once()) ->method('acquire') - ->with($this->anything(), 4) + ->with($this->anything(), 1.2) ->willReturn(true); $mutex->expects($this->once())->method('release')->willReturn(true); $mutex->synchronized(function () { - usleep(2999999); + usleep(199 * 1000); }); } } diff --git a/tests/util/LoopTest.php b/tests/util/LoopTest.php index 0e46c9b3..25ce01b6 100644 --- a/tests/util/LoopTest.php +++ b/tests/util/LoopTest.php @@ -43,13 +43,27 @@ public function testExecutionWithinTimeout() { $this->expectNotToPerformAssertions(); - $loop = new Loop(1); + $loop = new Loop(0.5); $loop->execute(function () use ($loop): void { - usleep(999999); + usleep(499 * 1000); $loop->end(); }); } + /** + * Tests execution within the timeout without calling end(). + */ + public function testExecutionWithinTimeoutWithoutExplicitEnd() + { + $this->expectException(TimeoutException::class); + $this->expectExceptionMessage('Timeout of 0.5 seconds exceeded.'); + + $loop = new Loop(0.5); + $loop->execute(function (): void { + usleep(10 * 1000); + }); + } + /** * Tests exceeding the execution timeout. */ @@ -57,9 +71,9 @@ public function testExceedTimeoutIsAcceptableIfEndWasCalled() { $this->expectNotToPerformAssertions(); - $loop = new Loop(1); + $loop = new Loop(0.5); $loop->execute(function () use ($loop): void { - sleep(1); + usleep(501 * 1000); $loop->end(); }); } @@ -70,11 +84,11 @@ public function testExceedTimeoutIsAcceptableIfEndWasCalled() public function testExceedTimeoutWithoutExplicitEnd() { $this->expectException(TimeoutException::class); - $this->expectExceptionMessage('Timeout of 1.0 seconds exceeded.'); + $this->expectExceptionMessage('Timeout of 0.5 seconds exceeded.'); - $loop = new Loop(1); + $loop = new Loop(0.5); $loop->execute(function (): void { - sleep(1); + usleep(501 * 1000); }); }