bug #34896 [Cache] fix memory leak when using PhpFilesAdapter (nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[Cache] fix memory leak when using PhpFilesAdapter

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34687
| License       | MIT
| Doc PR        | -

Similar to #34839 but for `PhpFilesAdapter`, as the "appendOnly" mode is a v4-only feature.

Commits
-------

0b46226648 [Cache] fix memory leak when using PhpFilesAdapter
This commit is contained in:
Nicolas Grekas 2019-12-10 11:24:37 +01:00
commit 5aff4a66f1
3 changed files with 60 additions and 12 deletions

View File

@ -0,0 +1,31 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Cache\Tests\Adapter;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\Adapter\PhpFilesAdapter;
/**
* @group time-sensitive
*/
class PhpFilesAdapterAppendOnlyTest extends PhpFilesAdapterTest
{
protected $skippedTests = [
'testDefaultLifeTime' => 'PhpFilesAdapter does not allow configuring a default lifetime.',
'testExpiration' => 'PhpFilesAdapter in append-only mode does not expiration.',
];
public function createCachePool(): CacheItemPoolInterface
{
return new PhpFilesAdapter('sf-cache', 0, null, true);
}
}

View File

@ -26,7 +26,7 @@ trait FilesystemCommonTrait
private function init($namespace, $directory) private function init($namespace, $directory)
{ {
if (!isset($directory[0])) { if (!isset($directory[0])) {
$directory = sys_get_temp_dir().'/symfony-cache'; $directory = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'symfony-cache';
} else { } else {
$directory = realpath($directory) ?: $directory; $directory = realpath($directory) ?: $directory;
} }
@ -55,8 +55,8 @@ trait FilesystemCommonTrait
{ {
$ok = true; $ok = true;
foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS)) as $file) { foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::CURRENT_AS_PATHNAME), \RecursiveIteratorIterator::LEAVES_ONLY) as $file) {
$ok = ($file->isDir() || $this->doUnlink($file) || !file_exists($file)) && $ok; $ok = ($this->doUnlink($file) || !file_exists($file)) && $ok;
} }
return $ok; return $ok;

View File

