Fix dogpile cache-overwrite race in CacheHelper::remember() (#91)
remember() had no protection against a slow cache-miss recomputation overwriting a fresher write. If Request A started computing stats just before a ticket mutation + invalidateCache(), and Request B started just after (correctly computing fresh, post-mutation data), A could finish (using stale pre-mutation data) after B and overwrite B's fresh cache entry — extending staleness by up to another full TTL. Added a per-prefix invalidation epoch: delete() bumps it, and remember() snapshots it before running the callback and only writes if the epoch hasn't changed since — otherwise a newer invalidation happened mid-computation and the result being written is already stale, so it's dropped (the caller still gets its own result; only the cache write is skipped). Verified with two real concurrent PHP processes racing against the same cache key (a slow "Request A" callback vs. a fast "Request B" that invalidates then recomputes): the cache ends up holding B's fresh value, not A's late stale overwrite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
This commit is contained in:
+36
-1
@@ -121,6 +121,33 @@ class CacheHelper
|
||||
return $written;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the current invalidation epoch for a prefix (0 if never bumped).
|
||||
* Used by remember() to detect an invalidation that happened while a
|
||||
* cache-miss recomputation was in flight.
|
||||
*/
|
||||
private static function getEpoch(string $prefix): int
|
||||
{
|
||||
$safePrefix = preg_replace('/[^a-zA-Z0-9_]/', '_', $prefix);
|
||||
$file = self::getCacheDir() . '/' . $safePrefix . '.epoch';
|
||||
$val = @file_get_contents($file);
|
||||
return $val !== false ? (int)$val : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bump a prefix's invalidation epoch. Called whenever anything under the
|
||||
* prefix is invalidated.
|
||||
*/
|
||||
private static function bumpEpoch(string $prefix): void
|
||||
{
|
||||
$safePrefix = preg_replace('/[^a-zA-Z0-9_]/', '_', $prefix);
|
||||
$file = self::getCacheDir() . '/' . $safePrefix . '.epoch';
|
||||
$next = self::getEpoch($prefix) + 1;
|
||||
if (@file_put_contents($file, (string)$next, LOCK_EX) !== false) {
|
||||
@chmod($file, 0600);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete cached data
|
||||
*
|
||||
@@ -130,6 +157,8 @@ class CacheHelper
|
||||
*/
|
||||
public static function delete(string $prefix, $identifier = null): bool
|
||||
{
|
||||
self::bumpEpoch($prefix);
|
||||
|
||||
if ($identifier !== null) {
|
||||
$key = self::makeKey($prefix, $identifier);
|
||||
unset(self::$memoryCache[$key]);
|
||||
@@ -192,8 +221,14 @@ class CacheHelper
|
||||
$data = self::get($prefix, $identifier, $ttl);
|
||||
|
||||
if ($data === null) {
|
||||
// Snapshot the epoch before running the (possibly slow) callback so
|
||||
// a concurrent invalidation mid-computation can be detected below —
|
||||
// otherwise this request's stale pre-invalidation result could
|
||||
// overwrite a newer request's fresher write, extending staleness by
|
||||
// up to another full TTL.
|
||||
$epochBefore = self::getEpoch($prefix);
|
||||
$data = $callback();
|
||||
if ($data !== null) {
|
||||
if ($data !== null && self::getEpoch($prefix) === $epochBefore) {
|
||||
self::set($prefix, $identifier, $data);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user