Merge branch '2.8' into 3.0

* 2.8:
  [TwigBundle] Removed redundant return statement.
  [DependencyInjection] Fixed deprecated default message template with XML
  [TwigBridge] Removed extra arguments in 2 places.
  [Process] Fix write access check for pipes on Windows
  [HttpKernel] Use flock() for HttpCache's lock files

Author: nicolas-grekas

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,44 +52,40 @@ public function cleanup()
     {
         // 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
      *
      * @return bool|string true if the lock is acquired, the path to the current lock otherwise
      */
     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);
 
-        $lock = @fopen($path, 'x');
-        if (false !== $lock) {
-            fclose($lock);
+        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);
 
-            $this->locks[] = $path;
+                return $path;
+            }
 
-            return true;
+            $this->locks[$key] = $h;
         }
 
-        return !file_exists($path) ?: $path;
+        return true;
     }
 
     /**
@@ -101,17 +97,37 @@ public function lock(Request $request)
      */
     public function unlock(Request $request)
     {
-        $file = $this->getPath($this->getCacheKey($request).'.lck');
+        $key = $this->getCacheKey($request);
+
+        if (isset($this->locks[$key])) {
+            flock($this->locks[$key], LOCK_UN);
+            fclose($this->locks[$key]);
+            unset($this->locks[$key]);
+
+            return true;
+        }
 
-        return is_file($file) ? @unlink($file) : false;
+        return false;
     }
 
     public function isLocked(Request $request)
     {
-        $path = $this->getPath($this->getCacheKey($request).'.lck');
-        clearstatcache(true, $path);
+        $key = $this->getCacheKey($request);
+
+        if (isset($this->locks[$key])) {
+            return true; // shortcut if lock held by this process
+        }
+
+        if (!file_exists($path = $this->getPath($key))) {
+            return false;
+        }
 
-        return is_file($path);
+        $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 @@ public function lookup(Request $request)
         }
 
         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 @@ private function requestsMatch($vary, $env1, $env2)
      */
     private function getMetadata($key)
     {
-        if (false === $entries = $this->load($key)) {
+        if (!$entries = $this->load($key)) {
             return array();
         }
 
@@ -307,7 +323,15 @@ private function getMetadata($key)
      */
     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 @@ private function load($key)
     {
         $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 @@ private function load($key)
     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());

Tests/HttpCache/HttpCacheTestCase.php

@@ -50,6 +50,9 @@ protected function setUp()
 
     protected function tearDown()
     {
+        if ($this->cache) {
+            $this->cache->getStore()->cleanup();
+        }
         $this->kernel = null;
         $this->cache = null;
         $this->caches = null;

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()