@ -35,12 +35,13 @@ trait PhpFilesTrait
private $files = []; private $files = [];
private static $startTime; private static $startTime;
private static $valuesCache = [];
public static function isSupported() public static function isSupported()
{ {
self::$startTime = self::$startTime ?? $_SERVER['REQUEST_TIME'] ?? time(); self::$startTime = self::$startTime ?? $_SERVER['REQUEST_TIME'] ?? time();
return \function_exists('opcache_invalidate') && ('cli' !== \PHP_SAPI || filter_var(ini_get('opcache.enable_cli'), FILTER_VALIDATE_BOOLEAN)) && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN); return \function_exists('opcache_invalidate') && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN) && (!\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) || filter_var(ini_get('opcache.enable_cli'), FILTER_VALIDATE_BOOLEAN));
} }
/** /**
@ -54,7 +55,7 @@ trait PhpFilesTrait
set_error_handler($this->includeHandler); set_error_handler($this->includeHandler);
try { try {
foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::LEAVES_ONLY) as $file) { foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::CURRENT_AS_PATHNAME), \RecursiveIteratorIterator::LEAVES_ONLY) as $file) {
try { try {
if (\is_array($expiresAt = include $file)) { if (\is_array($expiresAt = include $file)) {
$expiresAt = $expiresAt[0]; $expiresAt = $expiresAt[0];
@ -100,7 +101,6 @@ trait PhpFilesTrait
} elseif (!\is_object($value)) { } elseif (!\is_object($value)) {
$values[$id] = $value; $values[$id] = $value;
} elseif (!$value instanceof LazyValue) { } elseif (!$value instanceof LazyValue) {
// calling a Closure is for @deprecated BC and should be removed in Symfony 5.0
$values[$id] = $value(); $values[$id] = $value();
} elseif (false === $values[$id] = include $value->file) { } elseif (false === $values[$id] = include $value->file) {
unset($values[$id], $this->values[$id]); unset($values[$id], $this->values[$id]);
@ -123,14 +123,20 @@ trait PhpFilesTrait
try { try {
$file = $this->files[$id] ?? $this->files[$id] = $this->getFile($id); $file = $this->files[$id] ?? $this->files[$id] = $this->getFile($id);
if (\is_array($expiresAt = include $file)) { if (isset(self::$valuesCache[$file])) {
[$expiresAt, $this->values[$id]] = self::$valuesCache[$file];
} elseif (\is_array($expiresAt = include $file)) {
if ($this->appendOnly) {
self::$valuesCache[$file] = $expiresAt;
}
[$expiresAt, $this->values[$id]] = $expiresAt; [$expiresAt, $this->values[$id]] = $expiresAt;
} elseif ($now < $expiresAt) { } elseif ($now < $expiresAt) {
$this->values[$id] = new LazyValue($file); $this->values[$id] = new LazyValue($file);
} }
if ($now >= $expiresAt) { if ($now >= $expiresAt) {
unset($this->values[$id], $missingIds[$k]); unset($this->values[$id], $missingIds[$k], self::$valuesCache[$file]);
} }
} catch (\ErrorException $e) { } catch (\ErrorException $e) {
unset($missingIds[$k]); unset($missingIds[$k]);
@ -159,7 +165,13 @@ trait PhpFilesTrait
$file = $this->files[$id] ?? $this->files[$id] = $this->getFile($id); $file = $this->files[$id] ?? $this->files[$id] = $this->getFile($id);
$getExpiry = true; $getExpiry = true;
if (\is_array($expiresAt = include $file)) { if (isset(self::$valuesCache[$file])) {
[$expiresAt, $value] = self::$valuesCache[$file];
} elseif (\is_array($expiresAt = include $file)) {
if ($this->appendOnly) {
self::$valuesCache[$file] = $expiresAt;
}
[$expiresAt, $value] = $expiresAt; [$expiresAt, $value] = $expiresAt;
} elseif ($this->appendOnly) { } elseif ($this->appendOnly) {
$value = new LazyValue($file); $value = new LazyValue($file);
@ -211,12 +223,14 @@ trait PhpFilesTrait
$value = var_export($value, true); $value = var_export($value, true);
} }
if (!$isStaticValue) { if ($isStaticValue) {
$value = "<?php return [{$expiry}, {$value}];\n";
} elseif ($this->appendOnly) {
$value = "<?php return [{$expiry}, static function () { return {$value}; }];\n";
} else {
// We cannot use a closure here because of https://bugs.php.net/76982 // We cannot use a closure here because of https://bugs.php.net/76982
$value = str_replace('\Symfony\Component\VarExporter\Internal\\', '', $value); $value = str_replace('\Symfony\Component\VarExporter\Internal\\', '', $value);
$value = "<?php\n\nnamespace Symfony\Component\VarExporter\Internal;\n\nreturn \$getExpiry ? {$expiry} : {$value};\n"; $value = "<?php\n\nnamespace Symfony\Component\VarExporter\Internal;\n\nreturn \$getExpiry ? {$expiry} : {$value};\n";
} else {
$value = "<?php return [{$expiry}, {$value}];\n";
} }
$file = $this->files[$key] = $this->getFile($key, true); $file = $this->files[$key] = $this->getFile($key, true);
@ -227,6 +241,7 @@ trait PhpFilesTrait
@opcache_invalidate($file, true); @opcache_invalidate($file, true);
@opcache_compile_file($file); @opcache_compile_file($file);
} }
unset(self::$valuesCache[$file]);
} }
if (!$ok && !is_writable($this->directory)) { if (!$ok && !is_writable($this->directory)) {
@ -260,6 +275,8 @@ trait PhpFilesTrait
protected function doUnlink($file) protected function doUnlink($file)
{ {
unset(self::$valuesCache[$file]);
if (self::isSupported()) { if (self::isSupported()) {
@opcache_invalidate($file, true); @opcache_invalidate($file, true);
} }