diff --git a/src/Exception/LockReleaseException.php b/src/Exception/LockReleaseException.php index 72acafec..e9d1a27c 100644 --- a/src/Exception/LockReleaseException.php +++ b/src/Exception/LockReleaseException.php @@ -9,8 +9,9 @@ * * Take this exception very serious. * - * Failing to release a lock might have the potential to introduce deadlocks. Also the - * critical code was executed i.e. side effects may have happened. + * This exception implies that the critical code was executed, i.e. side effects may have happened. + * + * Failing to release a lock might have the potential to introduce deadlocks. */ class LockReleaseException extends MutexException { @@ -20,9 +21,9 @@ class LockReleaseException extends MutexException private ?\Throwable $codeException = null; /** - * Gets the result that has been returned during the critical code execution. + * The return value of the executed critical code. * - * @return mixed The return value of the executed code block + * @return mixed */ public function getCodeResult() { @@ -42,7 +43,7 @@ public function setCodeResult($codeResult): self } /** - * Gets the exception that has happened during the synchronized code execution. + * The exception that has happened during the critical code execution. */ public function getCodeException(): ?\Throwable { diff --git a/src/Mutex/AbstractLockMutex.php b/src/Mutex/AbstractLockMutex.php index 3658717a..745d46c0 100644 --- a/src/Mutex/AbstractLockMutex.php +++ b/src/Mutex/AbstractLockMutex.php @@ -15,48 +15,46 @@ abstract class AbstractLockMutex extends AbstractMutex { /** - * Acquires the lock. + * Acquire a lock. * * This method blocks until the lock was acquired. * - * @throws LockAcquireException The lock could not be acquired + * @throws LockAcquireException */ abstract protected function lock(): void; /** - * Releases the lock. + * Release the lock. * - * @throws LockReleaseException The lock could not be released + * @throws LockReleaseException */ abstract protected function unlock(): void; #[\Override] - public function synchronized(callable $code) + public function synchronized(callable $fx) { $this->lock(); - $codeResult = null; - $codeException = null; + $fxResult = null; + $fxException = null; try { - $codeResult = $code(); - } catch (\Throwable $exception) { - $codeException = $exception; - - throw $exception; + $fxResult = $fx(); + } catch (\Throwable $fxException) { + throw $fxException; } finally { try { $this->unlock(); } catch (LockReleaseException $lockReleaseException) { - $lockReleaseException->setCodeResult($codeResult); + $lockReleaseException->setCodeResult($fxResult); - if ($codeException !== null) { - $lockReleaseException->setCodeException($codeException); + if ($fxException !== null) { + $lockReleaseException->setCodeException($fxException); } throw $lockReleaseException; } } - return $codeResult; + return $fxResult; } } diff --git a/src/Mutex/AbstractRedlockMutex.php b/src/Mutex/AbstractRedlockMutex.php index b9715b21..d3b7bdde 100644 --- a/src/Mutex/AbstractRedlockMutex.php +++ b/src/Mutex/AbstractRedlockMutex.php @@ -32,11 +32,11 @@ abstract class AbstractRedlockMutex extends AbstractSpinlockMutex implements Log * called already. * * @param array $clients - * @param float $timeout The timeout in seconds a lock expires + * @param float $acquireTimeout In seconds */ - public function __construct(array $clients, string $name, float $timeout = 3) + public function __construct(array $clients, string $name, float $acquireTimeout = 3) { - parent::__construct($name, $timeout); + parent::__construct($name, $acquireTimeout); $this->clients = $clients; $this->logger = new NullLogger(); diff --git a/src/Mutex/AbstractSpinlockMutex.php b/src/Mutex/AbstractSpinlockMutex.php index 3a0c213d..33583b6c 100644 --- a/src/Mutex/AbstractSpinlockMutex.php +++ b/src/Mutex/AbstractSpinlockMutex.php @@ -20,18 +20,18 @@ abstract class AbstractSpinlockMutex extends AbstractLockMutex private string $key; /** In seconds */ - private float $timeout; + private float $acquireTimeout; /** The timestamp when the lock was acquired */ private ?float $acquiredTs = null; /** - * @param float $timeout The timeout in seconds a lock expires + * @param float $acquireTimeout In seconds */ - public function __construct(string $name, float $timeout = 3) + public function __construct(string $name, float $acquireTimeout = 3) { $this->key = LockUtil::getInstance()->getKeyPrefix() . ':' . $name; - $this->timeout = $timeout; + $this->acquireTimeout = $acquireTimeout; } #[\Override] @@ -49,18 +49,18 @@ protected function lock(): void * acquires successfully the same key which would then be deleted * by this process. */ - if ($this->acquire($this->key, $this->timeout + 1)) { + if ($this->acquire($this->key, $this->acquireTimeout + 1)) { $loop->end(); } - }, $this->timeout); + }, $this->acquireTimeout); } #[\Override] protected function unlock(): void { $elapsedTime = microtime(true) - $this->acquiredTs; - if ($elapsedTime > $this->timeout) { - throw ExecutionOutsideLockException::create($elapsedTime, $this->timeout); + if ($elapsedTime > $this->acquireTimeout) { + throw ExecutionOutsideLockException::create($elapsedTime, $this->acquireTimeout); } /* @@ -75,11 +75,11 @@ protected function unlock(): void /** * Try to acquire a lock. * - * @param float $expire The timeout in seconds when a lock expires + * @param float $expire In seconds * * @return bool True if the lock was acquired * - * @throws LockAcquireException an unexpected error happened + * @throws LockAcquireException An unexpected error happened */ abstract protected function acquire(string $key, float $expire): bool; diff --git a/src/Mutex/FlockMutex.php b/src/Mutex/FlockMutex.php index 9ad222fd..6aef156b 100644 --- a/src/Mutex/FlockMutex.php +++ b/src/Mutex/FlockMutex.php @@ -8,6 +8,7 @@ use Malkusch\Lock\Exception\LockAcquireException; use Malkusch\Lock\Exception\LockAcquireTimeoutException; use Malkusch\Lock\Exception\LockReleaseException; +use Malkusch\Lock\Util\LockUtil; use Malkusch\Lock\Util\Loop; use Malkusch\Lock\Util\PcntlTimeout; @@ -16,33 +17,30 @@ */ class FlockMutex extends AbstractLockMutex { - public const INFINITE_TIMEOUT = -1.0; - private const STRATEGY_BLOCK = 'block'; - private const STRATEGY_PCNTL = 'pcntl'; - private const STRATEGY_LOOP = 'loop'; /** @var resource */ private $fileHandle; - private float $timeout; + private float $acquireTimeout; /** @var self::STRATEGY_* */ private $strategy; /** * @param resource $fileHandle + * @param float $acquireTimeout In seconds */ - public function __construct($fileHandle, float $timeout = self::INFINITE_TIMEOUT) + public function __construct($fileHandle, float $acquireTimeout = \INF) { if (!is_resource($fileHandle)) { throw new \InvalidArgumentException('The file handle is not a valid resource'); } $this->fileHandle = $fileHandle; - $this->timeout = $timeout; + $this->acquireTimeout = $acquireTimeout; $this->strategy = $this->determineLockingStrategy(); } @@ -51,7 +49,7 @@ public function __construct($fileHandle, float $timeout = self::INFINITE_TIMEOUT */ private function determineLockingStrategy(): string { - if ($this->timeout === self::INFINITE_TIMEOUT) { + if ($this->acquireTimeout > 100 * 365.25 * 24 * 60 * 60) { // 100 years return self::STRATEGY_BLOCK; } @@ -62,9 +60,6 @@ private function determineLockingStrategy(): string return self::STRATEGY_LOOP; } - /** - * @throws LockAcquireException - */ private function lockBlocking(): void { if (!flock($this->fileHandle, \LOCK_EX)) { @@ -72,15 +67,11 @@ private function lockBlocking(): void } } - /** - * @throws LockAcquireException - * @throws LockAcquireTimeoutException - */ private function lockPcntl(): void { - $timeoutInt = (int) ceil($this->timeout); + $acquireTimeoutInt = LockUtil::getInstance()->castFloatToInt(ceil($this->acquireTimeout)); - $timebox = new PcntlTimeout($timeoutInt); + $timebox = new PcntlTimeout($acquireTimeoutInt); try { $timebox->timeBoxed( @@ -89,14 +80,10 @@ function (): void { } ); } catch (DeadlineException $e) { - throw LockAcquireTimeoutException::create($timeoutInt); + throw LockAcquireTimeoutException::create($acquireTimeoutInt); } } - /** - * @throws LockAcquireTimeoutException - * @throws LockAcquireException - */ private function lockBusy(): void { $loop = new Loop(); @@ -105,12 +92,9 @@ private function lockBusy(): void if ($this->acquireNonBlockingLock()) { $loop->end(); } - }, $this->timeout); + }, $this->acquireTimeout); } - /** - * @throws LockAcquireException - */ private function acquireNonBlockingLock(): bool { if (!flock($this->fileHandle, \LOCK_EX | \LOCK_NB, $wouldBlock)) { @@ -125,10 +109,6 @@ private function acquireNonBlockingLock(): bool return true; } - /** - * @throws LockAcquireException - * @throws LockAcquireTimeoutException - */ #[\Override] protected function lock(): void { diff --git a/src/Mutex/MemcachedMutex.php b/src/Mutex/MemcachedMutex.php index adc3807e..6eee2132 100644 --- a/src/Mutex/MemcachedMutex.php +++ b/src/Mutex/MemcachedMutex.php @@ -4,6 +4,8 @@ namespace Malkusch\Lock\Mutex; +use Malkusch\Lock\Util\LockUtil; + /** * Memcached based spinlock implementation. */ @@ -15,12 +17,11 @@ class MemcachedMutex extends AbstractSpinlockMutex * The Memcached API needs to have at least one server in its pool. I.e. * it has to be added with Memcached::addServer(). * - * @param string $name The lock name - * @param float $timeout The timeout in seconds a lock expires + * @param float $acquireTimeout In seconds */ - public function __construct(string $name, \Memcached $memcached, float $timeout = 3) + public function __construct(string $name, \Memcached $memcached, float $acquireTimeout = 3) { - parent::__construct($name, $timeout); + parent::__construct($name, $acquireTimeout); $this->memcached = $memcached; } @@ -30,7 +31,7 @@ protected function acquire(string $key, float $expire): bool { // memcached supports only integer expire // https://github.com/memcached/memcached/wiki/Commands#standard-protocol - $expireInt = (int) ceil($expire); + $expireInt = LockUtil::getInstance()->castFloatToInt(ceil($expire)); return $this->memcached->add($key, true, $expireInt); } diff --git a/src/Mutex/Mutex.php b/src/Mutex/Mutex.php index 631e4256..dbd95978 100644 --- a/src/Mutex/Mutex.php +++ b/src/Mutex/Mutex.php @@ -4,7 +4,6 @@ namespace Malkusch\Lock\Mutex; -use Malkusch\Lock\Exception\ExecutionOutsideLockException; use Malkusch\Lock\Exception\LockAcquireException; use Malkusch\Lock\Exception\LockReleaseException; use Malkusch\Lock\Util\DoubleCheckedLocking; @@ -30,10 +29,9 @@ interface Mutex * * @return T * - * @throws \Exception The execution callback threw an exception - * @throws LockAcquireException The mutex could not be acquired, no further side effects - * @throws LockReleaseException The mutex could not be released, the code was already executed - * @throws ExecutionOutsideLockException Some code has been executed outside of the lock + * @throws \Throwable + * @throws LockAcquireException + * @throws LockReleaseException */ public function synchronized(callable $code); diff --git a/src/Mutex/MySQLMutex.php b/src/Mutex/MySQLMutex.php index 1717139c..81460944 100644 --- a/src/Mutex/MySQLMutex.php +++ b/src/Mutex/MySQLMutex.php @@ -14,12 +14,13 @@ class MySQLMutex extends AbstractLockMutex private string $name; - private float $timeout; + /** In seconds */ + private float $acquireTimeout; /** - * @param float $timeout In seconds + * @param float $acquireTimeout In seconds */ - public function __construct(\PDO $PDO, string $name, float $timeout = 0) + public function __construct(\PDO $PDO, string $name, float $acquireTimeout = 0) { $this->pdo = $PDO; @@ -30,7 +31,7 @@ public function __construct(\PDO $PDO, string $name, float $timeout = 0) } $this->name = $namePrefix . $name; - $this->timeout = $timeout; + $this->acquireTimeout = $acquireTimeout; } #[\Override] @@ -42,11 +43,11 @@ protected function lock(): void // 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); + $acquireTimeoutInt = LockUtil::getInstance()->castFloatToInt(ceil($this->acquireTimeout)); $statement->execute([ $this->name, - $timeoutInt, + $acquireTimeoutInt, ]); $statement->setFetchMode(\PDO::FETCH_NUM); @@ -62,7 +63,7 @@ protected function lock(): void throw new LockAcquireException('An error occurred while acquiring the lock'); } - throw LockAcquireTimeoutException::create($this->timeout); + throw LockAcquireTimeoutException::create($this->acquireTimeout); } #[\Override] diff --git a/src/Mutex/RedisMutex.php b/src/Mutex/RedisMutex.php index 329b3c4b..d27aa988 100644 --- a/src/Mutex/RedisMutex.php +++ b/src/Mutex/RedisMutex.php @@ -6,6 +6,7 @@ use Malkusch\Lock\Exception\LockAcquireException; use Malkusch\Lock\Exception\LockReleaseException; +use Malkusch\Lock\Util\LockUtil; use Predis\ClientInterface as PredisClientInterface; use Predis\PredisException; @@ -40,7 +41,7 @@ private function isClientPHPRedis(object $client): bool #[\Override] protected function add(object $client, string $key, string $value, float $expire): bool { - $expireMillis = (int) ceil($expire * 1000); + $expireMillis = LockUtil::getInstance()->castFloatToInt(ceil($expire * 1000)); if ($this->isClientPHPRedis($client)) { try { diff --git a/src/Mutex/TransactionalMutex.php b/src/Mutex/TransactionalMutex.php index e0221f36..ac46b508 100644 --- a/src/Mutex/TransactionalMutex.php +++ b/src/Mutex/TransactionalMutex.php @@ -129,15 +129,15 @@ private static function hasPDOException(\Throwable $exception): bool /** * Rolls back a transaction. * - * @throws LockAcquireException The roll back failed + * @throws LockAcquireException */ private function rollBack(\Throwable $exception): void { try { $this->pdo->rollBack(); - } catch (\PDOException $e2) { + } catch (\PDOException $e) { throw new LockAcquireException( - 'Could not roll back transaction: ' . $e2->getMessage(), + 'Could not roll back transaction: ' . $e->getMessage(), 0, $exception ); diff --git a/src/Util/DoubleCheckedLocking.php b/src/Util/DoubleCheckedLocking.php index e25001aa..51deabdd 100644 --- a/src/Util/DoubleCheckedLocking.php +++ b/src/Util/DoubleCheckedLocking.php @@ -4,7 +4,6 @@ namespace Malkusch\Lock\Util; -use Malkusch\Lock\Exception\ExecutionOutsideLockException; use Malkusch\Lock\Exception\LockAcquireException; use Malkusch\Lock\Exception\LockReleaseException; use Malkusch\Lock\Mutex\Mutex; @@ -50,10 +49,9 @@ public function __construct(Mutex $mutex, callable $check) * * @return T|false False if check did not pass * - * @throws \Exception The execution callback or the check threw an exception - * @throws LockAcquireException The mutex could not be acquired - * @throws LockReleaseException The mutex could not be released - * @throws ExecutionOutsideLockException Some code has been executed outside of the lock + * @throws \Throwable + * @throws LockAcquireException + * @throws LockReleaseException */ public function then(callable $code) { diff --git a/src/Util/LockUtil.php b/src/Util/LockUtil.php index dcf10f8c..d07add11 100644 --- a/src/Util/LockUtil.php +++ b/src/Util/LockUtil.php @@ -54,6 +54,18 @@ public function makeRandomTemporaryFilePath(string $name = ''): string . $this->makeRandomToken() . '.txt'; } + public function castFloatToInt(float $value): int + { + \assert(!\is_nan($value)); + + // https://github.com/php/php-src/issues/17081 + return ($value > \PHP_INT_MIN && $value < \PHP_INT_MAX) + ? (int) round($value) + : ($value < 0 + ? \PHP_INT_MIN + : \PHP_INT_MAX); + } + /** * @return non-empty-string */ diff --git a/src/Util/Loop.php b/src/Util/Loop.php index c7dbcfb8..ffae548d 100644 --- a/src/Util/Loop.php +++ b/src/Util/Loop.php @@ -13,11 +13,11 @@ */ class Loop { - /** Minimum time that we want to wait, between lock checks. In micro seconds. */ - private const MINIMUM_WAIT_US = 1e4; // 0.01 seconds + /** Minimum time to wait between lock checks. In micro seconds. */ + private const MINIMUM_WAIT_US = 10_000; - /** Maximum time that we want to wait, between lock checks. In micro seconds. */ - private const MAXIMUM_WAIT_US = 5e5; // 0.50 seconds + /** Maximum time to wait between lock checks. In micro seconds. */ + private const MAXIMUM_WAIT_US = 500_000; /** True while code execution is repeating */ private bool $looping = false; @@ -47,8 +47,8 @@ public function end(): void * * @return T * - * @throws \Exception The execution callback threw an exception - * @throws LockAcquireTimeoutException The timeout has been reached + * @throws \Throwable + * @throws LockAcquireTimeoutException */ public function execute(callable $code, float $timeout) { @@ -63,10 +63,10 @@ public function execute(callable $code, float $timeout) $this->looping = true; // At this time, the lock will timeout. - $deadline = microtime(true) + $timeout; + $deadlineTs = microtime(true) + $timeout; $result = null; - for ($i = 0; $this->looping && microtime(true) < $deadline; ++$i) { // @phpstan-ignore booleanAnd.leftAlwaysTrue + for ($i = 0; $this->looping && microtime(true) < $deadlineTs; ++$i) { // @phpstan-ignore booleanAnd.leftAlwaysTrue $result = $code(); if (!$this->looping) { // @phpstan-ignore booleanNot.alwaysFalse // The $code callback has called $this->end() and the lock has been acquired. @@ -75,7 +75,7 @@ public function execute(callable $code, float $timeout) } // Calculate max time remaining, don't sleep any longer than that. - $usecRemaining = (int) (($deadline - microtime(true)) * 1e6); + $usecRemaining = LockUtil::getInstance()->castFloatToInt(($deadlineTs - microtime(true)) * 1e6); // We've ran out of time. if ($usecRemaining <= 0) { @@ -83,7 +83,7 @@ public function execute(callable $code, float $timeout) } $min = min( - (int) self::MINIMUM_WAIT_US * 1.25 ** $i, + self::MINIMUM_WAIT_US * 1.25 ** $i, self::MAXIMUM_WAIT_US ); $max = min($min * 2, self::MAXIMUM_WAIT_US); diff --git a/tests/Mutex/AbstractRedlockMutexTest.php b/tests/Mutex/AbstractRedlockMutexTest.php index bc1004ff..0c3e6628 100644 --- a/tests/Mutex/AbstractRedlockMutexTest.php +++ b/tests/Mutex/AbstractRedlockMutexTest.php @@ -170,8 +170,8 @@ static function () use (&$i, $available): bool { * Tests acquiring keys takes too long. * * @param int $count The total count of servers - * @param float $timeout The timeout in seconds - * @param float $delay The delay in seconds + * @param float $timeout In seconds + * @param float $delay In seconds * * @dataProvider provideAcquireTimeoutsCases */ diff --git a/tests/Util/LockUtilTest.php b/tests/Util/LockUtilTest.php index 12ad18b8..65b36637 100644 --- a/tests/Util/LockUtilTest.php +++ b/tests/Util/LockUtilTest.php @@ -56,6 +56,37 @@ public function testMakeRandomTemporaryFilePath(): void self::assertMatchesRegularExpression('~^' . $pathPrefixRegex . '-fo-o-\w{64}.txt$~', LockUtil::getInstance()->makeRandomTemporaryFilePath('fo:o')); } + /** + * @dataProvider provideCastFloatToIntCases + */ + #[DataProvider('provideCastFloatToIntCases')] + public function testCastFloatToInt(int $expectedResult, float $value): void + { + self::assertSame($expectedResult, LockUtil::getInstance()->castFloatToInt($value)); + } + + /** + * @return iterable> + */ + public static function provideCastFloatToIntCases(): iterable + { + yield [10, 10]; + yield [0, 0]; + yield [0, -0.0]; + yield [0, 0.49]; + yield [0, -0.49]; + yield [1, 0.5]; + yield [-1, -0.5]; + yield [1020304050607080, 1020304050607080]; + yield [-1020304050607080, -1020304050607080]; + yield [\PHP_INT_MAX, \PHP_INT_MAX]; + yield [\PHP_INT_MIN, \PHP_INT_MIN]; + yield [\PHP_INT_MAX, (float) \PHP_INT_MAX + 2000]; + yield [\PHP_INT_MIN, (float) \PHP_INT_MIN - 2000]; + yield [\PHP_INT_MAX, \INF]; + yield [\PHP_INT_MIN, -\INF]; + } + /** * @dataProvider provideFormatTimeoutCases */