10000 bug #19300 [HttpKernel] Use flock() for HttpCache's lock files (mpdude) · symfony/symfony@a8a9923 · GitHub
[go: up one dir, main page]
Skip to content

Commit a8a9923

Browse files
nicolas-grekasnicolas-grekas
committed
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
2 parents f9ba34f + 2668edd commit a8a9923

File tree

3 files changed

+89
-45
lines changed

3 files changed

+89
-45
lines changed

‎src/Symfony/Component/HttpKernel/HttpCache/Store.php

Lines changed: 82 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class Store implements StoreInterface
3838
public function __construct($root)
3939
{
4040
$this->root = $root;
41-
if (!is_dir($this->root) && !@mkdir($this->root, 0777, true) && !is_dir($this->root)) {
41+
if (!file_exists($this->root) && !@mkdir($this->root, 0777, true) && !is_dir($this->root)) {
4242
throw new \RuntimeException(sprintf('Unable to create the store directory (%s).', $this->root));
4343
}
4444
$this->keyCache = new \SplObjectStorage();
@@ -52,44 +52,40 @@ public function cleanup()
5252
{
5353
// unlock everything
5454
foreach ($this->locks as $lock) {
55-
if (file_exists($lock)) {
56-
@unlink($lock);
57-
}
55+
flock($lock, LOCK_UN);
56+
fclose($lock);
5857
}
5958

60-
$error = error_get_last();
61-
if (1 === $error['type'] && false === headers_sent()) {
62-
// send a 503
63-
header('HTTP/1.0 503 Service Unavailable');
64-
header('Retry-After: 10');
65-
echo '503 Service Unavailable';
66-
}
59+
$this->locks = array();
6760
}
6861

6962
/**
70-
* Locks the cache for a given Request.
63+
* Tries to lock the cache for a given Request, without blocking.
7164
*
7265
* @param Request $request A Request instance
7366
*
7467
* @return bool|string true if the lock is acquired, the path to the current lock otherwise
7568
*/
7669
public function lock(Request $request)
7770
{
78-
$path = $this->getPath($this->getCacheKey($request).'.lck');
79-
if (!is_dir(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) {
80-
return false;
81-
}
71+
$key = $this->getCacheKey($request);
8272

83-
$lock = @fopen($path, 'x');
84-
if (false !== $lock) {
85-
fclose($lock);
73+
if (!isset($this->locks[$key])) {
74+
$path = $this->getPath($key);
75+
if (!file_exists(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) {
76+
return $path;
77+
}
78+
$h = fopen($path, 'cb');
79+
if (!flock($h, LOCK_EX | LOCK_NB)) {
80+
fclose($h);
8681

87-
$this->locks[] = $path;
82+
return $path;
83+
}
8884

89-
return true;
85+
$this->locks[$key] = $h;
9086
}
9187

92-
return !file_exists($path) ?: $path;
88+
return true;
9389
}
9490

9591
/**
@@ -101,17 +97,37 @@ public function lock(Request $request)
10197
*/
10298
public function unlock(Request $request)
10399
{
104-
$file = $this->getPath($this->getCacheKey($request).'.lck');
100+
$key = $this->getCacheKey($request);
101+
102+
if (isset($this->locks[$key])) {
103+
flock($this->locks[$key], LOCK_UN);
104+
fclose($this->locks[$key]);
105+
unset($this->locks[$key]);
106+
107+
return true;
108+
}
105109

106-
return is_file($file) ? @unlink($file) : false;
110+
return false;
107111
}
108112

109113
public function isLocked(Request $request)
110114
{
111-
$path = $this->getPath($this->getCacheKey($request).'.lck');
112-
clearstatcache(true, $path);
115+
$key = $this->getCacheKey($request);
116+
117+
if (isset($this->locks[$key])) {
118+
return true; // shortcut if lock held by this process
119+
}
120+
121+
if (!file_exists($path = $this->getPath($key))) {
122+
return false;
123+
}
113124

114-
return is_file($path);
125+
$h = fopen($path, 'rb');
126+
flock($h, LOCK_EX | LOCK_NB, $wouldBlock);
127+
flock($h, LOCK_UN); // release the lock we just acquired
128+
fclose($h);
129+
130+
return (bool) $wouldBlock;
115131
}
116132

117133
/**
@@ -144,7 +160,7 @@ public function lookup(Request $request)
144160
}
145161

146162
list($req, $headers) = $match;
147-
if (is_file($body = $this->getPath($headers['x-content-digest'][0]))) {
163+
if (file_exists($body = $this->getPath($headers['x-content-digest'][0]))) {
148164
return $this->restoreResponse($headers, $body);
149165
}
150166

@@ -291,7 +307,7 @@ private function requestsMatch($vary, $env1, $env2)
291307
*/
292308
private function getMetadata($key)
293309
{
294-
if (false === $entries = $this->load($key)) {
310+
if (!$entries = $this->load($key)) {
295311
return array();
296312
}
297313

@@ -307,7 +323,15 @@ private function getMetadata($key)
307323
*/
308324
public function purge($url)
309325
{
310-
if (is_file($path = $this->getPath($this->getCacheKey(Request::create($url))))) {
326+
$key = $this->getCacheKey(Request::create($url));
327+
328+
if (isset($this->locks[$key])) {
329+
flock($this->locks[$key], LOCK_UN);
330+
fclose($this->locks[$key]);
331+
unset($this->locks[$key]);
332+
}
333+
334+
if (file_exists($path = $this->getPath($key))) {
311335
unlink($path);
312336

313337
return true;
@@ -327,7 +351,7 @@ private function load($key)
327351
{
328352
$path = $this->getPath($key);
329353

330-
return is_file($path) ? file_get_contents($path) : false;
354+
return file_exists($path) ? file_get_contents($path) : false;
331355
}
332356

333357
/**
@@ -341,23 +365,36 @@ private function load($key)
341365
private function save($key, $data)
342366
{
343367
$path = $this->getPath($key);
344-
if (!is_dir(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) {
345-
return false;
346-
}
347368

348-
$tmpFile = tempnam(dirname($path), basename($path));
349-
if (false === $fp = @fopen($tmpFile, 'wb')) {
350-
return false;
351-
}
352-
@fwrite($fp, $data);
353-
@fclose($fp);
369+
if (isset($this->locks[$key])) {
370+
$fp = $this->locks[$key];
371+
@ftruncate($fp, 0);
372+
@fseek($fp, 0);
373+
$len = @fwrite($fp, $data);
374+
if (strlen($data) !== $len) {
375+
@ftruncate($fp, 0);
354376

355-
if ($data != file_get_contents($tmpFile)) {
356-
return false;
357-
}
377+
return false;
378+
}
379+
} else {
380+
if (!file_exists(dirname($path)) && false === @mkdir(dirname($path), 0777, true) && !is_dir(dirname($path))) {
381+
return false;
382+
}
358383

359-
if (false === @rename($tmpFile, $path)) {
360-
return false;
384+
$tmpFile = tempnam(dirname($path), basename($path));
385+
if (false === $fp = @fopen($tmpFile, 'wb')) {
386+
return false;
387+
}
388+
@fwrite($fp, $data);
389+
@fclose($fp);
390+
391+
if ($data != file_get_contents($tmpFile)) {
392+
return false;
393+
}
394+
395+
if (false === @rename($tmpFile, $path)) {
396+
return false;
397+
}
361398
}
362399

363400
@chmod($path, 0666 & ~umask());

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ protected function setUp()
5050

5151
protected function tearDown()
5252
{
53+
if ($this->cache) {
54+
$this->cache->getStore()->cleanup();
55+
}
5356
$this->kernel = null;
5457
$this->cache = null;
5558
$this->caches = null;

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ class StoreTest extends \PHPUnit_Framework_TestCase
1919
{
2020
protected $request;
2121
protected $response;
22+
23+
/**
24+
* @var Store
25+
*/
2226
protected $store;
2327

2428
protected function setUp()

0 commit comments

Comments
 (0)
0