diff --git a/src/Exception/ExecutionOutsideLockException.php b/src/Exception/ExecutionOutsideLockException.php index e4bd7bcd..54916eef 100644 --- a/src/Exception/ExecutionOutsideLockException.php +++ b/src/Exception/ExecutionOutsideLockException.php @@ -4,6 +4,7 @@ namespace Malkusch\Lock\Exception; +use Malkusch\Lock\Mutex\AbstractSpinlockMutex; use Malkusch\Lock\Util\LockUtil; /** @@ -14,21 +15,21 @@ * * Should only be used in contexts where the lock is being released. * - * @see \Malkusch\Lock\Mutex\AbstractSpinlockMutex::unlock() + * @see AbstractSpinlockMutex::unlock() */ class ExecutionOutsideLockException extends LockReleaseException { /** - * @param float $elapsedTime Total elapsed time of the synchronized code callback execution - * @param float $timeout The lock timeout in seconds + * @param float $elapsedTime In seconds + * @param float $expireTimeout In seconds */ - public static function create(float $elapsedTime, float $timeout): self + public static function create(float $elapsedTime, float $expireTimeout): self { return new self(\sprintf( - 'The code executed for %s seconds. But the timeout is %s seconds. The last %s seconds were executed outside of the lock.', + 'The code executed for %s seconds. But the expire timeout is %s seconds. The last %s seconds were executed outside of the lock.', LockUtil::getInstance()->formatTimeout($elapsedTime), - LockUtil::getInstance()->formatTimeout($timeout), - LockUtil::getInstance()->formatTimeout(round($elapsedTime, 6) - round($timeout, 6)) + LockUtil::getInstance()->formatTimeout($expireTimeout), + LockUtil::getInstance()->formatTimeout(round($elapsedTime, 6) - round($expireTimeout, 6)) )); } } diff --git a/src/Mutex/AbstractLockMutex.php b/src/Mutex/AbstractLockMutex.php index 745d46c0..e663d2c1 100644 --- a/src/Mutex/AbstractLockMutex.php +++ b/src/Mutex/AbstractLockMutex.php @@ -8,8 +8,6 @@ use Malkusch\Lock\Exception\LockReleaseException; /** - * Locking mutex. - * * @internal */ abstract class AbstractLockMutex extends AbstractMutex diff --git a/src/Mutex/AbstractRedlockMutex.php b/src/Mutex/AbstractRedlockMutex.php index d3b7bdde..e0524ea5 100644 --- a/src/Mutex/AbstractRedlockMutex.php +++ b/src/Mutex/AbstractRedlockMutex.php @@ -18,44 +18,43 @@ * * @see http://redis.io/topics/distlock */ -abstract class AbstractRedlockMutex extends AbstractSpinlockMutex implements LoggerAwareInterface +abstract class AbstractRedlockMutex extends AbstractSpinlockWithTokenMutex implements LoggerAwareInterface { use LoggerAwareTrait; /** @var array */ private array $clients; - private string $token; - /** - * The Redis APIs needs to be connected. I.e. Redis::connect() was + * The Redis instance needs to be connected. I.e. Redis::connect() was * called already. * * @param array $clients * @param float $acquireTimeout In seconds + * @param float $expireTimeout In seconds */ - public function __construct(array $clients, string $name, float $acquireTimeout = 3) + public function __construct(array $clients, string $name, float $acquireTimeout = 3, float $expireTimeout = \INF) { - parent::__construct($name, $acquireTimeout); + parent::__construct($name, $acquireTimeout, $expireTimeout); $this->clients = $clients; $this->logger = new NullLogger(); } #[\Override] - protected function acquire(string $key, float $expire): bool + protected function acquireWithToken(string $key, float $expireTimeout) { // 1. This differs from the specification to avoid an overflow on 32-Bit systems. - $time = microtime(true); + $startTs = microtime(true); // 2. $acquired = 0; $errored = 0; - $this->token = LockUtil::getInstance()->makeRandomToken(); + $token = LockUtil::getInstance()->makeRandomToken(); $exception = null; foreach ($this->clients as $index => $client) { try { - if ($this->add($client, $key, $this->token, $expire)) { + if ($this->add($client, $key, $token, $expireTimeout)) { ++$acquired; } } catch (LockAcquireException $exception) { @@ -63,7 +62,7 @@ protected function acquire(string $key, float $expire): bool $context = [ 'key' => $key, 'index' => $index, - 'token' => $this->token, + 'token' => $token, 'exception' => $exception, ]; $this->logger->warning('Could not set {key} = {token} at server #{index}', $context); @@ -73,16 +72,16 @@ protected function acquire(string $key, float $expire): bool } // 3. - $elapsedTime = microtime(true) - $time; - $isAcquired = $this->isMajority($acquired) && $elapsedTime <= $expire; + $elapsedTime = microtime(true) - $startTs; + $isAcquired = $this->isMajority($acquired) && $elapsedTime <= $expireTimeout; if ($isAcquired) { // 4. - return true; + return $token; } // 5. - $this->release($key); + $this->releaseWithToken($key, $token); // In addition to RedLock it's an exception if too many servers fail. if (!$this->isMajority(count($this->clients) - $errored)) { @@ -99,7 +98,7 @@ protected function acquire(string $key, float $expire): bool } #[\Override] - protected function release(string $key): bool + protected function releaseWithToken(string $key, string $token): bool { /* * All Redis commands must be analyzed before execution to determine which keys the command will operate on. In @@ -117,7 +116,7 @@ protected function release(string $key): bool $released = 0; foreach ($this->clients as $index => $client) { try { - if ($this->evalScript($client, $script, [$key], [$this->token])) { + if ($this->evalScript($client, $script, [$key], [$token])) { ++$released; } } catch (LockReleaseException $e) { @@ -125,7 +124,7 @@ protected function release(string $key): bool $context = [ 'key' => $key, 'index' => $index, - 'token' => $this->token, + 'token' => $token, 'exception' => $e, ]; $this->logger->warning('Could not unset {key} = {token} at server #{index}', $context); diff --git a/src/Mutex/AbstractSpinlockMutex.php b/src/Mutex/AbstractSpinlockMutex.php index 33583b6c..36967ce3 100644 --- a/src/Mutex/AbstractSpinlockMutex.php +++ b/src/Mutex/AbstractSpinlockMutex.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\LockUtil; @@ -12,19 +11,15 @@ /** * Spinlock implementation. - * - * @internal */ abstract class AbstractSpinlockMutex extends AbstractLockMutex { + /** @var non-falsy-string */ private string $key; /** In seconds */ private float $acquireTimeout; - /** The timestamp when the lock was acquired */ - private ?float $acquiredTs = null; - /** * @param float $acquireTimeout In seconds */ @@ -40,16 +35,7 @@ protected function lock(): void $loop = new Loop(); $loop->execute(function () use ($loop): void { - $this->acquiredTs = microtime(true); - - /* - * The expiration timeout for the lock is increased by one second - * to ensure that we delete only our keys. This will prevent the - * case that this key expires before the timeout, and another process - * acquires successfully the same key which would then be deleted - * by this process. - */ - if ($this->acquire($this->key, $this->acquireTimeout + 1)) { + if ($this->acquire($this->key)) { $loop->end(); } }, $this->acquireTimeout); @@ -58,15 +44,6 @@ protected function lock(): void #[\Override] protected function unlock(): void { - $elapsedTime = microtime(true) - $this->acquiredTs; - if ($elapsedTime > $this->acquireTimeout) { - throw ExecutionOutsideLockException::create($elapsedTime, $this->acquireTimeout); - } - - /* - * Worst case would still be one second before the key expires. - * This guarantees that we don't delete a wrong key. - */ if (!$this->release($this->key)) { throw new LockReleaseException('Failed to release the lock'); } @@ -75,17 +52,19 @@ protected function unlock(): void /** * Try to acquire a lock. * - * @param float $expire In seconds + * @param non-falsy-string $key * * @return bool True if the lock was acquired * * @throws LockAcquireException An unexpected error happened */ - abstract protected function acquire(string $key, float $expire): bool; + abstract protected function acquire(string $key): bool; /** * Try to release a lock. * + * @param non-falsy-string $key + * * @return bool True if the lock was released */ abstract protected function release(string $key): bool; diff --git a/src/Mutex/AbstractSpinlockWithTokenMutex.php b/src/Mutex/AbstractSpinlockWithTokenMutex.php new file mode 100644 index 00000000..bdc193c4 --- /dev/null +++ b/src/Mutex/AbstractSpinlockWithTokenMutex.php @@ -0,0 +1,87 @@ +expireTimeout = $expireTimeout; + } + + #[\Override] + protected function acquire(string $key): bool + { + $acquireTs = microtime(true); + + $token = $this->acquireWithToken($key, $this->expireTimeout); + + if ($token === false) { + return false; + } + + $this->acquireTs = $acquireTs; + $this->token = $token; + + return true; + } + + #[\Override] + protected function release(string $key): bool + { + try { + return $this->releaseWithToken($key, $this->token); + } finally { + try { + $elapsedTime = microtime(true) - $this->acquireTs; + if ($elapsedTime >= $this->expireTimeout) { + throw ExecutionOutsideLockException::create($elapsedTime, $this->expireTimeout); + } + } finally { + $this->token = null; + $this->acquireTs = null; + } + } + } + + /** + * Same as self::acquire() but with expire timeout and token. + * + * @param non-falsy-string $key + * @param float $expireTimeout In seconds + * + * @return non-falsy-string|false + */ + abstract protected function acquireWithToken(string $key, float $expireTimeout); + + /** + * Same as self::release() but with expire timeout and token. + * + * @param non-falsy-string $key + * @param non-falsy-string $token + */ + abstract protected function releaseWithToken(string $key, string $token): bool; +} diff --git a/src/Mutex/MemcachedMutex.php b/src/Mutex/MemcachedMutex.php index 6eee2132..aeccff0a 100644 --- a/src/Mutex/MemcachedMutex.php +++ b/src/Mutex/MemcachedMutex.php @@ -9,36 +9,60 @@ /** * Memcached based spinlock implementation. */ -class MemcachedMutex extends AbstractSpinlockMutex +class MemcachedMutex extends AbstractSpinlockWithTokenMutex { private \Memcached $memcached; /** - * The Memcached API needs to have at least one server in its pool. I.e. + * The Memcached instance needs to have at least one server in its pool. I.e. * it has to be added with Memcached::addServer(). * * @param float $acquireTimeout In seconds + * @param float $expireTimeout In seconds */ - public function __construct(string $name, \Memcached $memcached, float $acquireTimeout = 3) + public function __construct(string $name, \Memcached $memcached, float $acquireTimeout = 3, float $expireTimeout = \INF) { - parent::__construct($name, $acquireTimeout); + parent::__construct($name, $acquireTimeout, $expireTimeout); $this->memcached = $memcached; } #[\Override] - protected function acquire(string $key, float $expire): bool + protected function acquireWithToken(string $key, float $expireTimeout) { - // memcached supports only integer expire - // https://github.com/memcached/memcached/wiki/Commands#standard-protocol - $expireInt = LockUtil::getInstance()->castFloatToInt(ceil($expire)); + $token = LockUtil::getInstance()->makeRandomToken(); - return $this->memcached->add($key, true, $expireInt); + return $this->memcached->add($key, $token, $this->makeMemcachedExpireTimeout($expireTimeout)) + ? $token + : false; } #[\Override] - protected function release(string $key): bool + protected function releaseWithToken(string $key, string $token): bool { + // TODO atomic delete only when the remove value matches token + return $this->memcached->delete($key); } + + private function makeMemcachedExpireTimeout(float $value): int + { + $res = LockUtil::getInstance()->castFloatToInt(ceil($value)); + + // workaround https://github.com/memcached/memcached/issues/307 + if ($res > 0 && $res < \PHP_INT_MAX) { + ++$res; + } + + // 0 means no expire + // https://github.com/php/doc-en/blob/af4410a7e1/reference/memcached/expiration.xml#L17 + $res = max(1, $res); + // >= 30 days means TS instead of TTL + // https://github.com/php/doc-en/blob/af4410a7e1/reference/memcached/expiration.xml#L12 + if ($res >= 30 * 24 * 60 * 60) { + $res = 0; + } + + return $res; + } } diff --git a/src/Mutex/RedisMutex.php b/src/Mutex/RedisMutex.php index d27aa988..19caf68d 100644 --- a/src/Mutex/RedisMutex.php +++ b/src/Mutex/RedisMutex.php @@ -35,18 +35,40 @@ private function isClientPHPRedis(object $client): bool return $res; } + private function makeRedisExpireTimeoutMillis(float $value): int + { + $res = LockUtil::getInstance()->castFloatToInt(ceil($value * 1000)); + + // workaround https://github.com/redis/docs/blob/377fb96c09/content/commands/expire/index.md?plain=1#L224 + if ($res > 0 && $res < \PHP_INT_MAX) { + ++$res; + } + + // workaround time + timeout math overflow + if ($res < 0) { + $res = 0; + } elseif (\PHP_INT_SIZE >= 6) { + $thousandYearsMillis = (int) (1000 * 365.25 * 24 * 60 * 60 * 1000); + if ($res > $thousandYearsMillis) { + $res = $thousandYearsMillis; + } + } + + return $res; + } + /** * @throws LockAcquireException */ #[\Override] protected function add(object $client, string $key, string $value, float $expire): bool { - $expireMillis = LockUtil::getInstance()->castFloatToInt(ceil($expire * 1000)); + $expireTimeoutMillis = $this->makeRedisExpireTimeoutMillis($expire); if ($this->isClientPHPRedis($client)) { try { // Will set the key, if it doesn't exist, with a ttl of $expire seconds - return $client->set($key, $value, ['nx', 'px' => $expireMillis]); + return $client->set($key, $value, ['nx', 'px' => $expireTimeoutMillis]); } catch (\RedisException $e) { $message = sprintf( 'Failed to acquire lock for key \'%s\'', @@ -57,7 +79,7 @@ protected function add(object $client, string $key, string $value, float $expire } } else { try { - return $client->set($key, $value, 'PX', $expireMillis, 'NX') !== null; + return $client->set($key, $value, 'PX', $expireTimeoutMillis, 'NX') !== null; } catch (PredisException $e) { $message = sprintf( 'Failed to acquire lock for key \'%s\'', diff --git a/src/Util/PcntlTimeout.php b/src/Util/PcntlTimeout.php index f4a22512..c066ee94 100644 --- a/src/Util/PcntlTimeout.php +++ b/src/Util/PcntlTimeout.php @@ -27,13 +27,11 @@ final class PcntlTimeout public function __construct(int $timeout) { if (!self::isSupported()) { - throw new \RuntimeException('PCNTL module not enabled'); + throw new \RuntimeException('PCNTL extension is required'); } if ($timeout <= 0) { - throw new \InvalidArgumentException( - 'Timeout must be positive and non zero' - ); + throw new \InvalidArgumentException('Timeout must be positive and non zero'); } $this->timeout = $timeout; @@ -94,8 +92,7 @@ public function timeBoxed(callable $code) */ public static function isSupported(): bool { - return - \PHP_SAPI === 'cli' + return \PHP_SAPI === 'cli' && extension_loaded('pcntl') && function_exists('pcntl_alarm') && function_exists('pcntl_signal') diff --git a/tests/Mutex/AbstractRedlockMutexTest.php b/tests/Mutex/AbstractRedlockMutexTest.php index 0c3e6628..807ee1de 100644 --- a/tests/Mutex/AbstractRedlockMutexTest.php +++ b/tests/Mutex/AbstractRedlockMutexTest.php @@ -9,6 +9,7 @@ use Malkusch\Lock\Exception\LockReleaseException; use Malkusch\Lock\Exception\MutexException; use Malkusch\Lock\Mutex\AbstractRedlockMutex; +use Malkusch\Lock\Util\LockUtil; use phpmock\environment\SleepEnvironmentBuilder; use phpmock\MockEnabledException; use phpmock\phpunit\PHPMock; @@ -44,7 +45,7 @@ protected function setUp(): void * * @return AbstractRedlockMutex&MockObject */ - private function createRedlockMutexMock(int $count, float $timeout = 1): AbstractRedlockMutex + private function createRedlockMutexMock(int $count, float $acquireTimeout = 1, float $expireTimeout = \INF): AbstractRedlockMutex { $clients = array_map( static fn ($i) => new class($i) { @@ -59,7 +60,7 @@ public function __construct(int $i) ); return $this->getMockBuilder(AbstractRedlockMutex::class) - ->setConstructorArgs([$clients, 'test', $timeout]) + ->setConstructorArgs([$clients, 'test', $acquireTimeout, $expireTimeout]) ->onlyMethods(['add', 'evalScript']) ->getMock(); } @@ -178,15 +179,13 @@ static function () use (&$i, $available): bool { #[DataProvider('provideAcquireTimeoutsCases')] public function testAcquireTimeouts(int $count, float $timeout, float $delay): void { - $timeoutStr = (string) round($timeout, 6); - if (strpos($timeoutStr, '.') === false) { - $timeoutStr .= '.0'; - } - $this->expectException(LockAcquireTimeoutException::class); - $this->expectExceptionMessage('Lock acquire timeout of ' . $timeoutStr . ' seconds has been exceeded'); + $this->expectExceptionMessage('Lock acquire timeout of ' . LockUtil::getInstance()->formatTimeout($timeout) . ' seconds has been exceeded'); - $mutex = $this->createRedlockMutexMock($count, $timeout); + $mutex = $this->createRedlockMutexMock($count, $timeout, $timeout); + $mutex->expects(self::exactly($count)) + ->method('evalScript') + ->willReturn(true); $mutex->expects(self::exactly($count)) ->method('add') @@ -206,8 +205,8 @@ public function testAcquireTimeouts(int $count, float $timeout, float $delay): v */ public static function provideAcquireTimeoutsCases(): iterable { - yield [1, 1.2 - 1, 1.201]; - yield [2, 1.2 - 1, 1.401]; + yield [1, 1.2, 1.201]; + yield [2, 20.4, 10.201]; } /** diff --git a/tests/Mutex/AbstractSpinlockMutexTest.php b/tests/Mutex/AbstractSpinlockMutexTest.php index c216d1e2..0e04e621 100644 --- a/tests/Mutex/AbstractSpinlockMutexTest.php +++ b/tests/Mutex/AbstractSpinlockMutexTest.php @@ -4,7 +4,6 @@ namespace Malkusch\Lock\Tests\Mutex; -use Malkusch\Lock\Exception\ExecutionOutsideLockException; use Malkusch\Lock\Exception\LockAcquireException; use Malkusch\Lock\Exception\LockAcquireTimeoutException; use Malkusch\Lock\Exception\LockReleaseException; @@ -41,10 +40,10 @@ protected function setUp(): void /** * @return AbstractSpinlockMutex&MockObject */ - private function createSpinlockMutexMock(float $timeout = 3): AbstractSpinlockMutex + private function createSpinlockMutexMock(float $acquireTimeout = 3): AbstractSpinlockMutex { return $this->getMockBuilder(AbstractSpinlockMutex::class) - ->setConstructorArgs(['test', $timeout]) + ->setConstructorArgs(['test', $acquireTimeout]) ->onlyMethods(['acquire', 'release']) ->getMock(); } @@ -85,36 +84,18 @@ public function testAcquireTimeouts(): void } /** - * Tests executing code which exceeds the timeout fails. + * Tests executing code which barely doesn't hit the acquire timeout. */ - public function testExecuteTooLong(): void + public function testExecuteBarelySucceeds(): void { $mutex = $this->createSpinlockMutexMock(0.5); $mutex->expects(self::any()) ->method('acquire') ->willReturn(true); - - $mutex->expects(self::any()) + $mutex->expects(self::once()) ->method('release') ->willReturn(true); - $this->expectException(ExecutionOutsideLockException::class); - $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(static function () { - usleep(501 * 1000); - }); - } - - /** - * Tests executing code which barely doesn't hit the timeout. - */ - public function testExecuteBarelySucceeds(): void - { - $mutex = $this->createSpinlockMutexMock(0.5); - $mutex->expects(self::any())->method('acquire')->willReturn(true); - $mutex->expects(self::once())->method('release')->willReturn(true); - $mutex->synchronized(static function () { usleep(499 * 1000); }); @@ -128,27 +109,13 @@ public function testFailReleasingLock(): void $this->expectException(LockReleaseException::class); $mutex = $this->createSpinlockMutexMock(); - $mutex->expects(self::any())->method('acquire')->willReturn(true); - $mutex->expects(self::any())->method('release')->willReturn(false); - - $mutex->synchronized(static function () {}); - } - - /** - * Tests executing exactly until the timeout will leave the key one more second. - */ - public function testExecuteTimeoutLeavesOneSecondForKeyToExpire(): void - { - $mutex = $this->createSpinlockMutexMock(0.2); - $mutex->expects(self::once()) + $mutex->expects(self::any()) ->method('acquire') - ->with(self::anything(), 1.2) ->willReturn(true); + $mutex->expects(self::any()) + ->method('release') + ->willReturn(false); - $mutex->expects(self::once())->method('release')->willReturn(true); - - $mutex->synchronized(static function () { - usleep(199 * 1000); - }); + $mutex->synchronized(static function () {}); } } diff --git a/tests/Mutex/AbstractSpinlockWithTokenMutexTest.php b/tests/Mutex/AbstractSpinlockWithTokenMutexTest.php new file mode 100644 index 00000000..38810c58 --- /dev/null +++ b/tests/Mutex/AbstractSpinlockWithTokenMutexTest.php @@ -0,0 +1,86 @@ +addNamespace(__NAMESPACE__); + $sleepBuilder->addNamespace('Malkusch\Lock\Mutex'); + $sleepBuilder->addNamespace('Malkusch\Lock\Util'); + $sleep = $sleepBuilder->build(); + try { + $sleep->enable(); + $this->registerForTearDown($sleep); + } catch (MockEnabledException $e) { + // workaround for burn testing + \assert($e->getMessage() === 'microtime is already enabled. Call disable() on the existing mock.'); + } + } + + /** + * @return AbstractSpinlockWithTokenMutex&MockObject + */ + private function createSpinlockWithTokenMutexMock(float $acquireTimeout = 3, float $expireTimeout = \INF): AbstractSpinlockWithTokenMutex + { + return $this->getMockBuilder(AbstractSpinlockWithTokenMutex::class) + ->setConstructorArgs(['test', $acquireTimeout, $expireTimeout]) + ->onlyMethods(['acquireWithToken', 'releaseWithToken']) + ->getMock(); + } + + public function testExecuteExpireTimeout(): void + { + $mutex = $this->createSpinlockWithTokenMutexMock(0.1, 0.2); + $mutex->expects(self::once()) + ->method('acquireWithToken') + ->with(self::anything(), 0.2) + ->willReturn('xx'); + + $mutex->expects(self::once()) + ->method('releaseWithToken') + ->with(self::anything(), 'xx') + ->willReturn(true); + + $mutex->synchronized(static function () { + usleep(199 * 1000); + }); + } + + public function testExecuteTooLong(): void + { + $mutex = $this->createSpinlockWithTokenMutexMock(0.1, 0.2); + $mutex->expects(self::any()) + ->method('acquireWithToken') + ->with(self::anything(), 0.2) + ->willReturn('xx'); + + $mutex->expects(self::any()) + ->method('releaseWithToken') + ->willReturn(true); + + $this->expectException(ExecutionOutsideLockException::class); + $this->expectExceptionMessageMatches('~^The code executed for 0\.2\d+ seconds\. But the expire timeout is 0\.2 seconds. The last 0\.0\d+ seconds were executed outside of the lock\.$~'); + + $mutex->synchronized(static function () { + usleep(201 * 1000); + }); + } +} diff --git a/tests/Mutex/MemcachedMutexTest.php b/tests/Mutex/MemcachedMutexTest.php index e7dbcdf6..c79e1b2c 100644 --- a/tests/Mutex/MemcachedMutexTest.php +++ b/tests/Mutex/MemcachedMutexTest.php @@ -31,19 +31,16 @@ protected function setUp(): void parent::setUp(); $this->memcached = $this->createMock(\Memcached::class); - $this->mutex = new MemcachedMutex('test', $this->memcached, 1); + $this->mutex = new MemcachedMutex('test', $this->memcached, 1, 2); } - /** - * Tests failing to acquire the lock within the timeout. - */ - public function testFailAcquireLock(): void + public function testAcquireFail(): void { $this->expectException(LockAcquireTimeoutException::class); $this->memcached->expects(self::atLeastOnce()) ->method('add') - ->with('php-malkusch-lock:test', true, 2) + ->with('php-malkusch-lock:test', true, 3) ->willReturn(false); $this->mutex->synchronized(static function (): void { @@ -51,16 +48,13 @@ public function testFailAcquireLock(): void }); } - /** - * Tests failing to release a lock. - */ - public function testFailReleasingLock(): void + public function testReleaseFail(): void { $this->expectException(LockReleaseException::class); $this->memcached->expects(self::once()) ->method('add') - ->with('php-malkusch-lock:test', true, 2) + ->with('php-malkusch-lock:test', true, 3) ->willReturn(true); $this->memcached->expects(self::once()) @@ -70,4 +64,21 @@ public function testFailReleasingLock(): void $this->mutex->synchronized(static function (): void {}); } + + public function testAcquireExpireTimeoutLimit(): void + { + $this->mutex = new MemcachedMutex('test', $this->memcached); + + $this->memcached->expects(self::once()) + ->method('add') + ->with('php-malkusch-lock:test', true, 0) + ->willReturn(true); + + $this->memcached->expects(self::once()) + ->method('delete') + ->with('php-malkusch-lock:test') + ->willReturn(true); + + $this->mutex->synchronized(static function (): void {}); + } } diff --git a/tests/Mutex/MutexTest.php b/tests/Mutex/MutexTest.php index 45fccfb4..9ea6ee9c 100644 --- a/tests/Mutex/MutexTest.php +++ b/tests/Mutex/MutexTest.php @@ -86,10 +86,22 @@ public static function provideMutexFactoriesCases(): iterable }]; } + yield 'AbstractLockMutex' => [static function (): Mutex { + $lock = new class extends AbstractLockMutex { + #[\Override] + protected function lock(): void {} + + #[\Override] + protected function unlock(): void {} + }; + + return $lock; + }]; + yield 'AbstractSpinlockMutex' => [static function (): Mutex { $lock = new class('test') extends AbstractSpinlockMutex { #[\Override] - protected function acquire(string $key, float $expire): bool + protected function acquire(string $key): bool { return true; } @@ -104,18 +116,6 @@ protected function release(string $key): bool return $lock; }]; - yield 'AbstractLockMutex' => [static function (): Mutex { - $lock = new class extends AbstractLockMutex { - #[\Override] - protected function lock(): void {} - - #[\Override] - protected function unlock(): void {} - }; - - return $lock; - }]; - if (getenv('MEMCACHE_HOST')) { yield 'MemcachedMutex' => [static function (): Mutex { $memcached = new \Memcached(); diff --git a/tests/Mutex/RedisMutexWithPredisTest.php b/tests/Mutex/RedisMutexWithPredisTest.php index 32c78f5e..a69e509e 100644 --- a/tests/Mutex/RedisMutexWithPredisTest.php +++ b/tests/Mutex/RedisMutexWithPredisTest.php @@ -45,7 +45,7 @@ protected function setUp(): void $this->client = $this->createMock(PredisClientInterfaceWithSetAndEvalMethods::class); - $this->mutex = new RedisMutex([$this->client], 'test', 2.5); + $this->mutex = new RedisMutex([$this->client], 'test', 2.5, 3.5); $this->logger = $this->createMock(LoggerInterface::class); $this->mutex->setLogger($this->logger); @@ -58,7 +58,7 @@ public function testAddFailsToSetKey(): void { $this->client->expects(self::atLeastOnce()) ->method('set') - ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3500, 'NX') + ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3501, 'NX') ->willReturn(null); $this->logger->expects(self::never()) @@ -80,7 +80,7 @@ public function testAddErrors(): void { $this->client->expects(self::atLeastOnce()) ->method('set') - ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3500, 'NX') + ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3501, 'NX') ->willThrowException($this->createMock(PredisException::class)); $this->logger->expects(self::once()) @@ -100,7 +100,7 @@ public function testWorksNormally(): void { $this->client->expects(self::atLeastOnce()) ->method('set') - ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3500, 'NX') + ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3501, 'NX') ->willReturnSelf(); $this->client->expects(self::once()) @@ -117,6 +117,23 @@ public function testWorksNormally(): void self::assertTrue($executed); } + public function testAcquireExpireTimeoutLimit(): void + { + $this->mutex = new RedisMutex([$this->client], 'test'); + + $this->client->expects(self::once()) + ->method('set') + ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 31_557_600_000_000, 'NX') + ->willReturnSelf(); + + $this->client->expects(self::once()) + ->method('eval') + ->with(self::anything(), 1, 'php-malkusch-lock:test', new IsType(IsType::TYPE_STRING)) + ->willReturn(true); + + $this->mutex->synchronized(static function (): void {}); + } + /** * Tests evalScript() fails. */ @@ -124,7 +141,7 @@ public function testEvalScriptFails(): void { $this->client->expects(self::atLeastOnce()) ->method('set') - ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3500, 'NX') + ->with('php-malkusch-lock:test', new IsType(IsType::TYPE_STRING), 'PX', 3501, 'NX') ->willReturnSelf(); $this->client->expects(self::once()) diff --git a/tests/Util/DoubleCheckedLockingTest.php b/tests/Util/DoubleCheckedLockingTest.php index b72786a3..2a848a97 100644 --- a/tests/Util/DoubleCheckedLockingTest.php +++ b/tests/Util/DoubleCheckedLockingTest.php @@ -25,7 +25,8 @@ protected function setUp(): void public function testCheckFailsAcquiresNoLock(): void { - $this->mutex->expects(self::never())->method('synchronized'); + $this->mutex->expects(self::never()) + ->method('synchronized'); $checkedLocking = new DoubleCheckedLocking($this->mutex, static function (): bool { return false; diff --git a/tests/Util/PcntlTimeoutTest.php b/tests/Util/PcntlTimeoutTest.php index b0ce46c2..775c0378 100644 --- a/tests/Util/PcntlTimeoutTest.php +++ b/tests/Util/PcntlTimeoutTest.php @@ -7,11 +7,13 @@ use Malkusch\Lock\Exception\DeadlineException; use Malkusch\Lock\Exception\LockAcquireException; use Malkusch\Lock\Util\PcntlTimeout; +use PHPUnit\Framework\Attributes\RequiresPhpExtension; use PHPUnit\Framework\TestCase; /** - * @requires pcntl + * @requires extension pcntl */ +#[RequiresPhpExtension('pcntl')] class PcntlTimeoutTest extends TestCase { /**