From 70cdeb91cf94260e011900cc6643756cdda74a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 00:19:43 +0100 Subject: [PATCH 1/6] improve readme --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6f73b48..60318bd 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,7 @@ if (!$newBalance) { } ``` -### Extracting code result after lock release exception +### LockReleaseException::getCode{Exception, Result}() Mutex implementations based on [`Malkush\Lock\Mutex\AbstractLockMutex`][10] will throw [`Malkusch\Lock\Exception\LockReleaseException`][11] in case of lock release @@ -142,17 +142,17 @@ try { return 'result'; }); -} catch (LockReleaseException $unlockException) { - if ($unlockException->getCodeException() !== null) { - $codeException = $unlockException->getCodeException(); +} catch (LockReleaseException $e) { + if ($e->getCodeException() !== null) { + $codeException = $e->getCodeException(); // do something with the code exception } else { - $codeResult = $unlockException->getCodeResult(); + $codeResult = $e->getCodeResult(); // do something with the code result } // deal with LockReleaseException or propagate it - throw $unlockException; + throw $e; } ``` From 483ca5abd8407124461e6da512e553eecbd4fab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 00:22:16 +0100 Subject: [PATCH 2/6] improve distributed dummy key --- src/Mutex/DistributedMutex.php | 3 +++ tests/Mutex/DistributedMutexTest.php | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Mutex/DistributedMutex.php b/src/Mutex/DistributedMutex.php index 4da4b08..991e81f 100644 --- a/src/Mutex/DistributedMutex.php +++ b/src/Mutex/DistributedMutex.php @@ -34,6 +34,9 @@ class DistributedMutex extends AbstractSpinlockWithTokenMutex implements LoggerA public function __construct(array $mutexes, float $acquireTimeout = 3, float $expireTimeout = \INF) { parent::__construct('', $acquireTimeout, $expireTimeout); + \Closure::bind(function () { + $this->key = 'distributed'; + }, $this, AbstractSpinlockMutex::class)(); $this->mutexes = $mutexes; $this->logger = new NullLogger(); diff --git a/tests/Mutex/DistributedMutexTest.php b/tests/Mutex/DistributedMutexTest.php index 97f83a3..be96d90 100644 --- a/tests/Mutex/DistributedMutexTest.php +++ b/tests/Mutex/DistributedMutexTest.php @@ -331,7 +331,7 @@ public function testAcquireMutexLogger(): void $mutex->expects(self::exactly(3)) ->method('acquireMutex') - ->with(self::isInstanceOf(AbstractSpinlockMutex::class), 'php-malkusch-lock:', 1.0, \INF) + ->with(self::isInstanceOf(AbstractSpinlockMutex::class), 'distributed', 1.0, \INF) ->willThrowException($this->createMock(/* PredisException::class */ LockAcquireException::class)); $logger->expects(self::exactly(3)) @@ -357,7 +357,7 @@ public function testReleaseMutexLogger(): void $mutex->expects(self::exactly(3)) ->method('releaseMutex') - ->with(self::isInstanceOf(AbstractSpinlockMutex::class), 'php-malkusch-lock:', \INF) + ->with(self::isInstanceOf(AbstractSpinlockMutex::class), 'distributed', \INF) ->willThrowException($this->createMock(/* PredisException::class */ LockReleaseException::class)); $logger->expects(self::exactly(3)) From 59e1ec075c9ca74893989f161352b378a88a7b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 00:23:03 +0100 Subject: [PATCH 3/6] simplify docs --- src/Exception/LockReleaseException.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Exception/LockReleaseException.php b/src/Exception/LockReleaseException.php index e9d1a27..008e9b0 100644 --- a/src/Exception/LockReleaseException.php +++ b/src/Exception/LockReleaseException.php @@ -9,9 +9,9 @@ * * Take this exception very serious. * - * This exception implies that the critical code was executed, i.e. side effects may have happened. + * This exception implies that the synchronized code was executed, i.e. side effects may have happened. * - * Failing to release a lock might have the potential to introduce deadlocks. + * Failing to release a lock might also introduce deadlock. */ class LockReleaseException extends MutexException { @@ -21,8 +21,6 @@ class LockReleaseException extends MutexException private ?\Throwable $codeException = null; /** - * The return value of the executed critical code. - * * @return mixed */ public function getCodeResult() @@ -42,9 +40,6 @@ public function setCodeResult($codeResult): self return $this; } - /** - * The exception that has happened during the critical code execution. - */ public function getCodeException(): ?\Throwable { return $this->codeException; From 0e997bfd261a71268f6f8b3bffe702fcaded8c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 00:45:55 +0100 Subject: [PATCH 4/6] simplify README --- README.md | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 60318bd..d8f0d08 100644 --- a/README.md +++ b/README.md @@ -115,12 +115,6 @@ $newBalance = $mutex->check(static function () use ($bankAccount, $amount): bool return $balance; }); - -if (!$newBalance) { - if ($balance < 0) { - throw new \DomainException('You have no credit'); - } -} ``` ### LockReleaseException::getCode{Exception, Result}() @@ -134,7 +128,6 @@ In order to read the code result (or an exception thrown there), Example: ```php try { - // OR $mutex->check(...) $result = $mutex->synchronized(static function () { if (someCondition()) { throw new \DomainException(); @@ -144,14 +137,11 @@ try { }); } catch (LockReleaseException $e) { if ($e->getCodeException() !== null) { - $codeException = $e->getCodeException(); - // do something with the code exception + // do something with the $e->getCodeException() exception } else { - $codeResult = $e->getCodeResult(); - // do something with the code result + // do something with the $e->getCodeResult() result } - // deal with LockReleaseException or propagate it throw $e; } ``` From ead2338c9c1ffcb0c8e21741231f898d81d76778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 00:46:37 +0100 Subject: [PATCH 5/6] add fail callback for DoubleCheckedLocking::then() --- src/Util/DoubleCheckedLocking.php | 28 ++++++++------- tests/Util/DoubleCheckedLockingTest.php | 46 +++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/Util/DoubleCheckedLocking.php b/src/Util/DoubleCheckedLocking.php index d45f179..1092257 100644 --- a/src/Util/DoubleCheckedLocking.php +++ b/src/Util/DoubleCheckedLocking.php @@ -24,8 +24,7 @@ class DoubleCheckedLocking private $check; /** - * @param callable(): bool $check Callback that decides if the lock should be acquired and is rechecked - * after a lock has been acquired + * @param callable(): bool $check Decides if a lock should be acquired and is rechecked after the lock has been acquired */ public function __construct(Mutex $mutex, callable $check) { @@ -34,31 +33,36 @@ public function __construct(Mutex $mutex, callable $check) } /** - * Executes a block of code only after the check callback passes - * before and after acquiring a lock. + * Execute a block of code only after the check callback passes before and after acquiring a lock. * - * @template T + * @template TSuccess + * @template TFail = never * - * @param callable(): T $code + * @param callable(): TSuccess $successFx + * @param callable(): TFail $failFx * - * @return T|false False if check did not pass + * @return TSuccess|TFail|false False if check did not pass * * @throws \Throwable * @throws LockAcquireException * @throws LockReleaseException */ - public function then(callable $code) + public function then(callable $successFx, ?callable $failFx = null) { if (!($this->check)()) { - return false; + return $failFx !== null + ? $failFx() + : false; } - return $this->mutex->synchronized(function () use ($code) { + return $this->mutex->synchronized(function () use ($successFx, $failFx) { if (!($this->check)()) { - return false; + return $failFx !== null + ? $failFx() + : false; } - return $code(); + return $successFx(); }); } } diff --git a/tests/Util/DoubleCheckedLockingTest.php b/tests/Util/DoubleCheckedLockingTest.php index 491e9ff..0fa22cd 100644 --- a/tests/Util/DoubleCheckedLockingTest.php +++ b/tests/Util/DoubleCheckedLockingTest.php @@ -129,4 +129,50 @@ public function testCodeExecuted(): void self::assertSame(1, $executedCount); self::assertSame('foo', $result); } + + public function testFailCodeExecutedBeforeLock(): void + { + $this->mutex->expects(self::never()) + ->method('synchronized'); + + $checkedLocking = new DoubleCheckedLocking($this->mutex, static function () { + return false; + }); + + $executedCount = 0; + $result = $checkedLocking->then(static function () { + self::fail(); + }, static function () use (&$executedCount) { + ++$executedCount; + + return 'foo'; + }); + + self::assertSame(1, $executedCount); + self::assertSame('foo', $result); + } + + public function testFailCodeExecutedAfterLock(): void + { + $this->mutex->expects(self::once()) + ->method('synchronized') + ->willReturnCallback(static fn (\Closure $block) => $block()); + + $i = 0; + $checkedLocking = new DoubleCheckedLocking($this->mutex, static function () use (&$i) { + return $i++ === 0; + }); + + $executedCount = 0; + $result = $checkedLocking->then(static function () { + self::fail(); + }, static function () use (&$executedCount) { + ++$executedCount; + + return 'foo'; + }); + + self::assertSame(1, $executedCount); + self::assertSame('foo', $result); + } } From 36d94d3e87a9f7fc37a5adefdcbad13227703daa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 01:02:56 +0100 Subject: [PATCH 6/6] improve type safety and phpdocs --- src/Util/DoubleCheckedLocking.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Util/DoubleCheckedLocking.php b/src/Util/DoubleCheckedLocking.php index 1092257..9f50729 100644 --- a/src/Util/DoubleCheckedLocking.php +++ b/src/Util/DoubleCheckedLocking.php @@ -21,15 +21,20 @@ class DoubleCheckedLocking private Mutex $mutex; /** @var callable(): bool */ - private $check; + private $checkFx; /** - * @param callable(): bool $check Decides if a lock should be acquired and is rechecked after the lock has been acquired + * @param callable(): bool $checkFx Decides if a lock should be acquired and is rechecked after the lock has been acquired */ - public function __construct(Mutex $mutex, callable $check) + public function __construct(Mutex $mutex, callable $checkFx) { $this->mutex = $mutex; - $this->check = $check; + $this->checkFx = $checkFx; + } + + private function invokeCheckFx(): bool + { + return ($this->checkFx)(); } /** @@ -41,7 +46,7 @@ public function __construct(Mutex $mutex, callable $check) * @param callable(): TSuccess $successFx * @param callable(): TFail $failFx * - * @return TSuccess|TFail|false False if check did not pass + * @return TSuccess|($failFx is null ? false : TFail) * * @throws \Throwable * @throws LockAcquireException @@ -49,14 +54,14 @@ public function __construct(Mutex $mutex, callable $check) */ public function then(callable $successFx, ?callable $failFx = null) { - if (!($this->check)()) { + if (!$this->invokeCheckFx()) { return $failFx !== null ? $failFx() : false; } return $this->mutex->synchronized(function () use ($successFx, $failFx) { - if (!($this->check)()) { + if (!$this->invokeCheckFx()) { return $failFx !== null ? $failFx() : false;