diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #34839 [Cache] fix memory leak when using PhpArrayAdapter (nicolas-grekas)

Browse Source 
This PR was merged into the 3.4 branch.

Discussion
----------

[Cache] fix memory leak when using PhpArrayAdapter

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

Thanks to @adrienfr, I've been able to understand what causes this massive memory leak when using `PhpArrayAdapter`:
![image](https://user-images.githubusercontent.com/243674/70262187-303b1b00-1794-11ea-9fcb-21ae29c31ff0.png)

When tests run, a new kernel is booted for each test case. This means a new instance of `PhpArrayAdapter` is created, which means it loads its state again and again using `include` for e.g. `annotations.php` in this example.

The first obvious thing is that we see this doing `compile::*`: this means PHP is parsing the same file again and again. But shouldn't opcache prevent this? Well, it's disabled by default because `opcache.enable_cli=0`. To prove the point, here is a comparison with the same tests run with `php -dopcache.enable_cli=1`. The comparison is swapped, but you'll get it:

![image](https://user-images.githubusercontent.com/243674/70262616-fb7b9380-1794-11ea-81c3-6fea0145a63b.png)

But that's not over: because of https://bugs.php.net/76982 (see #32236 also), we still have a memory leak when the included file contains closures. And this one does.

This PR fixes the issue by storing the return value of the include statement into a static property. This fits the caching model of `PhpArrayAdapter`: it's a read-only storage for system caches - i.e. its content is immutable.

Commits
-------

4194c4c56d [Cache] fix memory leak when using PhpArrayAdapter
This commit is contained in:
Fabien Potencier 2019-12-07 14:38:12 +01:00
commit 7dbc4c677b
7 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php
View File

@ -61,14 +61,13 @@ class PhpArrayAdapter implements AdapterInterface, PruneableInterface, Resettabl
* fallback pool with this adapter only if the current PHP version is supported.
*
* @param string $file The PHP file were values are cached
* @param CacheItemPoolInterface $fallbackPool Fallback for old PHP versions or opcache disabled
* @param CacheItemPoolInterface $fallbackPool A pool to fallback on when an item is not hit
*
* @return CacheItemPoolInterface
*/
public static function create($file, CacheItemPoolInterface $fallbackPool)
{
// Shared memory is available in PHP 7.0+ with OPCache enabled and in HHVM
if ((\PHP_VERSION_ID >= 70000 && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN)) || \defined('HHVM_VERSION')) {
if (\PHP_VERSION_ID >= 70000) {
if (!$fallbackPool instanceof AdapterInterface) {
$fallbackPool = new ProxyAdapter($fallbackPool);
}

6
src/Symfony/Component/Cache/Simple/PhpArrayCache.php
View File

@ -44,14 +44,14 @@ class PhpArrayCache implements CacheInterface, PruneableInterface, ResettableInt
* stores arrays in its latest versions. This factory method decorates the given
* fallback pool with this adapter only if the current PHP version is supported.
*
* @param string $file The PHP file were values are cached
* @param string $file The PHP file were values are cached
* @param CacheInterface $fallbackPool A pool to fallback on when an item is not hit
*
* @return CacheInterface
*/
public static function create($file, CacheInterface $fallbackPool)
{
// Shared memory is available in PHP 7.0+ with OPCache enabled and in HHVM
if ((\PHP_VERSION_ID >= 70000 && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN)) || \defined('HHVM_VERSION')) {
if (\PHP_VERSION_ID >= 70000) {
return new static($file, $fallbackPool);
}

2
src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php
View File

@ -62,6 +62,8 @@ class PhpArrayAdapterTest extends AdapterTestCase
protected function tearDown()
{
$this->createCachePool()->clear();
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
}

2
src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterWithFallbackTest.php
View File

@ -37,6 +37,8 @@ class PhpArrayAdapterWithFallbackTest extends AdapterTestCase
protected function tearDown()
{
$this->createCachePool()->clear();
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
}

2
src/Symfony/Component/Cache/Tests/Simple/PhpArrayCacheTest.php
View File

@ -56,6 +56,8 @@ class PhpArrayCacheTest extends CacheTestCase
protected function tearDown()
{
$this->createSimpleCache()->clear();
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
}

2
src/Symfony/Component/Cache/Tests/Simple/PhpArrayCacheWithFallbackTest.php
View File

@ -43,6 +43,8 @@ class PhpArrayCacheWithFallbackTest extends CacheTestCase
protected function tearDown()
{
$this->createSimpleCache()->clear();
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
}

12
src/Symfony/Component/Cache/Traits/PhpArrayTrait.php
View File

@ -28,6 +28,8 @@ trait PhpArrayTrait
private $values;
private $zendDetectUnicode;
private static $valuesCache = [];
/**
* Store an array of cached values.
*
@ -107,6 +109,7 @@ EOF;
unset($serialized, $unserialized, $value, $dump);
@rename($tmpFile, $this->file);
unset(self::$valuesCache[$this->file]);
$this->initialize();
}
@ -119,6 +122,7 @@ EOF;
$this->values = [];
$cleared = @unlink($this->file) || !file_exists($this->file);
unset(self::$valuesCache[$this->file]);
return $this->pool->clear() && $cleared;
}
@ -128,11 +132,17 @@ EOF;
*/
private function initialize()
{
if (isset(self::$valuesCache[$this->file])) {
$this->values = self::$valuesCache[$this->file];
return;
}
if ($this->zendDetectUnicode) {
$zmb = ini_set('zend.detect_unicode', 0);
}
try {
$this->values = file_exists($this->file) ? (include $this->file ?: []) : [];
$this->values = self::$valuesCache[$this->file] = file_exists($this->file) ? (include $this->file ?: []) : [];
} finally {
if ($this->zendDetectUnicode) {
ini_set('zend.detect_unicode', $zmb);