10000 bug #34839 [Cache] fix memory leak when using PhpArrayAdapter (nicola… · symfony/symfony@7dbc4c6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7dbc4c6

Browse files
fabpotfabpot
committed
bug #34839 [Cache] fix memory leak when using PhpArrayAdapter (nicolas-grekas)
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 ------- 4194c4c [Cache] fix memory leak when using PhpArrayAdapter
2 parents 8f2cd5b + 4194c4c commit 7dbc4c6

File tree

7 files changed

+24
-7
lines changed

7 files changed

+24
-7
lines changed

‎src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,13 @@ static function ($key, $value, $isHit) {
6161
* fallback pool with this adapter only if the current PHP version is supported.
6262
*
6363
* @param string $file The PHP file were values are cached
64-
* @param CacheItemPoolInterface $fallbackPool Fallback for old PHP versions or opcache disabled
64+
* @param CacheItemPoolInterface $fallbackPool A pool to fallback on when an item is not hit
6565
*
6666
* @return CacheItemPoolInterface
6767
*/
6868
public static function create($file, CacheItemPoolInterface $fallbackPool)
6969
{
70-
// Shared memory is available in PHP 7.0+ with OPCache enabled and in HHVM
71-
if ((\PHP_VERSION_ID >= 70000 && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN)) || \defined('HHVM_VERSION')) {
70+
if (\PHP_VERSION_ID >= 70000) {
7271
if (!$fallbackPool instanceof AdapterInterface) {
7372
$fallbackPool = new ProxyAdapter($fallbackPool);
7473
}

‎src/Symfony/Component/Cache/Simple/PhpArrayCache.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ public function __construct($file, CacheInterface $fallbackPool)
4444
* stores arrays in its latest versions. This factory method decorates the given
4545
* fallback pool with this adapter only if the current PHP version is supported.
4646
*
47-
* @param string $file The PHP file were values are cached
47+
* @param string $file The PHP file were values are cached
48+
* @param CacheInterface $fallbackPool A pool to fallback on when an item is not hit
4849
*
4950
* @return CacheInterface
5051
*/
5152
public static function create($file, CacheInterface $fallbackPool)
5253
{
53-
// Shared memory is available in PHP 7.0+ with OPCache enabled and in HHVM
54-
if ((\PHP_VERSION_ID >= 70000 && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN)) || \defined('HHVM_VERSION')) {
54+
if (\PHP_VERSION_ID >= 70000) {
5555
return new static($file, $fallbackPool);
5656
}
5757

‎src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public static function setUpBeforeClass()
6262

6363
protected function tearDown()
6464
{
65+
$this->createCachePool()->clear();
66+
6567
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
6668
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
6769
}

‎src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterWithFallbackTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public static function setUpBeforeClass()
3737

3838
protected function tearDown()
3939
{
40+
$this->createCachePool()->clear();
41+
4042
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
4143
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
4244
}

‎src/Symfony/Component/Cache/Tests/Simple/PhpArrayCacheTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ public static function setUpBeforeClass()
5656

5757
protected function tearDown()
5858
{
59+
$this->createSimpleCache()->clear();
60+
5961
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
6062
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
6163
}

‎src/Symfony/Component/Cache/Tests/Simple/PhpArrayCacheWithFallbackTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ public static function setUpBeforeClass()
4343

4444
protected function tearDown()
4545
{
46+
$this->createSimpleCache()->clear();
47+
4648
if (file_exists(sys_get_temp_dir().'/symfony-cache')) {
4749
FilesystemAdapterTest::rmdir(sys_get_temp_dir().'/symfony-cache');
4850
}

‎src/Symfony/Component/Cache/Traits/PhpArrayTrait.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ trait PhpArrayTrait
2828
private $values;
2929
private $zendDetectUnicode;
3030

31+
private static $valuesCache = [];
32+
3133
/**
3234
* Store an array of cached values.
3335
*
@@ -107,6 +109,7 @@ public function warmUp(array $values)
107109
unset($serialized, $unserialized, $value, $dump);
108110

109111
@rename($tmpFile, $this->file);
112+
unset(self::$valuesCache[$this->file]);
110113

111114
$this->initialize();
112115
}
@@ -119,6 +122,7 @@ public function clear()
119122
$this->values = [];
120123

121124
$cleared = @unlink($this->file) || !file_exists($this->file);
125+
unset(self::$valuesCache[$this->file]);
122126

123127
return $this->pool->clear() && $cleared;
124128
}
@@ -128,11 +132,17 @@ public function clear()
128132
*/
129133
private function initialize()
130134
{
135+
if (isset(self::$valuesCache[$this->file])) {
136+
$this->values = self::$valuesCache[$this->file];
137+
138+
return;
139+
}
140+
131141
if ($this->zendDetectUnicode) {
132142
$zmb = ini_set('zend.detect_unicode', 0);
133143
}
134144
try {
135-
$this->values = file_exists($this->file) ? (include $this->file ?: []) : [];
145+
$this->values = self::$valuesCache[$this->file] = file_exists($this->file) ? (include $this->file ?: []) : [];
136146
} finally {
137147
if ($this->zendDetectUnicode) {
138148
ini_set('zend.detect_unicode', $zmb);

0 commit comments

Comments
 (0)
0