From 3150ff5de589654363abe674ac8dc3dbae5bc9cb Mon Sep 17 00:00:00 2001 From: Paras Malhotra Date: Sat, 7 Nov 2020 03:22:55 +0530 Subject: [PATCH 1/5] [8.x] Add lock support for all cache drivers --- src/Illuminate/Cache/ApcStore.php | 6 +- src/Illuminate/Cache/CacheLock.php | 85 ++++++++++++++ src/Illuminate/Cache/FileStore.php | 5 +- src/Illuminate/Cache/HasCacheLock.php | 31 ++++++ src/Illuminate/Cache/NullStore.php | 6 +- tests/Integration/Cache/FileCacheLockTest.php | 104 ++++++++++++++++++ 6 files changed, 231 insertions(+), 6 deletions(-) create mode 100644 src/Illuminate/Cache/CacheLock.php create mode 100644 src/Illuminate/Cache/HasCacheLock.php create mode 100644 tests/Integration/Cache/FileCacheLockTest.php diff --git a/src/Illuminate/Cache/ApcStore.php b/src/Illuminate/Cache/ApcStore.php index 90132c16f45f..1fcc92e16be5 100755 --- a/src/Illuminate/Cache/ApcStore.php +++ b/src/Illuminate/Cache/ApcStore.php @@ -2,9 +2,11 @@ namespace Illuminate\Cache; -class ApcStore extends TaggableStore +use Illuminate\Contracts\Cache\LockProvider; + +class ApcStore extends TaggableStore implements LockProvider { - use RetrievesMultipleKeys; + use RetrievesMultipleKeys, HasCacheLock; /** * The APC wrapper instance. diff --git a/src/Illuminate/Cache/CacheLock.php b/src/Illuminate/Cache/CacheLock.php new file mode 100644 index 000000000000..bb3202b11e70 --- /dev/null +++ b/src/Illuminate/Cache/CacheLock.php @@ -0,0 +1,85 @@ +store = $store; + } + + /** + * Attempt to acquire the lock. + * + * @return bool + */ + public function acquire() + { + if (method_exists($this->store, 'add') && $this->seconds > 0) { + return $this->store->add( + $this->name, $this->owner, $this->seconds + ); + } + + if (! is_null($this->store->get($this->name))) { + return false; + } + + return ($this->seconds > 0) + ? $this->store->put($this->name, $this->owner, $this->seconds) + : $this->store->forever($this->name, $this->owner, $this->seconds); + } + + /** + * Release the lock. + * + * @return bool + */ + public function release() + { + if ($this->isOwnedByCurrentProcess()) { + return $this->store->forget($this->name); + } + + return false; + } + + /** + * Releases this lock in disregard of ownership. + * + * @return void + */ + public function forceRelease() + { + $this->store->forget($this->name); + } + + /** + * Returns the owner value written into the driver for this lock. + * + * @return mixed + */ + protected function getCurrentOwner() + { + return $this->store->get($this->name); + } +} diff --git a/src/Illuminate/Cache/FileStore.php b/src/Illuminate/Cache/FileStore.php index 7295d9e6d205..4ce8111b125f 100755 --- a/src/Illuminate/Cache/FileStore.php +++ b/src/Illuminate/Cache/FileStore.php @@ -3,13 +3,14 @@ namespace Illuminate\Cache; use Exception; +use Illuminate\Contracts\Cache\LockProvider; use Illuminate\Contracts\Cache\Store; use Illuminate\Filesystem\Filesystem; use Illuminate\Support\InteractsWithTime; -class FileStore implements Store +class FileStore implements Store, LockProvider { - use InteractsWithTime, RetrievesMultipleKeys; + use InteractsWithTime, RetrievesMultipleKeys, HasCacheLock; /** * The Illuminate Filesystem instance. diff --git a/src/Illuminate/Cache/HasCacheLock.php b/src/Illuminate/Cache/HasCacheLock.php new file mode 100644 index 000000000000..82ad9c2b31f5 --- /dev/null +++ b/src/Illuminate/Cache/HasCacheLock.php @@ -0,0 +1,31 @@ +lock($name, 0, $owner); + } +} diff --git a/src/Illuminate/Cache/NullStore.php b/src/Illuminate/Cache/NullStore.php index 43231b492347..96f5863025e5 100755 --- a/src/Illuminate/Cache/NullStore.php +++ b/src/Illuminate/Cache/NullStore.php @@ -2,9 +2,11 @@ namespace Illuminate\Cache; -class NullStore extends TaggableStore +use Illuminate\Contracts\Cache\LockProvider; + +class NullStore extends TaggableStore implements LockProvider { - use RetrievesMultipleKeys; + use RetrievesMultipleKeys, HasCacheLock; /** * Retrieve an item from the cache by key. diff --git a/tests/Integration/Cache/FileCacheLockTest.php b/tests/Integration/Cache/FileCacheLockTest.php new file mode 100644 index 000000000000..58ef8bdeb06b --- /dev/null +++ b/tests/Integration/Cache/FileCacheLockTest.php @@ -0,0 +1,104 @@ +set('cache.default', 'file'); + } + + public function testLocksCanBeAcquiredAndReleased() + { + Cache::lock('foo')->forceRelease(); + + $lock = Cache::lock('foo', 10); + $this->assertTrue($lock->get()); + $this->assertFalse(Cache::lock('foo', 10)->get()); + $lock->release(); + + $lock = Cache::lock('foo', 10); + $this->assertTrue($lock->get()); + $this->assertFalse(Cache::lock('foo', 10)->get()); + Cache::lock('foo')->release(); + } + + public function testLocksCanBlockForSeconds() + { + Carbon::setTestNow(); + + Cache::lock('foo')->forceRelease(); + $this->assertSame('taylor', Cache::lock('foo', 10)->block(1, function () { + return 'taylor'; + })); + + Cache::lock('foo')->forceRelease(); + $this->assertTrue(Cache::lock('foo', 10)->block(1)); + } + + public function testConcurrentLocksAreReleasedSafely() + { + Cache::lock('foo')->forceRelease(); + + $firstLock = Cache::lock('foo', 1); + $this->assertTrue($firstLock->get()); + sleep(2); + + $secondLock = Cache::lock('foo', 10); + $this->assertTrue($secondLock->get()); + + $firstLock->release(); + + $this->assertFalse(Cache::lock('foo')->get()); + } + + public function testLocksWithFailedBlockCallbackAreReleased() + { + Cache::lock('foo')->forceRelease(); + + $firstLock = Cache::lock('foo', 10); + + try { + $firstLock->block(1, function () { + throw new Exception('failed'); + }); + } catch (Exception $e) { + // Not testing the exception, just testing the lock + // is released regardless of the how the exception + // thrown by the callback was handled. + } + + $secondLock = Cache::lock('foo', 1); + + $this->assertTrue($secondLock->get()); + } + + public function testLocksCanBeReleasedUsingOwnerToken() + { + Cache::lock('foo')->forceRelease(); + + $firstLock = Cache::lock('foo', 10); + $this->assertTrue($firstLock->get()); + $owner = $firstLock->owner(); + + $secondLock = Cache::store('file')->restoreLock('foo', $owner); + $secondLock->release(); + + $this->assertTrue(Cache::lock('foo')->get()); + } +} From 50c1a43c71d3d0ec51ab2533111df4e227d0a0d0 Mon Sep 17 00:00:00 2001 From: Paras Malhotra Date: Sat, 7 Nov 2020 13:24:29 +0530 Subject: [PATCH 2/5] Add NoLock for null cache store --- src/Illuminate/Cache/NoLock.php | 46 +++++++++++++++++++++ src/Illuminate/Cache/NullStore.php | 27 ++++++++++++- tests/Integration/Cache/NoLockTest.php | 55 ++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 src/Illuminate/Cache/NoLock.php create mode 100644 tests/Integration/Cache/NoLockTest.php diff --git a/src/Illuminate/Cache/NoLock.php b/src/Illuminate/Cache/NoLock.php new file mode 100644 index 000000000000..68560f8f83d3 --- /dev/null +++ b/src/Illuminate/Cache/NoLock.php @@ -0,0 +1,46 @@ +owner; + } +} diff --git a/src/Illuminate/Cache/NullStore.php b/src/Illuminate/Cache/NullStore.php index 96f5863025e5..d19f0a6f16af 100755 --- a/src/Illuminate/Cache/NullStore.php +++ b/src/Illuminate/Cache/NullStore.php @@ -6,7 +6,7 @@ class NullStore extends TaggableStore implements LockProvider { - use RetrievesMultipleKeys, HasCacheLock; + use RetrievesMultipleKeys; /** * Retrieve an item from the cache by key. @@ -98,4 +98,29 @@ public function getPrefix() { return ''; } + + /** + * Get a lock instance. + * + * @param string $name + * @param int $seconds + * @param string|null $owner + * @return \Illuminate\Contracts\Cache\Lock + */ + public function lock($name, $seconds = 0, $owner = null) + { + return new NoLock($name, $seconds, $owner); + } + + /** + * Restore a lock instance using the owner identifier. + * + * @param string $name + * @param string $owner + * @return \Illuminate\Contracts\Cache\Lock + */ + public function restoreLock($name, $owner) + { + return $this->lock($name, 0, $owner); + } } diff --git a/tests/Integration/Cache/NoLockTest.php b/tests/Integration/Cache/NoLockTest.php new file mode 100644 index 000000000000..485c5926aeae --- /dev/null +++ b/tests/Integration/Cache/NoLockTest.php @@ -0,0 +1,55 @@ +set('cache.default', 'null'); + + $app['config']->set('cache.stores', [ + 'null' => [ + 'driver' => 'null', + ], + ]); + } + + public function testLocksCanAlwaysBeAcquiredAndReleased() + { + Cache::lock('foo')->forceRelease(); + + $lock = Cache::lock('foo', 10); + $this->assertTrue($lock->get()); + $this->assertTrue(Cache::lock('foo', 10)->get()); + $this->assertTrue($lock->release()); + $this->assertTrue($lock->release()); + } + + public function testLocksCanBlockForSeconds() + { + Carbon::setTestNow(); + + Cache::lock('foo')->forceRelease(); + $this->assertSame('taylor', Cache::lock('foo', 10)->block(1, function () { + return 'taylor'; + })); + + Cache::lock('foo')->forceRelease(); + $this->assertTrue(Cache::lock('foo', 10)->block(1)); + } +} From cdcf91b5b114ffe10f0da82035ab9dbe28cf7c42 Mon Sep 17 00:00:00 2001 From: Paras Malhotra Date: Sat, 7 Nov 2020 13:25:44 +0530 Subject: [PATCH 3/5] fix styleci --- tests/Integration/Cache/NoLockTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Integration/Cache/NoLockTest.php b/tests/Integration/Cache/NoLockTest.php index 485c5926aeae..a76caf58c4c4 100644 --- a/tests/Integration/Cache/NoLockTest.php +++ b/tests/Integration/Cache/NoLockTest.php @@ -2,7 +2,6 @@ namespace Illuminate\Tests\Integration\Cache; -use Exception; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; use Orchestra\Testbench\TestCase; From 56cc0ae5b77f9021dd5df29a707e8265ea4bfb9a Mon Sep 17 00:00:00 2001 From: Paras Malhotra Date: Tue, 10 Nov 2020 21:18:45 +0530 Subject: [PATCH 4/5] Create the filestore add method --- src/Illuminate/Cache/FileStore.php | 40 ++++ .../Filesystem/LockTimeoutException.php | 10 + src/Illuminate/Filesystem/File.php | 187 ++++++++++++++++++ src/Illuminate/Filesystem/Filesystem.php | 20 ++ 4 files changed, 257 insertions(+) create mode 100644 src/Illuminate/Contracts/Filesystem/LockTimeoutException.php create mode 100644 src/Illuminate/Filesystem/File.php diff --git a/src/Illuminate/Cache/FileStore.php b/src/Illuminate/Cache/FileStore.php index 4ce8111b125f..2f72d978b630 100755 --- a/src/Illuminate/Cache/FileStore.php +++ b/src/Illuminate/Cache/FileStore.php @@ -5,6 +5,7 @@ use Exception; use Illuminate\Contracts\Cache\LockProvider; use Illuminate\Contracts\Cache\Store; +use Illuminate\Contracts\Filesystem\LockTimeoutException; use Illuminate\Filesystem\Filesystem; use Illuminate\Support\InteractsWithTime; @@ -84,6 +85,45 @@ public function put($key, $value, $seconds) return false; } + /** + * Store an item in the cache if the key doesn't exist. + * + * @param string $key + * @param mixed $value + * @param int $seconds + * @return bool + */ + public function add($key, $value, $seconds) + { + $this->ensureCacheDirectoryExists($path = $this->path($key)); + + $file = $this->files->newFileForReadWrite($path); + + try { + $file->getExclusiveLock(); + } catch (LockTimeoutException $e) { + $file->close(); + + return false; + } + + $expire = $file->read(10); + + if (empty($expire) || $this->currentTime() >= $expire) { + $file->truncate() + ->write($this->expiration($seconds).serialize($value)) + ->close(); + + $this->ensureFileHasCorrectPermissions($path); + + return true; + } + + $file->close(); + + return false; + } + /** * Create the file cache directory if necessary. * diff --git a/src/Illuminate/Contracts/Filesystem/LockTimeoutException.php b/src/Illuminate/Contracts/Filesystem/LockTimeoutException.php new file mode 100644 index 000000000000..f03f5c4e6c1d --- /dev/null +++ b/src/Illuminate/Contracts/Filesystem/LockTimeoutException.php @@ -0,0 +1,10 @@ +files = $files; + $this->path = $path; + + $this->ensureDirectoryExists($path); + $this->createResource($path, $mode); + } + + /** + * Read the file contents. + * + * @param int|null $length + * @return string + */ + public function read($length = null) + { + clearstatcache(true, $this->path); + + return fread($this->handle, $length ?? ($this->size() ?: 1)); + } + + /** + * Write to the file. + * + * @param string $contents + * @return string + */ + public function write($contents) + { + fwrite($this->handle, $contents); + + fflush($this->handle); + + return $this; + } + + /** + * Truncate the file. + * + * @return $this + */ + public function truncate() + { + rewind($this->handle); + + ftruncate($this->handle, 0); + + return $this; + } + + /** + * Get a shared lock on the file. + * + * @return $this + */ + public function getSharedLock($block = false) + { + if (! flock($this->handle, LOCK_SH | ($block ? 0 : LOCK_NB))) { + throw new LockTimeoutException("Unable to acquire file lock at path {$path}."); + } + + $this->isLocked = true; + + return $this; + } + + /** + * Get an exclusive lock on the file. + * + * @return bool + */ + public function getExclusiveLock($block = false) + { + if (! flock($this->handle, LOCK_EX | ($block ? 0 : LOCK_NB))) { + throw new LockTimeoutException("Unable to acquire file lock at path {$path}."); + } + + $this->isLocked = true; + + return $this; + } + + /** + * Release the lock on the file. + * + * @return $this + */ + public function releaseLock() + { + flock($this->handle, LOCK_UN); + + $this->isLocked = false; + + return $this; + } + + /** + * Close the file. + * + * @return bool + */ + public function close() + { + if ($this->isLocked) { + $this->releaseLock(); + } + + return fclose($this->handle); + } + + /** + * Get the file size. + * + * @return int + */ + public function size() + { + return filesize($this->path); + } + + /** + * Create the file resource. + * + * @return void + */ + protected function createResource($path, $mode) + { + $this->handle = @fopen($path, $mode); + } + + /** + * Create the file directory if necessary. + * + * @param string $path + * @return void + */ + protected function ensureDirectoryExists($path) + { + if (! $this->files->exists(dirname($path))) { + $this->files->makeDirectory(dirname($path), 0777, true, true); + } + } +} diff --git a/src/Illuminate/Filesystem/Filesystem.php b/src/Illuminate/Filesystem/Filesystem.php index 2076f4ffe93e..e977a5f63197 100644 --- a/src/Illuminate/Filesystem/Filesystem.php +++ b/src/Illuminate/Filesystem/Filesystem.php @@ -185,6 +185,26 @@ public function put($path, $contents, $lock = false) return file_put_contents($path, $contents, $lock ? LOCK_EX : 0); } + /** + * Create a new File instance for read and write. + * + * @return \Illuminate\Filesystem\File + */ + public function newFileForReadWrite($path) + { + return new File($this, $path, 'c+'); + } + + /** + * Create a new File instance for read and write. + * + * @return \Illuminate\Filesystem\File + */ + public function newFileForRead($path) + { + return new File($this, $path, 'r'); + } + /** * Write the contents of a file, replacing it atomically if it already exists. * From 6cb85b2af91d58d35b64a13df4341991dacc09ae Mon Sep 17 00:00:00 2001 From: Paras Malhotra Date: Tue, 10 Nov 2020 21:24:27 +0530 Subject: [PATCH 5/5] Revert apc lock support --- src/Illuminate/Cache/ApcStore.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Cache/ApcStore.php b/src/Illuminate/Cache/ApcStore.php index 1fcc92e16be5..90132c16f45f 100755 --- a/src/Illuminate/Cache/ApcStore.php +++ b/src/Illuminate/Cache/ApcStore.php @@ -2,11 +2,9 @@ namespace Illuminate\Cache; -use Illuminate\Contracts\Cache\LockProvider; - -class ApcStore extends TaggableStore implements LockProvider +class ApcStore extends TaggableStore { - use RetrievesMultipleKeys, HasCacheLock; + use RetrievesMultipleKeys; /** * The APC wrapper instance.