bug #19300 [HttpKernel] Use flock() for HttpCache's lock files (mpdude)
This PR was merged into the 2.7 branch.
Discussion
----------
[HttpKernel] Use flock() for HttpCache's lock files
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | no
| Fixed tickets | #16777, #15813 and #16312 are also related
| License | MIT
| Doc PR |
When a PHP process crashes or terminates (maybe the OOM killer kicks in or other bad things ™️ happen) while the `HttpCache` holds a `.lck` file, that lock file may not get `unlink()`ed.
The result is that other requests trying to access this cache entry will see a few seconds delay while waiting for the lock; they will eventually continue but send 503 status codes along with the response. The sudden buildup of PHP processes caused by the additional delay may cause further problems (sudden load increase).
As `LockHandler` is using `flock()`-based locking, locks should be released by the OS when the PHP process terminates.
I wrote this as bugfix against 2.7 because every once in a while I encounter situations (not always reproducible) where `.lock` files are left over and keep the cache locked.
Commits
-------
2668edd
[HttpKernel] Use flock() for HttpCache's lock files
This commit is contained in:
commit
a8a9923260
@ -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());
|
||||
|
@ -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;
|
||||
|
@ -19,6 +19,10 @@ class StoreTest extends \PHPUnit_Framework_TestCase
|
||||
{
|
||||
protected $request;
|
||||
protected $response;
|
||||
|
||||
/**
|
||||
* @var Store
|
||||
*/
|
||||
protected $store;
|
||||
|
||||
protected function setUp()
|
||||
|
Reference in New Issue
Block a user