MC-32427: Sign locks in order to prevent unlock from parallel request

Author: vzabaznov

app/code/Magento/Config/App/Config/Type/System.php

@@ -20,6 +20,8 @@
 use Magento\Store\Model\Config\Processor\Fallback;
 use Magento\Framework\Encryption\Encryptor;
 use Magento\Store\Model\ScopeInterface as StoreScope;
+use Magento\Framework\App\Cache\StateInterface;
+use Magento\Framework\App\Cache\Type\Config;
 
 /**
  * System configuration type
@@ -98,6 +100,12 @@ class System implements ConfigTypeInterface
     private $lockQuery;
 
     /**
+     * @var StateInterface
+     */
+    private $cacheState;
+
+    /**
+     * System constructor.
      * @param ConfigSourceInterface $source
      * @param PostProcessorInterface $postProcessor
      * @param Fallback $fallback
@@ -110,6 +118,7 @@ class System implements ConfigTypeInterface
      * @param Encryptor|null $encryptor
      * @param LockManagerInterface|null $locker
      * @param LockGuardedCacheLoader|null $lockQuery
+     * @param StateInterface|null $cacheState
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
@@ -125,7 +134,8 @@ public function __construct(
         Reader $reader = null,
         Encryptor $encryptor = null,
         LockManagerInterface $locker = null,
-        LockGuardedCacheLoader $lockQuery = null
+        LockGuardedCacheLoader $lockQuery = null,
+        StateInterface $cacheState = null
     ) {
         $this->postProcessor = $postProcessor;
         $this->cache = $cache;
@@ -136,6 +146,8 @@ public function __construct(
             ?: ObjectManager::getInstance()->get(Encryptor::class);
         $this->lockQuery = $lockQuery
             ?: ObjectManager::getInstance()->get(LockGuardedCacheLoader::class);
+        $this->cacheState = $cacheState
+            ?: ObjectManager::getInstance()->get(StateInterface::class);
     }
 
     /**
@@ -220,6 +232,10 @@ private function getWithParts($path)
      */
     private function loadAllData()
     {
+        if (!$this->cacheState->isEnabled(Config::TYPE_IDENTIFIER)) {
+            return $this->readData();
+        }
+
         $loadAction = function () {
             $cachedData = $this->cache->load($this->configType);
             $data = false;
@@ -245,6 +261,10 @@ private function loadAllData()
      */
     private function loadDefaultScopeData($scopeType)
     {
+        if (!$this->cacheState->isEnabled(Config::TYPE_IDENTIFIER)) {
+            return $this->readData();
+        }
+
         $loadAction = function () use ($scopeType) {
             $cachedData = $this->cache->load($this->configType . '_' . $scopeType);
             $scopeData = false;
@@ -271,6 +291,10 @@ private function loadDefaultScopeData($scopeType)
      */
     private function loadScopeData($scopeType, $scopeId)
     {
+        if (!$this->cacheState->isEnabled(Config::TYPE_IDENTIFIER)) {
+            return $this->readData();
+        }
+
         $loadAction = function () use ($scopeType, $scopeId) {
             $cachedData = $this->cache->load($this->configType . '_' . $scopeType . '_' . $scopeId);
             $scopeData = false;
@@ -393,6 +417,10 @@ public function clean()
             $this->cache->clean(\Zend_Cache::CLEANING_MODE_MATCHING_TAG, [self::CACHE_TAG]);
         };
 
+        if (!$this->cacheState->isEnabled(Config::TYPE_IDENTIFIER)) {
+            return $cleanAction();
+        }
+
         $this->lockQuery->lockedCleanData(
             self::$lockName,
             $cleanAction

lib/internal/Magento/Framework/App/Cache/Type/AccessProxy.php

@@ -80,7 +80,7 @@ public function load($identifier)
     public function save($data, $identifier, array $tags = [], $lifeTime = null)
     {
         if (!$this->_isEnabled()) {
-            return true;
+            return false;
         }
         return parent::save($data, $identifier, $tags, $lifeTime);
     }

lib/internal/Magento/Framework/Cache/LockGuardedCacheLoader.php

@@ -91,7 +91,7 @@ public function lockedLoadData(
         callable $dataSaver
     ) {
         $cachedData = $dataLoader(); //optimistic read
-        $deadline = microtime(true) + $this->loadTimeout;
+        $deadline = microtime(true) + $this->loadTimeout / 100;
 
         while ($cachedData === false) {
             if ($deadline <= microtime(true)) {
@@ -129,13 +129,8 @@ public function lockedCleanData(string $lockName, callable $dataCleaner)
         while ($this->locker->isLocked($lockName)) {
             usleep($this->getLookupTimeout() * 1000);
         }
-        try {
-            if ($this->locker->lock($lockName, $this->lockTimeout / 1000)) {
-                $dataCleaner();
-            }
-        } finally {
-            $this->locker->unlock($lockName);
-        }
+
+        $dataCleaner();
     }
 
     /**
@@ -147,8 +142,6 @@ public function lockedCleanData(string $lockName, callable $dataCleaner)
      */
     private function getLookupTimeout()
     {
-        $lookupTimeout = rand($this->minimalDelayTimeout, $this->delayTimeout);
-
-        return $lookupTimeout;
+        return rand($this->minimalDelayTimeout, $this->delayTimeout);
     }
 }

lib/internal/Magento/Framework/Lock/Backend/Cache.php

@@ -55,11 +55,11 @@ public function lock(string $name, int $timeout = -1): bool
              return false;
         }
 
-        $saveResult = $this->cache->save($this->lockSign, $this->getIdentifier($name), [], $timeout * 100);
+        $this->cache->save($this->lockSign, $this->getIdentifier($name), [], $timeout * 100);
 
         $data = $this->cache->load($this->getIdentifier($name));
 
-        if ($data === $this->lockSign && $saveResult) {
+        if ($data === $this->lockSign) {
             return true;
         }
 
@@ -125,7 +125,7 @@ private function generateLockSign()
         try {
             $sign .= '-' . \bin2hex(\random_bytes(4));
         } catch (\Exception $e) {
-            $sign .= '-' . \rand(0, 1000);
+            $sign .= '-' . \mt_rand(0, 1000);
         }
 
         return $sign;