[12.x] Ability to refresh cache locks by bytestream · Pull Request #58349 · laravel/framework

bytestream · 2026-01-12T15:18:41Z

This PR adds the ability to increase the duration of an acquired lock atomically for all cache stores.

It's useful for long running operations so that you can set the TTL to a short value and then extend the lock expiration as the operation runs. When using expiring locks, it's possible that lock isn't released due to bugs, segmentation faults, temporary server issues, therefore long expiry times result in further issues.

Symfony has support for this, see https://symfony.com/doc/current/components/lock.html#expiring-locks

Due to bugs, fatal errors or segmentation faults, it cannot be guaranteed that the release() method will be called, which would cause the resource to be locked infinitely.

In case of long-running tasks, it's better to start with a not too long TTL and then use the refresh() method to reset the TTL to its original value

Support for this has been attempted before in #42342 but rejected (possibly due to race conditions?)

Demand for this functionality still exists, see #52778

Copilot

Pull request overview

This PR adds the ability to atomically refresh cache locks across all cache store implementations, allowing long-running operations to extend lock expiration times without releasing and re-acquiring locks. This prevents indefinite locking due to bugs, crashes, or server issues when using short TTLs that get extended as operations progress.

Changes:

Added refresh($seconds = null) method to all lock implementations (Redis, PhpRedis, Memcached, File, Database, DynamoDB, Array, and NoLock)
Added Lua script for atomic refresh operations in Redis
Added integration tests for File, Redis, and Database locks to verify refresh functionality

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/Illuminate/Cache/Lock.php	Added base `refresh()` method that throws RuntimeException by default
src/Illuminate/Cache/RedisLock.php	Implemented atomic refresh using Lua script
src/Illuminate/Cache/PhpRedisLock.php	Implemented atomic refresh using Lua script with PhpRedis argument packing
src/Illuminate/Cache/MemcachedLock.php	Implemented refresh using Memcached CAS for atomicity
src/Illuminate/Cache/FileLock.php	Implemented refresh delegating to FileStore's refreshIfOwned
src/Illuminate/Cache/FileStore.php	Added refreshIfOwned method with file locking for atomicity
src/Illuminate/Cache/DatabaseLock.php	Implemented refresh with database update query
src/Illuminate/Cache/DynamoDbLock.php	Implemented refresh delegating to DynamoDbStore's refreshIfOwned
src/Illuminate/Cache/DynamoDbStore.php	Added refreshIfOwned method with conditional expression for atomicity
src/Illuminate/Cache/ArrayLock.php	Implemented refresh for in-memory lock store
src/Illuminate/Cache/NoLock.php	Implemented no-op refresh that always returns true
src/Illuminate/Cache/LuaScripts.php	Added refreshLock Lua script for atomic Redis operations
tests/Integration/Cache/FileCacheLockTest.php	Added tests for file lock refresh functionality
tests/Integration/Cache/RedisCacheLockTest.php	Added tests for Redis lock refresh functionality
tests/Integration/Database/DatabaseLockTest.php	Added tests for database lock refresh functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-12T15:24:49Z

+    public function refresh($seconds = null)
+    {
+        if (! $this->isOwnedByCurrentProcess()) {
+            return false;
+        }
+
+        $seconds ??= $this->seconds;
+
+        $this->store->locks[$this->name]['expiresAt'] = $seconds === 0 ? null : Carbon::now()->addSeconds($seconds);
+
+        return true;
+    }


The ArrayLock refresh functionality lacks test coverage. While CacheArrayStoreTest has comprehensive tests for lock acquisition and release, there are no tests for the newly added refresh method. Consider adding test cases that verify: (1) successful refresh, (2) refresh preventing acquisition by another process, (3) refresh failure when attempted by another owner, (4) refresh with default seconds parameter, and (5) refresh behavior with expired locks.

Copilot · 2026-01-12T15:24:50Z

+    public function refresh($seconds = null)
+    {
+        return true;
+    }


The NoLock refresh functionality lacks test coverage. While NoLockTest has tests for lock acquisition and release, there are no tests for the newly added refresh method. Since NoLock always succeeds, add a test case to verify that refresh always returns true, maintaining the no-op behavior of this lock implementation.

Copilot · 2026-01-12T15:24:50Z

