From 7a80e41cd8e9087a897983b66bb15557e721fe18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Mon, 28 Sep 2020 10:48:36 +0200 Subject: [PATCH] Fix non-blocking store fallback --- src/Symfony/Component/Lock/Lock.php | 16 ++- src/Symfony/Component/Lock/Tests/LockTest.php | 136 +++++++++++++++++- 2 files changed, 148 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Lock/Lock.php b/src/Symfony/Component/Lock/Lock.php index ee7cb3bd11a43..078813801681e 100644 --- a/src/Symfony/Component/Lock/Lock.php +++ b/src/Symfony/Component/Lock/Lock.php @@ -71,7 +71,8 @@ public function acquire(bool $blocking = false): bool if (!$this->store instanceof BlockingStoreInterface) { while (true) { try { - $this->store->wait($this->key); + $this->store->save($this->key); + break; } catch (LockConflictedException $e) { usleep((100 + random_int(-10, 10)) * 1000); } @@ -127,7 +128,18 @@ public function acquireRead(bool $blocking = false): bool return $this->acquire($blocking); } if ($blocking) { - $this->store->waitAndSaveRead($this->key); + if (!$this->store instanceof BlockingSharedLockStoreInterface) { + while (true) { + try { + $this->store->saveRead($this->key); + break; + } catch (LockConflictedException $e) { + usleep((100 + random_int(-10, 10)) * 1000); + } + } + } else { + $this->store->waitAndSaveRead($this->key); + } } else { $this->store->saveRead($this->key); } diff --git a/src/Symfony/Component/Lock/Tests/LockTest.php b/src/Symfony/Component/Lock/Tests/LockTest.php index 9ac96f938562a..cc8b96f561ed2 100644 --- a/src/Symfony/Component/Lock/Tests/LockTest.php +++ b/src/Symfony/Component/Lock/Tests/LockTest.php @@ -13,11 +13,13 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +use Symfony\Component\Lock\BlockingSharedLockStoreInterface; use Symfony\Component\Lock\BlockingStoreInterface; use Symfony\Component\Lock\Exception\LockConflictedException; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\Lock; use Symfony\Component\Lock\PersistingStoreInterface; +use Symfony\Component\Lock\SharedLockStoreInterface; /** * @author Jérémy Derussé @@ -40,7 +42,7 @@ public function testAcquireNoBlocking() $this->assertTrue($lock->acquire(false)); } - public function testAcquireNoBlockingStoreInterface() + public function testAcquireNoBlockingWithPersistingStoreInterface() { $key = new Key(uniqid(__METHOD__, true)); $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock(); @@ -56,6 +58,44 @@ public function testAcquireNoBlockingStoreInterface() $this->assertTrue($lock->acquire(false)); } + public function testAcquireBlockingWithPersistingStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock(); + $lock = new Lock($key, $store); + + $store + ->expects($this->once()) + ->method('save'); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquire(true)); + } + + public function testAcquireBlockingRetryWithPersistingStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock(); + $lock = new Lock($key, $store); + + $store + ->expects($this->any()) + ->method('save') + ->willReturnCallback(static function () { + if (1 === random_int(0, 1)) { + return; + } + throw new LockConflictedException('boom'); + }); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquire(true)); + } + public function testAcquireReturnsFalse() { $key = new Key(uniqid(__METHOD__, true)); @@ -90,7 +130,7 @@ public function testAcquireReturnsFalseStoreInterface() $this->assertFalse($lock->acquire(false)); } - public function testAcquireBlocking() + public function testAcquireBlockingWithBlockingStoreInterface() { $key = new Key(uniqid(__METHOD__, true)); $store = $this->createMock(BlockingStoreInterface::class); @@ -372,4 +412,96 @@ public function provideExpiredDates() yield [[0.1], false]; yield [[-0.1, null], false]; } + + public function testAcquireReadNoBlockingWithSharedLockStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->createMock(SharedLockStoreInterface::class); + $lock = new Lock($key, $store); + + $store + ->expects($this->once()) + ->method('saveRead'); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquireRead(false)); + } + + public function testAcquireReadBlockingWithBlockingSharedLockStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->createMock(BlockingSharedLockStoreInterface::class); + $lock = new Lock($key, $store); + + $store + ->expects($this->once()) + ->method('waitAndSaveRead'); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquireRead(true)); + } + + public function testAcquireReadBlockingWithSharedLockStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->createMock(SharedLockStoreInterface::class); + $lock = new Lock($key, $store); + + $store + ->expects($this->any()) + ->method('saveRead') + ->willReturnCallback(static function () { + if (1 === random_int(0, 1)) { + return; + } + throw new LockConflictedException('boom'); + }); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquireRead(true)); + } + + public function testAcquireReadBlockingWithBlockingLockStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->createMock(BlockingStoreInterface::class); + $lock = new Lock($key, $store); + + $store + ->expects($this->once()) + ->method('waitAndSave'); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquireRead(true)); + } + + public function testAcquireReadBlockingWithPersistingStoreInterface() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->createMock(PersistingStoreInterface::class); + $lock = new Lock($key, $store); + + $store + ->expects($this->any()) + ->method('save') + ->willReturnCallback(static function () { + if (1 === random_int(0, 1)) { + return; + } + throw new LockConflictedException('boom'); + }); + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(true, false); + + $this->assertTrue($lock->acquireRead(true)); + } }