From 2668edd05bbcd60e1146dbaa0ad5ba9498b32ba7 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 6 Jul 2016 14:07:04 +0200 Subject: [PATCH] [HttpKernel] Use flock() for HttpCache's lock files --- .../Component/HttpKernel/HttpCache/Store.php | 133 +++++++++++------- .../Tests/HttpCache/HttpCacheTestCase.php | 3 + .../HttpKernel/Tests/HttpCache/StoreTest.php | 4 + 3 files changed, 92 insertions(+), 48 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/HttpCache/Store.php b/src/Symfony/Component/HttpKernel/HttpCache/Store.php index 433b0ef29b..b57d4a774d 100644 --- a/src/Symfony/Component/HttpKernel/HttpCache/Store.php +++ b/src/Symfony/Component/HttpKernel/HttpCache/Store.php @@ -38,7 +38,7 @@ class Store implements StoreInterface public function __construct($root) { $this->root = $root; - if (!is_dir($this->root) && !@mkdir($this->root, 0777, true) && !is_dir($this->root)) { + if (!file_exists($this->root) && !@mkdir($this->root, 0777, true) && !is_dir($this->root)) { throw new \RuntimeException(sprintf('Unable to create the store directory (%s).', $this->root)); } $this->keyCache = new \SplObjectStorage(); @@ -52,22 +52,15 @@ class Store implements StoreInterface { // unlock everything foreach ($this->locks as $lock) { - if (file_exists($lock)) { - @unlink($lock); - } + flock($lock, LOCK_UN); + fclose($lock); } - $error = error_get_last(); - if (1 === $error['type'] && false === headers_sent()) { - // send a 503 - header('HTTP/1.0 503 Service Unavailable'); - header('Retry-After: 10'); - echo '503 Service Unavailable'; - } + $this->locks = array(); } /** - * Locks the cache for a given Request. + * Tries to lock the cache for a given Request, without blocking. * * @param Request $request A Request instance * @@ -75,21 +68,24 @@ class Store implements StoreInterface */ public function lock(Request $request) { - $path = $this->getPath($this->getCacheKey($request).'.lck'); - if (!is_dir(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) { - return false; + $key = $this->getCacheKey($request); + + if (!isset($this->locks[$key])) { + $path = $this->getPath($key); + if (!file_exists(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) { + return $path; + } + $h = fopen($path, 'cb'); + if (!flock($h, LOCK_EX | LOCK_NB)) { + fclose($h); + + return $path; + } + + $this->locks[$key] = $h; } - $lock = @fopen($path, 'x'); - if (false !== $lock) { - fclose($lock); - - $this->locks[] = $path; - - return true; - } - - return !file_exists($path) ?: $path; + return true; } /** @@ -101,17 +97,37 @@ class Store implements StoreInterface */ public function unlock(Request $request) { - $file = $this->getPath($this->getCacheKey($request).'.lck'); + $key = $this->getCacheKey($request); - return is_file($file) ? @unlink($file) : false; + if (isset($this->locks[$key])) { + flock($this->locks[$key], LOCK_UN); + fclose($this->locks[$key]); + unset($this->locks[$key]); + + return true; + } + + return false; } public function isLocked(Request $request) { - $path = $this->getPath($this->getCacheKey($request).'.lck'); - clearstatcache(true, $path); + $key = $this->getCacheKey($request); - return is_file($path); + if (isset($this->locks[$key])) { + return true; // shortcut if lock held by this process + } + + if (!file_exists($path = $this->getPath($key))) { + return false; + } + + $h = fopen($path, 'rb'); + flock($h, LOCK_EX | LOCK_NB, $wouldBlock); + flock($h, LOCK_UN); // release the lock we just acquired + fclose($h); + + return (bool) $wouldBlock; } /** @@ -144,7 +160,7 @@ class Store implements StoreInterface } list($req, $headers) = $match; - if (is_file($body = $this->getPath($headers['x-content-digest'][0]))) { + if (file_exists($body = $this->getPath($headers['x-content-digest'][0]))) { return $this->restoreResponse($headers, $body); } @@ -291,7 +307,7 @@ class Store implements StoreInterface */ private function getMetadata($key) { - if (false === $entries = $this->load($key)) { + if (!$entries = $this->load($key)) { return array(); } @@ -307,7 +323,15 @@ class Store implements StoreInterface */ public function purge($url) { - if (is_file($path = $this->getPath($this->getCacheKey(Request::create($url))))) { + $key = $this->getCacheKey(Request::create($url)); + + if (isset($this->locks[$key])) { + flock($this->locks[$key], LOCK_UN); + fclose($this->locks[$key]); + unset($this->locks[$key]); + } + + if (file_exists($path = $this->getPath($key))) { unlink($path); return true; @@ -327,7 +351,7 @@ class Store implements StoreInterface { $path = $this->getPath($key); - return is_file($path) ? file_get_contents($path) : false; + return file_exists($path) ? file_get_contents($path) : false; } /** @@ -341,23 +365,36 @@ class Store implements StoreInterface private function save($key, $data) { $path = $this->getPath($key); - if (!is_dir(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) { - return false; - } - $tmpFile = tempnam(dirname($path), basename($path)); - if (false === $fp = @fopen($tmpFile, 'wb')) { - return false; - } - @fwrite($fp, $data); - @fclose($fp); + if (isset($this->locks[$key])) { + $fp = $this->locks[$key]; + @ftruncate($fp, 0); + @fseek($fp, 0); + $len = @fwrite($fp, $data); + if (strlen($data) !== $len) { + @ftruncate($fp, 0); - if ($data != file_get_contents($tmpFile)) { - return false; - } + return false; + } + } else { + if (!file_exists(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) { + return false; + } - if (false === @rename($tmpFile, $path)) { - return false; + $tmpFile = tempnam(dirname($path), basename($path)); + if (false === $fp = @fopen($tmpFile, 'wb')) { + return false; + } + @fwrite($fp, $data); + @fclose($fp); + + if ($data != file_get_contents($tmpFile)) { + return false; + } + + if (false === @rename($tmpFile, $path)) { + return false; + } } @chmod($path, 0666 & ~umask()); diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php index 1fcd210751..722e98760f 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php @@ -50,6 +50,9 @@ class HttpCacheTestCase extends \PHPUnit_Framework_TestCase protected function tearDown() { + if ($this->cache) { + $this->cache->getStore()->cleanup(); + } $this->kernel = null; $this->cache = null; $this->caches = null; diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php index 4198ce4031..bf4239beae 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php @@ -19,6 +19,10 @@ class StoreTest extends \PHPUnit_Framework_TestCase { protected $request; protected $response; + + /** + * @var Store + */ protected $store; protected function setUp()