From 6553c0227d44d79c3db1840ffbae16116acfe552 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Tue, 8 Sep 2026 12:25:04 -0400 Subject: [PATCH] Fix dogpile cache-overwrite race in CacheHelper::remember() (#91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X --- helpers/CacheHelper.php | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/helpers/CacheHelper.php b/helpers/CacheHelper.php index 40c1c83..4300adf 100644 --- a/helpers/CacheHelper.php +++ b/helpers/CacheHelper.php @@ -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); } }