Memcached locks no longer report free locks as locked
Contributed by mrjavadseydi
Part of the framework v13.35.0 release
If you use the Memcached cache driver and rely on atomic locks, this patch fixes a quietly nasty bug: a lock that nobody held could still report isLocked() === true. The root cause is a small impedance mismatch between Laravel's lock contract and the underlying memcached extension, and the fix — shipped in PR #61801 for anyone upgrading from v13.34.0 — normalizes a cache miss to null so lock-state checks behave the way they already do on every other driver. It's a targeted bug fix with no API or configuration changes.
What changed
The entire change lives in one method, MemcachedLock::getCurrentOwner():
// Before
protected function getCurrentOwner()
{
return $this->memcached->get($this->name);
}
// After
protected function getCurrentOwner()
{
$owner = $this->memcached->get($this->name);
return $this->memcached->getResultCode() === \Memcached::RES_SUCCESS ? $owner : null;
}
The Memcached extension's get() returns false when the key doesn't exist — not null. The old implementation passed that false straight up the chain, so anything downstream that asked "is this lock free?" got a non-null answer and concluded the lock was held. The new code consults getResultCode() and returns null on anything other than a successful hit, which is exactly how MemcachedStore::get() already distinguishes misses from stored values. Lock state on Memcached now agrees with lock state on Redis, the file driver, and the rest.
The bug in practice
Because the base Lock::isLocked() implementation checks whether the current owner is !== null, a miss reported as false translated into "locked":
$lock = Cache::store('memcached')->lock('order:42', 10);
$lock->isLocked(); // true, before anyone acquired it
$lock->get();
$lock->release();
$lock->isLocked(); // still true, after release
The same skew affected isOwnedBy(null): with nothing stored under the key, it returned false instead of correctly answering "yes, no one owns this lock." In application code this bites hardest in polling patterns:
// This loop would spin until timeout even when the lock was free
while ($lock->isLocked()) {
usleep(250000);
}
After the fix, a free lock correctly reports isLocked() === false from the first call, and ownership checks against null behave sanely.
Why it matters beyond correctness
Atomic locks are usually load-bearing: order processing, job deduplication, deploy scripts. A lock that claims to be held when it isn't doesn't fail loudly — it just steers your logic down the wrong branch. Code that skips work because a resource "looks busy," or that falls back to a secondary path when contention is detected, may have been taking those paths more often than intended without any exception to tip you off. Fixing this makes contention signals from Memcached trustworthy again.
Upgrade impact
This is a low-risk upgrade. There are no new methods, no signature changes, and no configuration. The visible behavior differences are:
isLocked()now returnsfalsefor a key that doesn't exist (previouslytrue).isOwnedBy(null)now returnstruewhen nothing is stored (previouslyfalse).- Calling
release()when the lock is gone short-circuits cleanly: the new tests pin down that it returnsfalseand never sends adeletecommand to the server.
The only code that could notice a difference is code that depended on the buggy behavior — for example, treating isLocked() as a generic "does this key exist" check. That was never the documented contract, so in practice you should be able to pull this in without touching application code.
Tests that actually run
A curious detail from the PR: an integration test asserting the correct behavior already existed in tests/Integration/Cache/MemcachedCacheLockTestCase.php — but PHPUnit only collects files ending in Test.php, so that file never executed. The author deliberately left it unrenamed, since flipping it on would enable every test in the file, several of which require a live Memcached server. Instead, this PR adds a new suite at tests/Cache/CacheMemcachedLockTest.php that stubs the Memcached extension and covers the four cases that matter: miss returns not-locked, a stored owner returns locked, isOwnedBy(null) is true on a miss, and release() on a miss returns false without calling delete. These run without any infrastructure, so the regression can't silently slip through CI again.
Takeaways
- Fixed:
MemcachedLock::getCurrentOwner()now returnsnullon a cache miss (by checkinggetResultCode()), soisLocked()andisOwnedBy(null)report free locks correctly. - Root cause:
Memcached::get()returnsfalseon a miss, and the base lock logic treats any non-null owner as "locked." - Upgrade: No API or config changes; only code relying on the old incorrect
truefromisLocked()on free locks will notice anything. - Bonus: New unit tests in
CacheMemcachedLockTest.phpcover the miss path without requiring a Memcached server. - Category: Fix — safe to include in your next upgrade from v13.34.0.