Skip to content

DatabaseLock::acquire() with database cache driver mishandles unrelated QueryException errors (e.g. SQLSTATE 22001), causing block() to retry for the full timeout instead of propagating the real failure #60171

@david-windsock

Description

@david-windsock

Laravel Version

13.8

PHP Version

8.4

Database Driver & Version

MySQL 8.0.18 for Windows 11 Pro on x86_64

Description

DatabaseLock::acquire() catches all QueryException instances indiscriminately in its INSERT fallback path, including errors that are not related (e.g. SQLSTATE[22001]: String data, right truncated). This turns permanent, unrecoverable errors into a 30-second retry loop that always ends in LockTimeoutException, masking the real cause.

When the lock INSERT fails, acquire() catches the QueryException and attempts an UPDATE assuming the failure was due to a duplicate key (i.e. the lock already exists). This assumption only holds for SQLSTATE 23xxx (Integrity Constraint Violation). Other SQL errors such as 22001 (value too long for column) are silently swallowed and cause the UPDATE to match zero rows, so acquire() returns false on every attempt until the block timeout expires.

I think DatabaseLock::adquire() must re-throw the exception if it is not an integrity constraint violation (SQLSTATE 23xxx), which is the only class of errors that legitimately indicates locking:

} catch (QueryException $e) {
    // just an example, maybe put this in a trait like `DetectsIntegrityConstraints`
    if (! str_starts_with($e->getSqlState(), '23')) {
        throw $e;
    }

    $updated = $this->connection->table($this->table)
        ->where('key', $this->name)
        ->where(fn ($q) => $q->where('owner', $this->owner)->orWhere('expiration', '<=',
$this->currentTime()))
        ->update(['owner' => $this->owner, 'expiration' => $this->expiresAt()]);

    $acquired = $updated >= 1;
}

I tested it on MySQL only. The fix should work on any SQL-92 compliant engine since SQLSTATE 23xxx is standardised, but I didn't verified this behaviour on other engines.

An alternative approach would be to validate the key length before attempting the INSERT and throw an explicit exception if it exceeds 255 characters, since the cache_locks migration defines the key column as $table->string('key')->primary();. However, this would only address the symptom in this specific table, whereas re-throwing non-23xxx exceptions fixes the underlying assumption in the catch block and protects against any unrecoverable QueryException, regardless of cause. Both fixes are complementary but the exception re-throw is the more robust solution.

This bug was discovered when investigating an infinite 503 refresh loop in a Statamic static cache installation. The lock key exceeded the VARCHAR(255) limit of the cache_locks table, causing repeated SQLSTATE[22001] errors that DatabaseLock::acquire() misidentified, retrying for 30 seconds before throwing LockTimeoutException.

Steps To Reproduce

Quick steps using composer:

composer create-project laravel/laravel laravel-db-cache-lock
cd laravel-db-cache-lock
# configure MySQL database on .env
php artisan migrate

php artisan tinker --execute="
  try {
      Cache::store('database')->lock(str_repeat('a', 260), 30)->block(5, fn() => 'ok');
  } catch (\Illuminate\Contracts\Cache\LockTimeoutException \$e) {
      echo 'LockTimeoutException thrown after 5s' . PHP_EOL;
  }
  "

The last command shows LockTimeoutException thrown after 5s and the expected result is an exception with the message SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'key' at row 1

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions