From 0b46226648fea726cfc7ce26f1032d6c57dc0582 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 9 Dec 2019 17:45:12 +0100 Subject: [PATCH] [Cache] fix memory leak when using PhpFilesAdapter --- .../Adapter/PhpFilesAdapterAppendOnlyTest.php | 31 ++++++++++++++++ .../Cache/Traits/FilesystemCommonTrait.php | 6 ++-- .../Component/Cache/Traits/PhpFilesTrait.php | 35 ++++++++++++++----- 3 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 src/Symfony/Component/Cache/Tests/Adapter/PhpFilesAdapterAppendOnlyTest.php diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PhpFilesAdapterAppendOnlyTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PhpFilesAdapterAppendOnlyTest.php new file mode 100644 index 0000000000..a38a6f9f5c --- /dev/null +++ b/src/Symfony/Component/Cache/Tests/Adapter/PhpFilesAdapterAppendOnlyTest.php @@ -0,0 +1,31 @@ + + * + * 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); + } +} diff --git a/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php b/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php index 37e1fd1f06..ed72899524 100644 --- a/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php +++ b/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php @@ -26,7 +26,7 @@ trait FilesystemCommonTrait private function init($namespace, $directory) { if (!isset($directory[0])) { - $directory = sys_get_temp_dir().'/symfony-cache'; + $directory = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'symfony-cache'; } else { $directory = realpath($directory) ?: $directory; } @@ -55,8 +55,8 @@ trait FilesystemCommonTrait { $ok = true; - foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS)) as $file) { - $ok = ($file->isDir() || $this->doUnlink($file) || !file_exists($file)) && $ok; + foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::CURRENT_AS_PATHNAME), \RecursiveIteratorIterator::LEAVES_ONLY) as $file) { + $ok = ($this->doUnlink($file) || !file_exists($file)) && $ok; } return $ok; diff --git a/src/Symfony/Component/Cache/Traits/PhpFilesTrait.php b/src/Symfony/Component/Cache/Traits/PhpFilesTrait.php index 5ed4d6023e..a9a1092ad1 100644 --- a/src/Symfony/Component/Cache/Traits/PhpFilesTrait.php +++ b/src/Symfony/Component/Cache/Traits/PhpFilesTrait.php @@ -35,12 +35,13 @@ trait PhpFilesTrait private $files = []; private static $startTime; + private static $valuesCache = []; public static function isSupported() { 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); 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 { if (\is_array($expiresAt = include $file)) { $expiresAt = $expiresAt[0]; @@ -100,7 +101,6 @@ trait PhpFilesTrait } elseif (!\is_object($value)) { $values[$id] = $value; } elseif (!$value instanceof LazyValue) { - // calling a Closure is for @deprecated BC and should be removed in Symfony 5.0 $values[$id] = $value(); } elseif (false === $values[$id] = include $value->file) { unset($values[$id], $this->values[$id]); @@ -123,14 +123,20 @@ trait PhpFilesTrait try { $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; } elseif ($now < $expiresAt) { $this->values[$id] = new LazyValue($file); } if ($now >= $expiresAt) { - unset($this->values[$id], $missingIds[$k]); + unset($this->values[$id], $missingIds[$k], self::$valuesCache[$file]); } } catch (\ErrorException $e) { unset($missingIds[$k]); @@ -159,7 +165,13 @@ trait PhpFilesTrait $file = $this->files[$id] ?? $this->files[$id] = $this->getFile($id); $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; } elseif ($this->appendOnly) { $value = new LazyValue($file); @@ -211,12 +223,14 @@ trait PhpFilesTrait $value = var_export($value, true); } - if (!$isStaticValue) { + if ($isStaticValue) { + $value = "appendOnly) { + $value = "files[$key] = $this->getFile($key, true); @@ -227,6 +241,7 @@ trait PhpFilesTrait @opcache_invalidate($file, true); @opcache_compile_file($file); } + unset(self::$valuesCache[$file]); } if (!$ok && !is_writable($this->directory)) { @@ -260,6 +275,8 @@ trait PhpFilesTrait protected function doUnlink($file) { + unset(self::$valuesCache[$file]); + if (self::isSupported()) { @opcache_invalidate($file, true); }