From 110274da94d69587bc360d3443068002b8ebba24 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 21 May 2025 14:18:44 +0200 Subject: [PATCH] [HttpCache] Hit the backend only once after waiting for the cache lock --- .../HttpCache/CacheWasLockedException.php | 7 +++ .../HttpKernel/HttpCache/HttpCache.php | 16 ++--- .../Tests/HttpCache/HttpCacheTest.php | 60 +++++++++++++++++++ .../Tests/HttpCache/HttpCacheTestCase.php | 11 +++- 4 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 src/Symfony/Component/HttpKernel/HttpCache/CacheWasLockedException.php diff --git a/src/Symfony/Component/HttpKernel/HttpCache/CacheWasLockedException.php b/src/Symfony/Component/HttpKernel/HttpCache/CacheWasLockedException.php new file mode 100644 index 0000000000000..6559c55226fa7 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/HttpCache/CacheWasLockedException.php @@ -0,0 +1,7 @@ +record($request, 'reload'); $response = $this->fetch($request, $catch); } else { - $response = $this->lookup($request, $catch); + try { + $response = $this->lookup($request, $catch); + } catch (CacheWasLockedException) { + $response = $this->lookup($request, $catch); + } } $this->restoreResponseBody($request, $response); @@ -576,15 +580,7 @@ protected function lock(Request $request, Response $entry): bool // wait for the lock to be released if ($this->waitForLock($request)) { - // replace the current entry with the fresh one - $new = $this->lookup($request); - $entry->headers = $new->headers; - $entry->setContent($new->getContent()); - $entry->setStatusCode($new->getStatusCode()); - $entry->setProtocolVersion($new->getProtocolVersion()); - foreach ($new->headers->getCookies() as $cookie) { - $entry->headers->setCookie($cookie); - } + throw new CacheWasLockedException(); // unwind back to handle(), try again } else { // backend is slow as hell, send a 503 response (to avoid the dog pile effect) $entry->setStatusCode(503); diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php index a72c08b8723a2..39f00a0139a25 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpKernel\Event\TerminateEvent; use Symfony\Component\HttpKernel\HttpCache\Esi; use Symfony\Component\HttpKernel\HttpCache\HttpCache; +use Symfony\Component\HttpKernel\HttpCache\Store; use Symfony\Component\HttpKernel\HttpCache\StoreInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Kernel; @@ -717,6 +718,7 @@ public function testDegradationWhenCacheLocked() */ sleep(10); + $this->store = $this->createStore(); // create another store instance that does not hold the current lock $this->request('GET', '/'); $this->assertHttpKernelIsNotCalled(); $this->assertEquals(200, $this->response->getStatusCode()); @@ -735,6 +737,64 @@ public function testDegradationWhenCacheLocked() $this->assertEquals('Old response', $this->response->getContent()); } + public function testHitBackendOnlyOnceWhenCacheWasLocked() + { + // Disable stale-while-revalidate, it circumvents waiting for the lock + $this->cacheConfig['stale_while_revalidate'] = 0; + + $this->setNextResponses([ + [ + 'status' => 200, + 'body' => 'initial response', + 'headers' => [ + 'Cache-Control' => 'public, no-cache', + 'Last-Modified' => 'some while ago', + ], + ], + [ + 'status' => 304, + 'body' => '', + 'headers' => [ + 'Cache-Control' => 'public, no-cache', + 'Last-Modified' => 'some while ago', + ], + ], + [ + 'status' => 500, + 'body' => 'The backend should not be called twice during revalidation', + 'headers' => [], + ], + ]); + + $this->request('GET', '/'); // warm the cache + + // Use a store that simulates a cache entry being locked upon first attempt + $this->store = new class(sys_get_temp_dir() . '/http_cache') extends Store { + private bool $hasLock = false; + + public function lock(Request $request): bool + { + $hasLock = $this->hasLock; + $this->hasLock = true; + + return $hasLock; + } + + public function isLocked(Request $request): bool + { + return false; + } + }; + + $this->request('GET', '/'); // hit the cache with simulated lock/concurrency block + + $this->assertEquals(200, $this->response->getStatusCode()); + $this->assertEquals('initial response', $this->response->getContent()); + + $traces = $this->cache->getTraces(); + $this->assertSame(['stale', 'valid', 'store'], current($traces)); + } + public function testHitsCachedResponseWithSMaxAgeDirective() { $time = \DateTimeImmutable::createFromFormat('U', time() - 5); diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php index 26a29f16b2b75..88f6bed56f4cf 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php @@ -30,7 +30,7 @@ abstract class HttpCacheTestCase extends TestCase protected $responses; protected $catch; protected $esi; - protected Store $store; + protected ?Store $store = null; protected function setUp(): void { @@ -115,7 +115,9 @@ public function request($method, $uri = '/', $server = [], $cookies = [], $esi = $this->kernel->reset(); - $this->store = new Store(sys_get_temp_dir().'/http_cache'); + if (! $this->store) { + $this->store = $this->createStore(); + } if (!isset($this->cacheConfig['debug'])) { $this->cacheConfig['debug'] = true; @@ -183,4 +185,9 @@ public static function clearDirectory($directory) closedir($fp); } + + protected function createStore(): Store + { + return new Store(sys_get_temp_dir() . '/http_cache'); + } }