[Cache] fix saving unrelated keys in recursive callback calls

nicolas-grekas · nicolas-grekas · commit 4443c2018ceb · 2019-05-11T20:09:18.000+02:00
diff --git a/src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php b/src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php
@@ -59,6 +59,26 @@ public function testGet()
         $this->assertFalse($isHit);
     }
 
+    public function testRecursiveGet()
+    {
+        if (isset($this->skippedTests[__FUNCTION__])) {
+            $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
+        }
+
+        $cache = $this->createCachePool(0, __FUNCTION__);
+
+        $v = $cache->get('k1', function () use (&$counter, $cache) {
+            $v = $cache->get('k2', function () use (&$counter) { return ++$counter; });
+            $v = $cache->get('k2', function () use (&$counter) { return ++$counter; });
+
+            return $v;
+        });
+
+        $this->assertSame(1, $counter);
+        $this->assertSame(1, $v);
+        $this->assertSame(1, $cache->get('k2', function () { return 2; }));
+    }
+
     public function testGetMetadata()
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php
@@ -23,6 +23,7 @@ class PhpArrayAdapterTest extends AdapterTestCase
 {
     protected $skippedTests = [
         'testGet' => 'PhpArrayAdapter is read-only.',
+        'testRecursiveGet' => 'PhpArrayAdapter is read-only.',
         'testBasicUsage' => 'PhpArrayAdapter is read-only.',
         'testBasicUsageWithLongKey' => 'PhpArrayAdapter is read-only.',
         'testClear' => 'PhpArrayAdapter is read-only.',
diff --git a/src/Symfony/Component/Cache/Traits/ContractsTrait.php b/src/Symfony/Component/Cache/Traits/ContractsTrait.php
@@ -31,6 +31,7 @@ trait ContractsTrait
     }
 
     private $callbackWrapper = [LockRegistry::class, 'compute'];
+    private $computing = [];
 
     /**
      * Wraps the callback passed to ->get() in a callable.
@@ -68,26 +69,27 @@ function (CacheItem $item, float $startTime, ?array &$metadata) {
             CacheItem::class
         );
 
-        return $this->contractsGet($pool, $key, function (CacheItem $item, bool &$save) use ($pool, $callback, $setMetadata, &$metadata) {
+        return $this->contractsGet($pool, $key, function (CacheItem $item, bool &$save) use ($pool, $callback, $setMetadata, &$metadata, $key) {
             // don't wrap nor save recursive calls
-            if (null === $callbackWrapper = $this->callbackWrapper) {
+            if (isset($this->computing[$key])) {
                 $value = $callback($item, $save);
                 $save = false;
 
                 return $value;
             }
-            $this->callbackWrapper = null;
+
+            $this->computing[$key] = $key;
             $startTime = microtime(true);
 
             try {
-                $value = $callbackWrapper($callback, $item, $save, $pool, function (CacheItem $item) use ($setMetadata, $startTime, &$metadata) {
+                $value = ($this->callbackWrapper)($callback, $item, $save, $pool, function (CacheItem $item) use ($setMetadata, $startTime, &$metadata) {
                     $setMetadata($item, $startTime, $metadata);
                 });
                 $setMetadata($item, $startTime, $metadata);
 
                 return $value;
             } finally {
-                $this->callbackWrapper = $callbackWrapper;
+                unset($this->computing[$key]);
             }
         }, $beta, $metadata);
     }

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ class PhpArrayAdapterTest extends AdapterTestCase`
`23`	`23`	`{`
`24`	`24`	`protected $skippedTests = [`
`25`	`25`	`'testGet' => 'PhpArrayAdapter is read-only.',`
	`26`	`+ 'testRecursiveGet' => 'PhpArrayAdapter is read-only.',`
`26`	`27`	`'testBasicUsage' => 'PhpArrayAdapter is read-only.',`
`27`	`28`	`'testBasicUsageWithLongKey' => 'PhpArrayAdapter is read-only.',`
`28`	`29`	`'testClear' => 'PhpArrayAdapter is read-only.',`