+    public function testLockCanBeRefreshed()
+    {
+        $lock = Cache::driver('database')->lock('foo', 10);
+        $this->assertTrue($lock->get());
+
+        // Refresh the lock for another 20 seconds
+        $this->assertTrue($lock->refresh(20));
+
+        // Lock should still be held
+        $this->assertFalse(Cache::driver('database')->lock('foo', 10)->get());
+
+        $lock->release();
+    }
+
+    public function testLockCannotBeRefreshedByAnotherOwner()
+    {
+        $firstLock = Cache::driver('database')->lock('foo', 10);
+        $this->assertTrue($firstLock->get());
+
+        // Create a new lock with a different owner
+        $secondLock = Cache::store('database')->restoreLock('foo', 'other_owner');
+
+        // Second lock should not be able to refresh
+        $this->assertFalse($secondLock->refresh(20));
+
+        // Original lock should still be able to refresh
+        $this->assertTrue($firstLock->refresh(20));
+
+        $firstLock->release();
+    }


The DatabaseLockTest is missing a test case for refreshing with default seconds (when no seconds parameter is provided to refresh). The FileCacheLockTest and RedisCacheLockTest both include testLockRefreshWithDefaultSeconds test methods that verify the lock can be refreshed using the original duration. Consider adding this test case to ensure consistent test coverage across all lock implementations.

Copilot · 2026-01-12T15:24:50Z

+    public function refresh($seconds = null)
+    {
+        $seconds ??= $this->seconds;
+
+        $value = $this->memcached->get($this->name, null, \Memcached::GET_EXTENDED);
+
+        if ($value === false || ($value['value'] ?? null) !== $this->owner) {
+            return false;
+        }
+
+        return $this->memcached->cas($value['cas'], $this->name, $this->owner, $seconds);
+    }


The MemcachedLock refresh functionality lacks test coverage. While MemcachedCacheLockTestCase has comprehensive tests for lock acquisition and release, there are no tests for the newly added refresh method. Consider adding test cases similar to those in FileCacheLockTest and RedisCacheLockTest that verify: (1) successful refresh, (2) refresh preventing acquisition by another process, (3) refresh failure when attempted by another owner, and (4) refresh with default seconds parameter.

Copilot · 2026-01-12T15:24:51Z

+    public function refresh($seconds = null)
+    {
+        $seconds ??= $this->seconds;
+
+        if ($seconds <= 0) {
+            $seconds = 86400;
+        }
+
+        return $this->dynamo->refreshIfOwned($this->name, $this->owner, $seconds);
+    }


The DynamoDbLock refresh functionality lacks test coverage. There are no integration tests for DynamoDbLock at all. Consider adding test cases similar to those in FileCacheLockTest, RedisCacheLockTest, and DatabaseLockTest that verify: (1) successful refresh, (2) refresh preventing acquisition by another process, (3) refresh failure when attempted by another owner, and (4) refresh with default seconds parameter. Also verify the special handling of seconds <= 0 defaulting to 86400.

Copilot · 2026-01-12T15:24:51Z

+    public function refresh($seconds = null)
+    {
+        if (! $this->isOwnedByCurrentProcess()) {
+            return false;
+        }
+
+        $seconds ??= $this->seconds;
+
+        $this->store->locks[$this->name]['expiresAt'] = $seconds === 0 ? null : Carbon::now()->addSeconds($seconds);
+
+        return true;
+    }


The refresh operation should verify that the lock hasn't expired before allowing the refresh. Currently, isOwnedByCurrentProcess checks ownership but doesn't validate that the lock's expiration hasn't passed. This means an expired lock could be refreshed by its previous owner. Consider adding expiration validation similar to how the acquire method checks if expiration.isFuture() before preventing acquisition.

Copilot · 2026-01-12T15:24:52Z

+
+        return $this->connection->table($this->table)
+            ->where('key', $this->name)
+            ->where('owner', $this->owner)


The refresh operation should verify that the lock hasn't expired before allowing the refresh. Currently, the update query only checks if the key and owner match, but doesn't validate that the current expiration hasn't passed. This means an expired lock could be refreshed by its previous owner, potentially extending a lock that should have been released. Consider adding a condition to check that the current expiration is greater than the current time, similar to how the acquire method handles expired locks.

Suggested change

