Skip to content

Commit 2c6fca7

Browse files
jderussejderusse
committed
Fix expired lock not cleaned
1 parent a8520ae commit 2c6fca7

File tree

6 files changed

+68
-16
lines changed

6 files changed

+68
-16
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ public function acquire($blocking = false)
8383
}
8484

8585
if ($this->key->isExpired()) {
86+
try {
87+
$this->release();
88+
} catch (\Exception $e) {
89+
// swallow exception to not hide the original issue
90+
}
8691
throw new LockExpiredException(sprintf('Failed to store the "%s" lock.', $this->key));
8792
}
8893

@@ -117,6 +122,11 @@ public function refresh()
117122
$this->dirty = true;
118123

119124
if ($this->key->isExpired()) {
125+
try {
126+
$this->release();
127+
} catch (\Exception $e) {
128+
// swallow exception to not hide the original issue
129+
}
120130
throw new LockExpiredException(sprintf('Failed to put off the expiration of the "%s" lock within the specified time.', $this->key));
121131
}
122132

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
class CombinedStore implements StoreInterface, LoggerAwareInterface
3131
{
3232
use LoggerAwareTrait;
33+
use ExpiringStoreTrait;
3334

3435
/** @var StoreInterface[] */
3536
private $stores;
@@ -78,6 +79,8 @@ public function save(Key $key)
7879
}
7980
}
8081

82+
$this->checkNotExpired($key);
83+
8184
if ($this->strategy->isMet($successCount, $storesCount)) {
8285
return;
8386
}
@@ -125,9 +128,7 @@ public function putOffExpiration(Key $key, $ttl)
125128
}
126129
}
127130

128-
if ($key->isExpired()) {
129-
throw new LockExpiredException(sprintf('Failed to put off the expiration of the "%s" lock within the specified time.', $key));
130-
}
131+
$this->checkNotExpired($key);
131132

132133
if ($this->strategy->isMet($successCount, $storesCount)) {
133134
return;

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Symfony\Component\Lock\Store;
4+
5+
use Symfony\Component\Lock\Exception\LockExpiredException;
6+
use Symfony\Component\Lock\Key;
7+
8+
trait ExpiringStoreTrait
9+
{
10+
private function checkNotExpired(Key $key)
11+
{
12+
if ($key->isExpired()) {
13+
try {
14+
$this->delete($key);
15+
} catch (\Exception $e) {
16+
// swallow exception to not hide the original issue
17+
}
18+
throw new LockExpiredException(sprintf('Failed to store the "%s" lock.', $key));
19+
}
20+
}
21+
}

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
*/
2525
class MemcachedStore implements StoreInterface
2626
{
27+
use ExpiringStoreTrait;
28+
2729
private $memcached;
2830
private $initialTtl;
2931
/** @var bool */
@@ -64,9 +66,7 @@ public function save(Key $key)
6466
$this->putOffExpiration($key, $this->initialTtl);
6567
}
6668

67-
if ($key->isExpired()) {
68-
throw new LockExpiredException(sprintf('Failed to store the "%s" lock.', $key));
69-
}
69+
$this->checkNotExpired($key);
7070
}
7171

7272
public function waitAndSave(Key $key)
@@ -110,9 +110,7 @@ public function putOffExpiration(Key $key, $ttl)
110110
throw new LockConflictedException();
111111
}
112112

113-
if ($key->isExpired()) {
114-
throw new LockExpiredException(sprintf('Failed to put off the expiration of the "%s" lock within the specified time.', $key));
115-
}
113+
$this->checkNotExpired($key);
116114
}
117115

118116
/**

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use Symfony\Component\Cache\Traits\RedisProxy;
1515
use Symfony\Component\Lock\Exception\InvalidArgumentException;
1616
use Symfony\Component\Lock\Exception\LockConflictedException;
17-
use Symfony\Component\Lock\Exception\LockExpiredException;
1817
use Symfony\Component\Lock\Key;
1918
use Symfony\Component\Lock\StoreInterface;
2019

@@ -25,6 +24,8 @@
2524
*/
2625
class RedisStore implements StoreInterface
2726
{
27+
use ExpiringStoreTrait;
28+
2829
private $redis;
2930
private $initialTtl;
3031

@@ -66,9 +67,7 @@ public function save(Key $key)
6667
throw new LockConflictedException();
6768
}
6869

69-
if ($key->isExpired()) {
70-
throw new LockExpiredException(sprintf('Failed to store the "%s" lock.', $key));
71-
}
70+
$this->checkNotExpired($key);
7271
}
7372

7473
public function waitAndSave(Key $key)
@@ -94,9 +93,7 @@ public function putOffExpiration(Key $key, $ttl)
9493
throw new LockConflictedException();
9594
}
9695

97-
if ($key->isExpired()) {
98-
throw new LockExpiredException(sprintf('Failed to put off the expiration of the "%s" lock within the specified time.', $key));
99-
}
96+
$this->checkNotExpired($key);
10097
}
10198

10299
/**

‎src/Symfony/Component/Lock/Tests/Store/ExpiringStoreTestTrait.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Lock\Tests\Store;
1313

14+
use Symfony\Component\Lock\Exception\LockExpiredException;
1415
use Symfony\Component\Lock\Key;
1516
use Symfony\Component\Lock\StoreInterface;
1617

@@ -105,4 +106,28 @@ public function testSetExpiration()
105106
$this->assertGreaterThanOrEqual(0, $key->getRemainingLifetime());
106107
$this->assertLessThanOrEqual(1, $key->getRemainingLifetime());
107108
}
109+
110+
public function testExpiredLockCleaned()
111+
{
112+
$resource = uniqid(__METHOD__, true);
113+
114+
$key1 = new Key($resource);
115+
$key2 = new Key($resource);
116+
117+
/** @var StoreInterface $store */
118+
$store = $this->getStore();
119+
$key1->reduceLifetime(0);
120+
121+
$this->assertTrue($key1->isExpired());
122+
try {
123+
$store->save($key1);
124+
$this->fail('The store shouldn\'t have save an expired key');
125+
} catch (LockExpiredException $e) {
126+
}
127+
128+
$this->assertFalse($store->exists($key1));
129+
130+
$store->save($key2);
131+
$this->assertTrue($store->exists($key2));
132+
}
108133
}

0 commit comments

Comments
 (0)