From a7d51937f0c44f10076ba977edbd0f33a3c3f39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Wed, 5 Aug 2020 19:09:46 +0200 Subject: [PATCH] [RFC][lock] Introduce Shared Lock (or Read/Write Lock) --- src/Symfony/Component/Lock/CHANGELOG.md | 1 + src/Symfony/Component/Lock/Lock.php | 49 +++- .../Component/Lock/SharedLockInterface.php | 34 +++ .../Lock/SharedLockStoreInterface.php | 36 +++ .../Store/BlockingSharedLockStoreTrait.php | 33 +++ .../Component/Lock/Store/CombinedStore.php | 54 ++++- .../Component/Lock/Store/FlockStore.php | 74 ++++-- .../Component/Lock/Store/RedisStore.php | 225 ++++++++++++++++-- .../Tests/Store/AbstractRedisStoreTest.php | 84 +++++++ .../Lock/Tests/Store/AbstractStoreTest.php | 16 ++ .../Lock/Tests/Store/CombinedStoreTest.php | 29 +++ .../Lock/Tests/Store/FlockStoreTest.php | 1 + .../Lock/Tests/Store/RedisStoreTest.php | 2 + .../Tests/Store/SharedLockStoreTestTrait.php | 177 ++++++++++++++ 14 files changed, 768 insertions(+), 47 deletions(-) create mode 100644 src/Symfony/Component/Lock/SharedLockInterface.php create mode 100644 src/Symfony/Component/Lock/SharedLockStoreInterface.php create mode 100644 src/Symfony/Component/Lock/Store/BlockingSharedLockStoreTrait.php create mode 100644 src/Symfony/Component/Lock/Tests/Store/SharedLockStoreTestTrait.php diff --git a/src/Symfony/Component/Lock/CHANGELOG.md b/src/Symfony/Component/Lock/CHANGELOG.md index 8254caf78d..d61ba7f288 100644 --- a/src/Symfony/Component/Lock/CHANGELOG.md +++ b/src/Symfony/Component/Lock/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * `MongoDbStore` does not implement `BlockingStoreInterface` anymore, typehint against `PersistingStoreInterface` instead. + * added support for shared locks 5.1.0 ----- diff --git a/src/Symfony/Component/Lock/Lock.php b/src/Symfony/Component/Lock/Lock.php index 411246d98e..b13456722f 100644 --- a/src/Symfony/Component/Lock/Lock.php +++ b/src/Symfony/Component/Lock/Lock.php @@ -26,7 +26,7 @@ use Symfony\Component\Lock\Exception\NotSupportedException; * * @author Jérémy Derussé */ -final class Lock implements LockInterface, LoggerAwareInterface +final class Lock implements SharedLockInterface, LoggerAwareInterface { use LoggerAwareTrait; @@ -109,6 +109,53 @@ final class Lock implements LockInterface, LoggerAwareInterface } } + /** + * {@inheritdoc} + */ + public function acquireRead(bool $blocking = false): bool + { + try { + if (!$this->store instanceof SharedLockStoreInterface) { + throw new NotSupportedException(sprintf('The store "%s" does not support shared locks.', get_debug_type($this->store))); + } + if ($blocking) { + $this->store->waitAndSaveRead($this->key); + } else { + $this->store->saveRead($this->key); + } + + $this->dirty = true; + $this->logger->debug('Successfully acquired the "{resource}" lock.', ['resource' => $this->key]); + + if ($this->ttl) { + $this->refresh(); + } + + if ($this->key->isExpired()) { + try { + $this->release(); + } catch (\Exception $e) { + // swallow exception to not hide the original issue + } + throw new LockExpiredException(sprintf('Failed to store the "%s" lock.', $this->key)); + } + + return true; + } catch (LockConflictedException $e) { + $this->dirty = false; + $this->logger->info('Failed to acquire the "{resource}" lock. Someone else already acquired the lock.', ['resource' => $this->key]); + + if ($blocking) { + throw $e; + } + + return false; + } catch (\Exception $e) { + $this->logger->notice('Failed to acquire the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]); + throw new LockAcquiringException(sprintf('Failed to acquire the "%s" lock.', $this->key), 0, $e); + } + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Lock/SharedLockInterface.php b/src/Symfony/Component/Lock/SharedLockInterface.php new file mode 100644 index 0000000000..62cedc5cd0 --- /dev/null +++ b/src/Symfony/Component/Lock/SharedLockInterface.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Lock; + +use Symfony\Component\Lock\Exception\LockAcquiringException; +use Symfony\Component\Lock\Exception\LockConflictedException; + +/** + * SharedLockInterface defines an interface to manipulate the status of a shared lock. + * + * @author Jérémy Derussé + */ +interface SharedLockInterface extends LockInterface +{ + /** + * Acquires the lock for reading. If the lock is acquired by someone else in write mode, the parameter `blocking` + * determines whether or not the call should block until the release of the lock. + * + * @return bool whether or not the lock had been acquired + * + * @throws LockConflictedException If the lock is acquired by someone else in blocking mode + * @throws LockAcquiringException If the lock can not be acquired + */ + public function acquireRead(bool $blocking = false); +} diff --git a/src/Symfony/Component/Lock/SharedLockStoreInterface.php b/src/Symfony/Component/Lock/SharedLockStoreInterface.php new file mode 100644 index 0000000000..92cc5d1c30 --- /dev/null +++ b/src/Symfony/Component/Lock/SharedLockStoreInterface.php @@ -0,0 +1,36 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Lock; + +use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Exception\NotSupportedException; + +/** + * @author Jérémy Derussé + */ +interface SharedLockStoreInterface extends PersistingStoreInterface +{ + /** + * Stores the resource if it's not locked for reading by someone else. + * + * @throws NotSupportedException + * @throws LockConflictedException + */ + public function saveRead(Key $key); + + /** + * Waits until a key becomes free for reading, then stores the resource. + * + * @throws LockConflictedException + */ + public function waitAndSaveRead(Key $key); +} diff --git a/src/Symfony/Component/Lock/Store/BlockingSharedLockStoreTrait.php b/src/Symfony/Component/Lock/Store/BlockingSharedLockStoreTrait.php new file mode 100644 index 0000000000..c314871d0f --- /dev/null +++ b/src/Symfony/Component/Lock/Store/BlockingSharedLockStoreTrait.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Lock\Store; + +use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Key; + +trait BlockingSharedLockStoreTrait +{ + abstract public function saveRead(Key $key); + + public function waitAndSaveRead(Key $key) + { + while (true) { + try { + $this->saveRead($key); + + return; + } catch (LockConflictedException $e) { + usleep((100 + random_int(-10, 10)) * 1000); + } + } + } +} diff --git a/src/Symfony/Component/Lock/Store/CombinedStore.php b/src/Symfony/Component/Lock/Store/CombinedStore.php index 35af7bc347..2afc023fe1 100644 --- a/src/Symfony/Component/Lock/Store/CombinedStore.php +++ b/src/Symfony/Component/Lock/Store/CombinedStore.php @@ -16,8 +16,10 @@ use Psr\Log\LoggerAwareTrait; use Psr\Log\NullLogger; use Symfony\Component\Lock\Exception\InvalidArgumentException; use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Exception\NotSupportedException; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\PersistingStoreInterface; +use Symfony\Component\Lock\SharedLockStoreInterface; use Symfony\Component\Lock\Strategy\StrategyInterface; /** @@ -25,8 +27,9 @@ use Symfony\Component\Lock\Strategy\StrategyInterface; * * @author Jérémy Derussé */ -class CombinedStore implements PersistingStoreInterface, LoggerAwareInterface +class CombinedStore implements SharedLockStoreInterface, LoggerAwareInterface { + use BlockingSharedLockStoreTrait; use ExpiringStoreTrait; use LoggerAwareTrait; @@ -34,6 +37,8 @@ class CombinedStore implements PersistingStoreInterface, LoggerAwareInterface private $stores; /** @var StrategyInterface */ private $strategy; + /** @var SharedLockStoreInterface[] */ + private $sharedLockStores; /** * @param PersistingStoreInterface[] $stores The list of synchronized stores @@ -90,6 +95,53 @@ class CombinedStore implements PersistingStoreInterface, LoggerAwareInterface throw new LockConflictedException(); } + public function saveRead(Key $key) + { + if (null === $this->sharedLockStores) { + $this->sharedLockStores = []; + foreach ($this->stores as $store) { + if ($store instanceof SharedLockStoreInterface) { + $this->sharedLockStores[] = $store; + } + } + } + + $successCount = 0; + $storesCount = \count($this->stores); + $failureCount = $storesCount - \count($this->sharedLockStores); + + if (!$this->strategy->canBeMet($failureCount, $storesCount)) { + throw new NotSupportedException(sprintf('The store "%s" does not contains enough compatible store to met the requirements.', get_debug_type($this))); + } + + foreach ($this->sharedLockStores as $store) { + try { + $store->saveRead($key); + ++$successCount; + } catch (\Exception $e) { + $this->logger->debug('One store failed to save the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]); + ++$failureCount; + } + + if (!$this->strategy->canBeMet($failureCount, $storesCount)) { + break; + } + } + + $this->checkNotExpired($key); + + if ($this->strategy->isMet($successCount, $storesCount)) { + return; + } + + $this->logger->info('Failed to store the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]); + + // clean up potential locks + $this->delete($key); + + throw new LockConflictedException(); + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Lock/Store/FlockStore.php b/src/Symfony/Component/Lock/Store/FlockStore.php index 4f2fca8b50..e6320b650a 100644 --- a/src/Symfony/Component/Lock/Store/FlockStore.php +++ b/src/Symfony/Component/Lock/Store/FlockStore.php @@ -16,6 +16,7 @@ use Symfony\Component\Lock\Exception\InvalidArgumentException; use Symfony\Component\Lock\Exception\LockConflictedException; use Symfony\Component\Lock\Exception\LockStorageException; use Symfony\Component\Lock\Key; +use Symfony\Component\Lock\SharedLockStoreInterface; /** * FlockStore is a PersistingStoreInterface implementation using the FileSystem flock. @@ -27,7 +28,7 @@ use Symfony\Component\Lock\Key; * @author Romain Neutron * @author Nicolas Grekas */ -class FlockStore implements BlockingStoreInterface +class FlockStore implements BlockingStoreInterface, SharedLockStoreInterface { private $lockPath; @@ -53,7 +54,15 @@ class FlockStore implements BlockingStoreInterface */ public function save(Key $key) { - $this->lock($key, false); + $this->lock($key, false, false); + } + + /** + * {@inheritdoc} + */ + public function saveRead(Key $key) + { + $this->lock($key, true, false); } /** @@ -61,33 +70,48 @@ class FlockStore implements BlockingStoreInterface */ public function waitAndSave(Key $key) { - $this->lock($key, true); + $this->lock($key, false, true); } - private function lock(Key $key, bool $blocking) + /** + * {@inheritdoc} + */ + public function waitAndSaveRead(Key $key) { + $this->lock($key, true, true); + } + + private function lock(Key $key, bool $read, bool $blocking) + { + $handle = null; // The lock is maybe already acquired. if ($key->hasState(__CLASS__)) { - return; - } - - $fileName = sprintf('%s/sf.%s.%s.lock', - $this->lockPath, - preg_replace('/[^a-z0-9\._-]+/i', '-', $key), - strtr(substr(base64_encode(hash('sha256', $key, true)), 0, 7), '/', '_') - ); - - // Silence error reporting - set_error_handler(function ($type, $msg) use (&$error) { $error = $msg; }); - if (!$handle = fopen($fileName, 'r+') ?: fopen($fileName, 'r')) { - if ($handle = fopen($fileName, 'x')) { - chmod($fileName, 0666); - } elseif (!$handle = fopen($fileName, 'r+') ?: fopen($fileName, 'r')) { - usleep(100); // Give some time for chmod() to complete - $handle = fopen($fileName, 'r+') ?: fopen($fileName, 'r'); + [$stateRead, $handle] = $key->getState(__CLASS__); + // Check for promotion or demotion + if ($stateRead === $read) { + return; } } - restore_error_handler(); + + if (!$handle) { + $fileName = sprintf('%s/sf.%s.%s.lock', + $this->lockPath, + preg_replace('/[^a-z0-9\._-]+/i', '-', $key), + strtr(substr(base64_encode(hash('sha256', $key, true)), 0, 7), '/', '_') + ); + + // Silence error reporting + set_error_handler(function ($type, $msg) use (&$error) { $error = $msg; }); + if (!$handle = fopen($fileName, 'r+') ?: fopen($fileName, 'r')) { + if ($handle = fopen($fileName, 'x')) { + chmod($fileName, 0666); + } elseif (!$handle = fopen($fileName, 'r+') ?: fopen($fileName, 'r')) { + usleep(100); // Give some time for chmod() to complete + $handle = fopen($fileName, 'r+') ?: fopen($fileName, 'r'); + } + } + restore_error_handler(); + } if (!$handle) { throw new LockStorageException($error, 0, null); @@ -95,12 +119,12 @@ class FlockStore implements BlockingStoreInterface // On Windows, even if PHP doc says the contrary, LOCK_NB works, see // https://bugs.php.net/54129 - if (!flock($handle, \LOCK_EX | ($blocking ? 0 : \LOCK_NB))) { + if (!flock($handle, ($read ? \LOCK_SH : \LOCK_EX) | ($blocking ? 0 : \LOCK_NB))) { fclose($handle); throw new LockConflictedException(); } - $key->setState(__CLASS__, $handle); + $key->setState(__CLASS__, [$read, $handle]); } /** @@ -121,7 +145,7 @@ class FlockStore implements BlockingStoreInterface return; } - $handle = $key->getState(__CLASS__); + $handle = $key->getState(__CLASS__)[1]; flock($handle, \LOCK_UN | \LOCK_NB); fclose($handle); diff --git a/src/Symfony/Component/Lock/Store/RedisStore.php b/src/Symfony/Component/Lock/Store/RedisStore.php index 68a1c0a2a4..3d699b2177 100644 --- a/src/Symfony/Component/Lock/Store/RedisStore.php +++ b/src/Symfony/Component/Lock/Store/RedisStore.php @@ -11,25 +11,31 @@ namespace Symfony\Component\Lock\Store; +use Predis\Response\ServerException; use Symfony\Component\Cache\Traits\RedisClusterProxy; use Symfony\Component\Cache\Traits\RedisProxy; use Symfony\Component\Lock\Exception\InvalidArgumentException; use Symfony\Component\Lock\Exception\InvalidTtlException; use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Exception\LockStorageException; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\PersistingStoreInterface; +use Symfony\Component\Lock\SharedLockStoreInterface; /** * RedisStore is a PersistingStoreInterface implementation using Redis as store engine. * * @author Jérémy Derussé + * @author Grégoire Pineau */ -class RedisStore implements PersistingStoreInterface +class RedisStore implements SharedLockStoreInterface { use ExpiringStoreTrait; + use BlockingSharedLockStoreTrait; private $redis; private $initialTtl; + private $supportTime; /** * @param \Redis|\RedisArray|\RedisCluster|RedisProxy|RedisClusterProxy|\Predis\ClientInterface $redisClient @@ -55,17 +61,82 @@ class RedisStore implements PersistingStoreInterface public function save(Key $key) { $script = ' - if redis.call("GET", KEYS[1]) == ARGV[1] then - return redis.call("PEXPIRE", KEYS[1], ARGV[2]) - elseif redis.call("SET", KEYS[1], ARGV[1], "NX", "PX", ARGV[2]) then - return 1 - else - return 0 + local key = KEYS[1] + local uniqueToken = ARGV[2] + local ttl = tonumber(ARGV[3]) + + -- asserts the KEY is compatible with current version (old Symfony <5.2 BC) + if redis.call("TYPE", key).ok == "string" then + return false end + + '.$this->getNowCode().' + + -- Remove expired values + redis.call("ZREMRANGEBYSCORE", key, "-inf", now) + + -- is already acquired + if redis.call("ZSCORE", key, uniqueToken) then + -- is not WRITE lock and cannot be promoted + if not redis.call("ZSCORE", key, "__write__") and redis.call("ZCOUNT", key, "-inf", "+inf") > 1 then + return false + end + elseif redis.call("ZCOUNT", key, "-inf", "+inf") > 0 then + return false + end + + redis.call("ZADD", key, now + ttl, uniqueToken) + redis.call("ZADD", key, now + ttl, "__write__") + + -- Extend the TTL of the key + local maxExpiration = redis.call("ZREVRANGE", key, 0, 0, "WITHSCORES")[2] + redis.call("PEXPIREAT", key, maxExpiration) + + return true '; $key->reduceLifetime($this->initialTtl); - if (!$this->evaluate($script, (string) $key, [$this->getUniqueToken($key), (int) ceil($this->initialTtl * 1000)])) { + if (!$this->evaluate($script, (string) $key, [microtime(true), $this->getUniqueToken($key), (int) ceil($this->initialTtl * 1000)])) { + throw new LockConflictedException(); + } + + $this->checkNotExpired($key); + } + + public function saveRead(Key $key) + { + $script = ' + local key = KEYS[1] + local uniqueToken = ARGV[2] + local ttl = tonumber(ARGV[3]) + + -- asserts the KEY is compatible with current version (old Symfony <5.2 BC) + if redis.call("TYPE", key).ok == "string" then + return false + end + + '.$this->getNowCode().' + + -- Remove expired values + redis.call("ZREMRANGEBYSCORE", key, "-inf", now) + + -- lock not already acquired and a WRITE lock exists? + if not redis.call("ZSCORE", key, uniqueToken) and redis.call("ZSCORE", key, "__write__") then + return false + end + + redis.call("ZADD", key, now + ttl, uniqueToken) + redis.call("ZREM", key, "__write__") + + -- Extend the TTL of the key + local maxExpiration = redis.call("ZREVRANGE", key, 0, 0, "WITHSCORES")[2] + redis.call("PEXPIREAT", key, maxExpiration) + + return true + '; + + $key->reduceLifetime($this->initialTtl); + if (!$this->evaluate($script, (string) $key, [microtime(true), $this->getUniqueToken($key), (int) ceil($this->initialTtl * 1000)])) { throw new LockConflictedException(); } @@ -78,15 +149,37 @@ class RedisStore implements PersistingStoreInterface public function putOffExpiration(Key $key, float $ttl) { $script = ' - if redis.call("GET", KEYS[1]) == ARGV[1] then - return redis.call("PEXPIRE", KEYS[1], ARGV[2]) - else - return 0 + local key = KEYS[1] + local uniqueToken = ARGV[2] + local ttl = tonumber(ARGV[3]) + + -- asserts the KEY is compatible with current version (old Symfony <5.2 BC) + if redis.call("TYPE", key).ok == "string" then + return false end + + '.$this->getNowCode().' + + -- lock already acquired acquired? + if not redis.call("ZSCORE", key, uniqueToken) then + return false + end + + redis.call("ZADD", key, now + ttl, uniqueToken) + -- if the lock is also a WRITE lock, increase the TTL + if redis.call("ZSCORE", key, "__write__") then + redis.call("ZADD", key, now + ttl, "__write__") + end + + -- Extend the TTL of the key + local maxExpiration = redis.call("ZREVRANGE", key, 0, 0, "WITHSCORES")[2] + redis.call("PEXPIREAT", key, maxExpiration) + + return true '; $key->reduceLifetime($ttl); - if (!$this->evaluate($script, (string) $key, [$this->getUniqueToken($key), (int) ceil($ttl * 1000)])) { + if (!$this->evaluate($script, (string) $key, [microtime(true), $this->getUniqueToken($key), (int) ceil($ttl * 1000)])) { throw new LockConflictedException(); } @@ -99,11 +192,28 @@ class RedisStore implements PersistingStoreInterface public function delete(Key $key) { $script = ' - if redis.call("GET", KEYS[1]) == ARGV[1] then - return redis.call("DEL", KEYS[1]) - else - return 0 + local key = KEYS[1] + local uniqueToken = ARGV[1] + + -- asserts the KEY is compatible with current version (old Symfony <5.2 BC) + if redis.call("TYPE", key).ok == "string" then + return false end + + -- lock not already acquired + if not redis.call("ZSCORE", key, uniqueToken) then + return false + end + + redis.call("ZREM", key, uniqueToken) + redis.call("ZREM", key, "__write__") + + local maxExpiration = redis.call("ZREVRANGE", key, 0, 0, "WITHSCORES")[2] + if nil ~= maxExpiration then + redis.call("PEXPIREAT", key, maxExpiration) + end + + return true '; $this->evaluate($script, (string) $key, [$this->getUniqueToken($key)]); @@ -114,7 +224,28 @@ class RedisStore implements PersistingStoreInterface */ public function exists(Key $key) { - return $this->redis->get((string) $key) === $this->getUniqueToken($key); + $script = ' + local key = KEYS[1] + local uniqueToken = ARGV[2] + + -- asserts the KEY is compatible with current version (old Symfony <5.2 BC) + if redis.call("TYPE", key).ok == "string" then + return false + end + + '.$this->getNowCode().' + + -- Remove expired values + redis.call("ZREMRANGEBYSCORE", key, "-inf", now) + + if redis.call("ZSCORE", key, uniqueToken) then + return true + end + + return false + '; + + return (bool) $this->evaluate($script, (string) $key, [microtime(true), $this->getUniqueToken($key)]); } /** @@ -130,15 +261,34 @@ class RedisStore implements PersistingStoreInterface $this->redis instanceof RedisProxy || $this->redis instanceof RedisClusterProxy ) { - return $this->redis->eval($script, array_merge([$resource], $args), 1); + $this->redis->clearLastError(); + $result = $this->redis->eval($script, array_merge([$resource], $args), 1); + if (null !== $err = $this->redis->getLastError()) { + throw new LockStorageException($err); + } + + return $result; } if ($this->redis instanceof \RedisArray) { + $client = $this->redis->_instance($this->redis->_target($resource)); + $client->clearLastError(); + $result = $client->eval($script, array_merge([$resource], $args), 1); + if (null !== $err = $client->getLastError()) { + throw new LockStorageException($err); + } + + return $result; + return $this->redis->_instance($this->redis->_target($resource))->eval($script, array_merge([$resource], $args), 1); } if ($this->redis instanceof \Predis\ClientInterface) { - return $this->redis->eval(...array_merge([$script, 1, $resource], $args)); + try { + return $this->redis->eval(...array_merge([$script, 1, $resource], $args)); + } catch (ServerException $e) { + throw new LockStorageException($e->getMessage(), $e->getCode(), $e); + } } throw new InvalidArgumentException(sprintf('"%s()" expects being initialized with a Redis, RedisArray, RedisCluster or Predis\ClientInterface, "%s" given.', __METHOD__, get_debug_type($this->redis))); @@ -153,4 +303,39 @@ class RedisStore implements PersistingStoreInterface return $key->getState(__CLASS__); } + + private function getNowCode(): string + { + if (null === $this->supportTime) { + // Redis < 5.0 does not support TIME (not deterministic) in script. + // https://redis.io/commands/eval#replicating-commands-instead-of-scripts + // This code asserts TIME can be use, otherwise will fallback to a timestamp generated by the PHP process. + $script = ' + local now = redis.call("TIME") + redis.call("SET", KEYS[1], "1", "PX", 1) + + return 1 + '; + try { + $this->supportTime = 1 === $this->evaluate($script, 'symfony_check_support_time', []); + } catch (LockStorageException $e) { + if (false === strpos($e->getMessage(), 'commands not allowed after non deterministic')) { + throw $e; + } + $this->supportTime = false; + } + } + + if ($this->supportTime) { + return ' + local now = redis.call("TIME") + now = now[1] * 1000 + math.floor(now[2] / 1000) + '; + } + + return ' + local now = tonumber(ARGV[1]) + now = math.floor(now * 1000) + '; + } } diff --git a/src/Symfony/Component/Lock/Tests/Store/AbstractRedisStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/AbstractRedisStoreTest.php index bd744d0c5f..6f3eb47125 100644 --- a/src/Symfony/Component/Lock/Tests/Store/AbstractRedisStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/AbstractRedisStoreTest.php @@ -11,6 +11,11 @@ namespace Symfony\Component\Lock\Tests\Store; +use Symfony\Component\Cache\Traits\RedisClusterProxy; +use Symfony\Component\Cache\Traits\RedisProxy; +use Symfony\Component\Lock\Exception\InvalidArgumentException; +use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Key; use Symfony\Component\Lock\PersistingStoreInterface; use Symfony\Component\Lock\Store\RedisStore; @@ -43,4 +48,83 @@ abstract class AbstractRedisStoreTest extends AbstractStoreTest { return new RedisStore($this->getRedisConnection()); } + + public function testBackwardCompatibility() + { + $resource = uniqid(__METHOD__, true); + $key1 = new Key($resource); + $key2 = new Key($resource); + + $oldStore = new Symfony51Store($this->getRedisConnection()); + $newStore = $this->getStore(); + + $oldStore->save($key1); + $this->assertTrue($oldStore->exists($key1)); + + $this->expectException(LockConflictedException::class); + $newStore->save($key2); + } +} + +class Symfony51Store +{ + private $redis; + + public function __construct($redis) + { + $this->redis = $redis; + } + + public function save(Key $key) + { + $script = ' + if redis.call("GET", KEYS[1]) == ARGV[1] then + return redis.call("PEXPIRE", KEYS[1], ARGV[2]) + elseif redis.call("SET", KEYS[1], ARGV[1], "NX", "PX", ARGV[2]) then + return 1 + else + return 0 + end + '; + if (!$this->evaluate($script, (string) $key, [$this->getUniqueToken($key), (int) ceil(5 * 1000)])) { + throw new LockConflictedException(); + } + } + + public function exists(Key $key) + { + return $this->redis->get((string) $key) === $this->getUniqueToken($key); + } + + private function evaluate(string $script, string $resource, array $args) + { + if ( + $this->redis instanceof \Redis || + $this->redis instanceof \RedisCluster || + $this->redis instanceof RedisProxy || + $this->redis instanceof RedisClusterProxy + ) { + return $this->redis->eval($script, array_merge([$resource], $args), 1); + } + + if ($this->redis instanceof \RedisArray) { + return $this->redis->_instance($this->redis->_target($resource))->eval($script, array_merge([$resource], $args), 1); + } + + if ($this->redis instanceof \Predis\ClientInterface) { + return $this->redis->eval(...array_merge([$script, 1, $resource], $args)); + } + + throw new InvalidArgumentException(sprintf('"%s()" expects being initialized with a Redis, RedisArray, RedisCluster or Predis\ClientInterface, "%s" given.', __METHOD__, get_debug_type($this->redis))); + } + + private function getUniqueToken(Key $key): string + { + if (!$key->hasState(__CLASS__)) { + $token = base64_encode(random_bytes(32)); + $key->setState(__CLASS__, $token); + } + + return $key->getState(__CLASS__); + } } diff --git a/src/Symfony/Component/Lock/Tests/Store/AbstractStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/AbstractStoreTest.php index 2868e6032e..4ac11e00da 100644 --- a/src/Symfony/Component/Lock/Tests/Store/AbstractStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/AbstractStoreTest.php @@ -109,4 +109,20 @@ abstract class AbstractStoreTest extends TestCase $store->delete($key); } + + public function testDeleteIsolated() + { + $store = $this->getStore(); + + $key1 = new Key(uniqid(__METHOD__, true)); + $key2 = new Key(uniqid(__METHOD__, true)); + + $store->save($key1); + $this->assertTrue($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + + $store->delete($key2); + $this->assertTrue($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + } } diff --git a/src/Symfony/Component/Lock/Tests/Store/CombinedStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/CombinedStoreTest.php index c8da617fae..33d04b0f52 100644 --- a/src/Symfony/Component/Lock/Tests/Store/CombinedStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/CombinedStoreTest.php @@ -14,8 +14,10 @@ namespace Symfony\Component\Lock\Tests\Store; use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Lock\BlockingStoreInterface; use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Exception\NotSupportedException; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\PersistingStoreInterface; +use Symfony\Component\Lock\SharedLockStoreInterface; use Symfony\Component\Lock\Store\CombinedStore; use Symfony\Component\Lock\Store\RedisStore; use Symfony\Component\Lock\Strategy\StrategyInterface; @@ -27,6 +29,7 @@ use Symfony\Component\Lock\Strategy\UnanimousStrategy; class CombinedStoreTest extends AbstractStoreTest { use ExpiringStoreTestTrait; + use SharedLockStoreTestTrait; /** * {@inheritdoc} @@ -351,4 +354,30 @@ class CombinedStoreTest extends AbstractStoreTest $this->store->delete($key); } + + public function testSaveReadWithIncompatibleStores() + { + $key = new Key(uniqid(__METHOD__, true)); + + $badStore = $this->createMock(PersistingStoreInterface::class); + + $store = new CombinedStore([$badStore], new UnanimousStrategy()); + $this->expectException(NotSupportedException::class); + + $store->saveRead($key); + } + + public function testSaveReadWithCompatibleStore() + { + $key = new Key(uniqid(__METHOD__, true)); + + $goodStore = $this->createMock(SharedLockStoreInterface::class); + $goodStore->expects($this->once()) + ->method('saveRead') + ->with($key); + + $store = new CombinedStore([$goodStore], new UnanimousStrategy()); + + $store->saveRead($key); + } } diff --git a/src/Symfony/Component/Lock/Tests/Store/FlockStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/FlockStoreTest.php index 1b1b498358..f197a501cd 100644 --- a/src/Symfony/Component/Lock/Tests/Store/FlockStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/FlockStoreTest.php @@ -21,6 +21,7 @@ use Symfony\Component\Lock\Store\FlockStore; class FlockStoreTest extends AbstractStoreTest { use BlockingStoreTestTrait; + use SharedLockStoreTestTrait; /** * {@inheritdoc} diff --git a/src/Symfony/Component/Lock/Tests/Store/RedisStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/RedisStoreTest.php index c4ef09acf5..ed696af03c 100644 --- a/src/Symfony/Component/Lock/Tests/Store/RedisStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/RedisStoreTest.php @@ -21,6 +21,8 @@ use Symfony\Component\Lock\Store\RedisStore; */ class RedisStoreTest extends AbstractRedisStoreTest { + use SharedLockStoreTestTrait; + public static function setUpBeforeClass(): void { try { diff --git a/src/Symfony/Component/Lock/Tests/Store/SharedLockStoreTestTrait.php b/src/Symfony/Component/Lock/Tests/Store/SharedLockStoreTestTrait.php new file mode 100644 index 0000000000..498191beea --- /dev/null +++ b/src/Symfony/Component/Lock/Tests/Store/SharedLockStoreTestTrait.php @@ -0,0 +1,177 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Lock\Tests\Store; + +use Symfony\Component\Lock\Exception\LockConflictedException; +use Symfony\Component\Lock\Key; +use Symfony\Component\Lock\PersistingStoreInterface; + +/** + * @author Jérémy Derussé + */ +trait SharedLockStoreTestTrait +{ + /** + * @see AbstractStoreTest::getStore() + * + * @return PersistingStoreInterface + */ + abstract protected function getStore(); + + public function testSharedLockReadFirst() + { + $store = $this->getStore(); + + $resource = uniqid(__METHOD__, true); + $key1 = new Key($resource); + $key2 = new Key($resource); + $key3 = new Key($resource); + + $store->saveRead($key1); + $this->assertTrue($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + $this->assertFalse($store->exists($key3)); + + // assert we can store multiple keys in read mode + $store->saveRead($key2); + $this->assertTrue($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + $this->assertFalse($store->exists($key3)); + + try { + $store->save($key3); + $this->fail('The store shouldn\'t save the second key'); + } catch (LockConflictedException $e) { + } + + // The failure of previous attempt should not impact the state of current locks + $this->assertTrue($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + $this->assertFalse($store->exists($key3)); + + $store->delete($key1); + $this->assertFalse($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + $this->assertFalse($store->exists($key3)); + + $store->delete($key2); + $this->assertFalse($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + $this->assertFalse($store->exists($key3)); + + $store->save($key3); + $this->assertFalse($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + $this->assertTrue($store->exists($key3)); + + $store->delete($key3); + $this->assertFalse($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + $this->assertFalse($store->exists($key3)); + } + + public function testSharedLockWriteFirst() + { + $store = $this->getStore(); + + $resource = uniqid(__METHOD__, true); + $key1 = new Key($resource); + $key2 = new Key($resource); + + $store->save($key1); + $this->assertTrue($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + + try { + $store->saveRead($key2); + $this->fail('The store shouldn\'t save the second key'); + } catch (LockConflictedException $e) { + } + + // The failure of previous attempt should not impact the state of current locks + $this->assertTrue($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + + $store->delete($key1); + $this->assertFalse($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + + $store->save($key2); + $this->assertFalse($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + + $store->delete($key2); + $this->assertFalse($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + } + + public function testSharedLockPromote() + { + $store = $this->getStore(); + + $resource = uniqid(__METHOD__, true); + $key1 = new Key($resource); + $key2 = new Key($resource); + + $store->saveRead($key1); + $store->saveRead($key2); + $this->assertTrue($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + + try { + $store->save($key1); + $this->fail('The store shouldn\'t save the second key'); + } catch (LockConflictedException $e) { + } + } + + public function testSharedLockPromoteAllowed() + { + $store = $this->getStore(); + + $resource = uniqid(__METHOD__, true); + $key1 = new Key($resource); + $key2 = new Key($resource); + + $store->saveRead($key1); + $store->save($key1); + + try { + $store->saveRead($key2); + $this->fail('The store shouldn\'t save the second key'); + } catch (LockConflictedException $e) { + } + $this->assertTrue($store->exists($key1)); + $this->assertFalse($store->exists($key2)); + + $store->delete($key1); + $store->saveRead($key2); + $this->assertFalse($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + } + + public function testSharedLockDemote() + { + $store = $this->getStore(); + + $resource = uniqid(__METHOD__, true); + $key1 = new Key($resource); + $key2 = new Key($resource); + + $store->save($key1); + $store->saveRead($key1); + $store->saveRead($key2); + + $this->assertTrue($store->exists($key1)); + $this->assertTrue($store->exists($key2)); + } +}