->where('owner', $this->owner)

->where('owner', $this->owner)

->where('expiration', '>', $this->currentTime())

Copilot · 2026-01-12T15:24:53Z

+        $this->seconds = $seconds;
+
+        return $this->connection->table($this->table)
+            ->where('key', $this->name)
+            ->where('owner', $this->owner)
+            ->update(['expiration' => $this->expiresAt()]) >= 1;


The refresh operation modifies the instance's $seconds property as a side effect. This state mutation is unexpected for a method that's meant to atomically refresh a lock's expiration. If refresh is called multiple times with different durations, the instance state will change each time. Consider removing this line to avoid side effects, as the refresh operation should only update the lock's expiration in the storage backend without modifying the lock instance's state.

Suggested change

$this->seconds = $seconds;

return $this->connection->table($this->table)

->where('key', $this->name)

->where('owner', $this->owner)

->update(['expiration' => $this->expiresAt()]) >= 1;

$expiration = $this->currentTime() + $seconds;

return $this->connection->table($this->table)

->where('key', $this->name)

->where('owner', $this->owner)

->update(['expiration' => $expiration]) >= 1;

marius-ciclistu · 2026-03-17T11:50:10Z

@bytestream Hi. Why did you close it?

bytestream · 2026-03-17T12:15:39Z

@taylorotwell any thoughts?

marius-ciclistu · 2026-03-17T12:25:15Z

@bytestream touch is not refresh. Touch is for cache, refresh is for locks.

taylorotwell · 2026-05-30T06:38:33Z

Based on AI analysis...

src/Illuminate/Cache/LuaScripts.php:54-55 and src/Illuminate/Cache/RedisLock.php:81-84
refresh() on a Redis lock created with the default seconds = 0 deletes the lock and returns true. Redis EXPIRE key 0 removes the key, while RedisLock::acquire() uses SETNX for seconds <= 0, meaning “no expiration”. This affects both refresh() with no argument on default/restored Redis locks and explicit refresh(0). PhpRedisLock uses the same Lua script, so it has the same issue.
src/Illuminate/Cache/DatabaseLock.php:185-188
DatabaseLock::refresh() does not verify that the lock has not already expired. Since expired database lock rows can remain until pruning or another acquisition, the old owner can refresh an already-expired lock and make it active again. That differs from Redis/Memcached behavior and from the new File/DynamoDB implementations, which only refresh non-expired locks.
src/Illuminate/Cache/ArrayLock.php:114-120
ArrayLock::refresh() also ignores expiration. isOwnedByCurrentProcess() only compares the stored owner and does not account for expiresAt, so an expired array lock can be resurrected by its old owner before another process/test reacquires it.
src/Illuminate/Bus/UniqueLock.php:69-71
UniqueLock::refresh() creates a fresh lock instance without the owner token used when the unique job lock was acquired. For owner-aware drivers, this will normally return false because the new random owner does not match the stored owner, so the new API is effectively unusable for normal unique job locks.

bytestream added 5 commits January 12, 2026 14:42

[12.x] Ability to refresh cache locks

e33a9e0

improve file lock

adbe10e

use cas to prevent race condition in memcache

2ae7655

fix race condition in dynamodb

b188cac

remove from CacheLock (implementation dependent)

b2c53e3

Copilot AI review requested due to automatic review settings January 12, 2026 15:18

Copilot started reviewing on behalf of bytestream January 12, 2026 15:19 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

add to UniqueLock

175576e

bytestream closed this Mar 17, 2026

bytestream reopened this Mar 17, 2026

taylorotwell marked this pull request as draft May 30, 2026 06:38

taylorotwell added 2 commits June 15, 2026 13:32

formatting

b4b25ac

formatting

c90e3bc

taylorotwell marked this pull request as ready for review June 15, 2026 18:49

taylorotwell merged commit f898f56 into laravel:12.x Jun 15, 2026
70 checks passed

	->where('owner', $this->owner)
	->where('owner', $this->owner)
	->where('expiration', '>', $this->currentTime())

Conversation

bytestream commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

marius-ciclistu commented Mar 17, 2026

Uh oh!

bytestream commented Mar 17, 2026

Uh oh!

marius-ciclistu commented Mar 17, 2026

Uh oh!

taylorotwell commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants