Skip to content

Commit a429dee

Browse files
fabpotfabpot
committed
bug symfony#38329 [lock] Fix non-blocking store fallback (jderusse)
This PR was merged into the 5.2-dev branch. Discussion ---------- [lock] Fix non-blocking store fallback | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | / | License | MIT | Doc PR | / Fix issue introduced in symfony#38328 Tests added to avoid regressions. Commits ------- 7a80e41 Fix non-blocking store fallback
2 parents 88412a1 + 7a80e41 commit a429dee

File tree

2 files changed

+148
-4
lines changed

2 files changed

+148
-4
lines changed

‎src/Symfony/Component/Lock/Lock.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ public function acquire(bool $blocking = false): bool
7171
if (!$this->store instanceof BlockingStoreInterface) {
7272
while (true) {
7373
try {
74-
$this->store->wait($this->key);
74+
$this->store->save($this->key);
75+
break;
7576
} catch (LockConflictedException $e) {
7677
usleep((100 + random_int(-10, 10)) * 1000);
7778
}
@@ -127,7 +128,18 @@ public function acquireRead(bool $blocking = false): bool
127128
return $this->acquire($blocking);
128129
}
129130
if ($blocking) {
130-
$this->store->waitAndSaveRead($this->key);
131+
if (!$this->store instanceof BlockingSharedLockStoreInterface) {
132+
while (true) {
133+
try {
134+
$this->store->saveRead($this->key);
135+
break;
136+
} catch (LockConflictedException $e) {
137+
usleep((100 + random_int(-10, 10)) * 1000);
138+
}
139+
}
140+
} else {
141+
$this->store->waitAndSaveRead($this->key);
142+
}
131143
} else {
132144
$this->store->saveRead($this->key);
133145
}

‎src/Symfony/Component/Lock/Tests/LockTest.php

Lines changed: 134 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Psr\Log\LoggerInterface;
16+
use Symfony\Component\Lock\BlockingSharedLockStoreInterface;
1617
use Symfony\Component\Lock\BlockingStoreInterface;
1718
use Symfony\Component\Lock\Exception\LockConflictedException;
1819
use Symfony\Component\Lock\Key;
1920
use Symfony\Component\Lock\Lock;
2021
use Symfony\Component\Lock\PersistingStoreInterface;
22+
use Symfony\Component\Lock\SharedLockStoreInterface;
2123

2224
/**
2325
* @author Jérémy Derussé <[email protected]>
@@ -40,7 +42,7 @@ public function testAcquireNoBlocking()
4042
$this->assertTrue($lock->acquire(false));
4143
}
4244

43-
public function testAcquireNoBlockingStoreInterface()
45+
public function testAcquireNoBlockingWithPersistingStoreInterface()
4446
{
4547
$key = new Key(uniqid(__METHOD__, true));
4648
$store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
@@ -56,6 +58,44 @@ public function testAcquireNoBlockingStoreInterface()
5658
$this->assertTrue($lock->acquire(false));
5759
}
5860

61+
public function testAcquireBlockingWithPersistingStoreInterface()
62+
{
63+
$key = new Key(uniqid(__METHOD__, true));
64+
$store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
65+
$lock = new Lock($key, $store);
66+
67+
$store
68+
->expects($this->once())
69+
->method('save');
70+
$store
71+
->method('exists')
72+
->willReturnOnConsecutiveCalls(true, false);
73+
74+
$this->assertTrue($lock->acquire(true));
75+
}
76+
77+
public function testAcquireBlockingRetryWithPersistingStoreInterface()
78+
{
79+
$key = new Key(uniqid(__METHOD__, true));
80+
$store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
81+
$lock = new Lock($key, $store);
82+
83+
$store
84+
->expects($this->any())
85+
->method('save')
86+
->willReturnCallback(static function () {
87+
if (1 === random_int(0, 1)) {
88+
return;
89+
}
90+
throw new LockConflictedException('boom');
91+
});
92+
$store
93+
->method('exists')
94+
->willReturnOnConsecutiveCalls(true, false);
95+
96+
$this->assertTrue($lock->acquire(true));
97+
}
98+
5999
public function testAcquireReturnsFalse()
60100
{
61101
$key = new Key(uniqid(__METHOD__, true));
@@ -90,7 +130,7 @@ public function testAcquireReturnsFalseStoreInterface()
90130
$this->assertFalse($lock->acquire(false));
91131
}
92132

93-
public function testAcquireBlocking()
133+
public function testAcquireBlockingWithBlockingStoreInterface()
94134
{
95135
$key = new Key(uniqid(__METHOD__, true));
96136
$store = $this->createMock(BlockingStoreInterface::class);
@@ -372,4 +412,96 @@ public function provideExpiredDates()
372412
yield [[0.1], false];
373413
yield [[-0.1, null], false];
374414
}
415+
416+
public function testAcquireReadNoBlockingWithSharedLockStoreInterface()
417+
{
418+
$key = new Key(uniqid(__METHOD__, true));
419+
$store = $this->createMock(SharedLockStoreInterface::class);
420+
$lock = new Lock($key, $store);
421+
422+
$store
423+
->expects($this->once())
424+
->method('saveRead');
425+
$store
426+
->method('exists')
427+
->willReturnOnConsecutiveCalls(true, false);
428+
429+
$this->assertTrue($lock->acquireRead(false));
430+
}
431+
432+
public function testAcquireReadBlockingWithBlockingSharedLockStoreInterface()
433+
{
434+
$key = new Key(uniqid(__METHOD__, true));
435+
$store = $this->createMock(BlockingSharedLockStoreInterface::class);
436+
$lock = new Lock($key, $store);
437+
438+
$store
439+
->expects($this->once())
440+
->method('waitAndSaveRead');
441+
$store
442+
->method('exists')
443+
->willReturnOnConsecutiveCalls(true, false);
444+
445+
$this->assertTrue($lock->acquireRead(true));
446+
}
447+
448+
public function testAcquireReadBlockingWithSharedLockStoreInterface()
449+
{
450+
$key = new Key(uniqid(__METHOD__, true));
451+
$store = $this->createMock(SharedLockStoreInterface::class);
452+
$lock = new Lock($key, $store);
453+
454+
$store
455+
->expects($this->any())
456+
->method('saveRead')
457+
->willReturnCallback(static function () {
458+
if (1 === random_int(0, 1)) {
459+
return;
460+
}
461+
throw new LockConflictedException('boom');
462+
});
463+
$store
464+
->method('exists')
465+
->willReturnOnConsecutiveCalls(true, false);
466+
467+
$this->assertTrue($lock->acquireRead(true));
468+
}
469+
470+
public function testAcquireReadBlockingWithBlockingLockStoreInterface()
471+
{
472+
$key = new Key(uniqid(__METHOD__, true));
473+
$store = $this->createMock(BlockingStoreInterface::class);
474+
$lock = new Lock($key, $store);
475+
476+
$store
477+
->expects($this->once())
478+
->method('waitAndSave');
479+
$store
480+
->method('exists')
481+
->willReturnOnConsecutiveCalls(true, false);
482+
483+
$this->assertTrue($lock->acquireRead(true));
484+
}
485+
486+
public function testAcquireReadBlockingWithPersistingStoreInterface()
487+
{
488+
$key = new Key(uniqid(__METHOD__, true));
489+
$store = $this->createMock(PersistingStoreInterface::class);
490+
$lock = new Lock($key, $store);
491+
492+
$store
493+
->expects($this->any())
494+
->method('save')
495+
->willReturnCallback(static function () {
496+
if (1 === random_int(0, 1)) {
497+
return;
498+
}
499+
throw new LockConflictedException('boom');
500+
});
501+
$store
502+
->method('exists')
503+
->willReturnOnConsecutiveCalls(true, false);
504+
505+
$this->assertTrue($lock->acquireRead(true));
506+
}
375507
}

0 commit comments

Comments
 (0)