symfony · nicolas-grekas · Oct 31, 2018 · Oct 30, 2018
diff --git a/src/Symfony/Component/Cache/LockRegistry.php b/src/Symfony/Component/Cache/LockRegistry.php
@@ -25,7 +25,6 @@
  */
 class LockRegistry
 {
-    private static $save;
     private static $openedFiles = array();
     private static $lockedFiles = array();
 
@@ -75,29 +74,43 @@ public static function setFiles(array $files): array
         return $previousFiles;
     }
 
-    public static function compute(ItemInterface $item, callable $callback, CacheInterface $pool)
+    public static function compute(callable $callback, ItemInterface $item, bool &$save, CacheInterface $pool)
     {
         $key = self::$files ? crc32($item->getKey()) % \count(self::$files) : -1;
 
         if ($key < 0 || (self::$lockedFiles[$key] ?? false) || !$lock = self::open($key)) {
-            return $callback($item);
+            return $callback($item, $save);
         }
 
-        try {
-            // race to get the lock in non-blocking mode
-            if (flock($lock, LOCK_EX | LOCK_NB)) {
-                self::$lockedFiles[$key] = true;
+        while (true) {
+            try {
+                // race to get the lock in non-blocking mode
+                if (flock($lock, LOCK_EX | LOCK_NB)) {
+                    self::$lockedFiles[$key] = true;
 
-                return $callback($item);
+                    return $callback($item, $save);
+                }
+                // if we failed the race, retry locking in blocking mode to wait for the winner
+                flock($lock, LOCK_SH);
+            } finally {
+                flock($lock, LOCK_UN);
+                unset(self::$lockedFiles[$key]);
             }
-            // if we failed the race, retry locking in blocking mode to wait for the winner
-            flock($lock, LOCK_SH);
-        } finally {
-            flock($lock, LOCK_UN);
-            unset(self::$lockedFiles[$key]);
-        }
+            static $signalingException, $signalingCallback;
+            $signalingException = $signalingException ?? unserialize("O:9:\"Exception\":1:{s:16:\"\0Exception\0trace\";a:0:{}}");
+            $signalingCallback = $signalingCallback ?? function () use ($signalingException) { throw $signalingException; };
 
-        return $pool->get($item->getKey(), $callback, 0);
+            try {
+                $value = $pool->get($item->getKey(), $signalingCallback, 0);
+                $save = false;
+
+                return $value;
+            } catch (\Exception $e) {
+                if ($signalingException !== $e) {
+                    throw $e;
+                }
+            }
+        }
     }
 
     private static function open(int $key)

diff --git a/src/Symfony/Component/Cache/Traits/ContractsTrait.php b/src/Symfony/Component/Cache/Traits/ContractsTrait.php
@@ -35,14 +35,14 @@ trait ContractsTrait
     /**
      * Wraps the callback passed to ->get() in a callable.
      *
-     * @param callable(ItemInterface, callable, CacheInterface):mixed $callbackWrapper
-     *
      * @return callable the previous callback wrapper
      */
-    public function setCallbackWrapper(callable $callbackWrapper): callable
+    public function setCallbackWrapper(?callable $callbackWrapper): callable
     {
         $previousWrapper = $this->callbackWrapper;
-        $this->callbackWrapper = $callbackWrapper;
+        $this->callbackWrapper = $callbackWrapper ?? function (callable $callback, ItemInterface $item, bool &$save, CacheInterface $pool) {
+            return $callback($item, $save);
+        };
 
         return $previousWrapper;
     }
@@ -53,32 +53,35 @@ private function doGet(AdapterInterface $pool, string $key, callable $callback,
             throw new InvalidArgumentException(sprintf('Argument "$beta" provided to "%s::get()" must be a positive number, %f given.', \get_class($this), $beta));
         }
 
-        static $save;
+        static $setMetadata;
 
-        $save = $save ?? \Closure::bind(
-            function (AdapterInterface $pool, ItemInterface $item, $value, float $startTime) {
-                if ($startTime && $item->expiry > $endTime = microtime(true)) {
+        $setMetadata = $setMetadata ?? \Closure::bind(
+            function (AdapterInterface $pool, ItemInterface $item, float $startTime) {
+                if ($item->expiry > $endTime = microtime(true)) {
                     $item->newMetadata[ItemInterface::METADATA_EXPIRY] = $item->expiry;
                     $item->newMetadata[ItemInterface::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime);
                 }
-                $pool->save($item->set($value));
-
-                return $value;
             },
             null,
             CacheItem::class
         );
 
-        return $this->contractsGet($pool, $key, function (CacheItem $item) use ($pool, $callback, $save) {
+        return $this->contractsGet($pool, $key, function (CacheItem $item, bool &$save) use ($pool, $callback, $setMetadata) {
             // don't wrap nor save recursive calls
             if (null === $callbackWrapper = $this->callbackWrapper) {
-                return $callback($item);
+                $value = $callback($item, $save);
+                $save = false;
+
+                return $value;
             }
             $this->callbackWrapper = null;
-            $t = microtime(true);
+            $startTime = microtime(true);
 
             try {
-                return $save($pool, $item, $callbackWrapper($item, $callback, $pool), $t);
+                $value = $callbackWrapper($callback, $item, $save, $pool);
+                $setMetadata($pool, $item, $startTime);
+
+                return $value;
             } finally {
                 $this->callbackWrapper = $callbackWrapper;
             }

diff --git a/src/Symfony/Contracts/Cache/CacheInterface.php b/src/Symfony/Contracts/Cache/CacheInterface.php
@@ -29,13 +29,13 @@ interface CacheInterface
      * requested key, that could be used e.g. for expiration control. It could also
      * be an ItemInterface instance when its additional features are needed.
      *
-     * @param string                             $key      The key of the item to retrieve from the cache
-     * @param callable(CacheItemInterface):mixed $callback Should return the computed value for the given key/item
-     * @param float|null                         $beta     A float that, as it grows, controls the likeliness of triggering
-     *                                                     early expiration. 0 disables it, INF forces immediate expiration.
-     *                                                     The default (or providing null) is implementation dependent but should
-     *                                                     typically be 1.0, which should provide optimal stampede protection.
-     *                                                     See https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration
+     * @param string                     $key      The key of the item to retrieve from the cache
+     * @param callable|CallbackInterface $callback Should return the computed value for the given key/item
+     * @param float|null                 $beta     A float that, as it grows, controls the likeliness of triggering
+     *                                             early expiration. 0 disables it, INF forces immediate expiration.
+     *                                             The default (or providing null) is implementation dependent but should
+     *                                             typically be 1.0, which should provide optimal stampede protection.
+     *                                             See https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration
      *
      * @return mixed The value corresponding to the provided key
      *

diff --git a/src/Symfony/Contracts/Cache/CacheTrait.php b/src/Symfony/Contracts/Cache/CacheTrait.php
@@ -59,7 +59,11 @@ private function doGet(CacheItemPoolInterface $pool, string $key, callable $call
         }
 
         if ($recompute) {
-            $pool->save($item->set($callback($item)));
+            $save = true;
+            $item->set($callback($item, $save));
+            if ($save) {
+                $pool->save($item);
+            }
         }
 
         return $item->get();

diff --git a/src/Symfony/Contracts/Cache/CallbackInterface.php b/src/Symfony/Contracts/Cache/CallbackInterface.php
@@ -0,0 +1,30 @@
+<?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\Contracts\Cache;
+
+use Psr\Cache\CacheItemInterface;
+
+/**
+ * Computes and returns the cached value of an item.
+ *
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+interface CallbackInterface
+{
+    /**
+     * @param CacheItemInterface|ItemInterface $item  The item to compute the value for
+     * @param bool                             &$save Should be set to false when the value should not be saved in the pool
+     *
+     * @return mixed The computed value for the passed item
+     */
+    public function __invoke(CacheItemInterface $item, bool &$save);
+}
diff --git a/src/Symfony/Contracts/Cache/TagAwareCacheInterface.php b/src/Symfony/Contracts/Cache/TagAwareCacheInterface.php
@@ -22,8 +22,6 @@ interface TagAwareCacheInterface extends CacheInterface
 {
        /**
      * {@inheritdoc}
-     *
-     * @param callable(ItemInterface):mixed $callback Should return the computed value for the given key/item
      */
     public function get(string $key, callable $callback, float $beta = null);
-Original file line number
+Diff line change
@@ Expand Up / @@ -22,8 +22,6 @@ interface TagAwareCacheInterface extends CacheInterface @@
     {
         /**
          * {@inheritdoc}
-         *
-         * @param callable(ItemInterface):mixed $callback Should return the computed value for the given key/item
          */
         public function get(string $key, callable $callback, float $beta = null);
@@ Expand Down @@