Avoid self-deadlocks in MessageCache::replace()
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 21 Aug 2015 21:46:44 +0000 (14:46 -0700)
committerOri.livneh <ori@wikimedia.org>
Tue, 25 Aug 2015 20:04:19 +0000 (20:04 +0000)
* This makes use of the $rclass flag in BagOStuff

Bug: T109183
Change-Id: I305f51e744aac53876e5865f860c282aa2efbd8b

includes/cache/MessageCache.php

index f11a648..9aac37a 100644 (file)
@@ -346,7 +346,7 @@ class MessageCache {
                                                # Wait for the other thread to finish, then retry. Normally,
                                                # the memcached get() will then yeild the other thread's result.
                                                $where[] = 'waited for other thread to complete';
-                                               $this->mMemc->getScopedLock( $cacheKey, self::WAIT_SEC, self::LOCK_TTL );
+                                               $this->getReentrantScopedLock( $cacheKey );
                                        }
                                }
                        }
@@ -400,7 +400,7 @@ class MessageCache {
                # If this lock fails, it doesn't really matter, it just means the
                # write is potentially non-atomic, e.g. the results of a replace()
                # may be discarded.
-               $mainUnlocker = $memCache->getScopedLock( $cacheKey, self::WAIT_SEC, self::LOCK_TTL );
+               $mainUnlocker = $this->getReentrantScopedLock( $cacheKey );
                if ( !$mainUnlocker ) {
                        $where[] = 'could not acquire main lock';
                }
@@ -548,8 +548,13 @@ class MessageCache {
                        return;
                }
 
+               # Note that if the cache is volatile, load() may trigger a DB fetch.
+               # In that case we reenter/reuse the existing cache key lock to avoid
+               # a self-deadlock. This is safe as no reads happen *directly* in this
+               # method between getReentrantScopedLock() and load() below. There is
+               # no risk of data "changing under our feet" for replace().
                $cacheKey = wfMemcKey( 'messages', $code );
-               $scopedLock = $this->mMemc->getScopedLock( $cacheKey, self::WAIT_SEC, self::LOCK_TTL );
+               $scopedLock = $this->getReentrantScopedLock( $cacheKey );
                $this->load( $code, self::FOR_UPDATE );
 
                $titleKey = wfMemcKey( 'messages', 'individual', $title );
@@ -673,6 +678,14 @@ class MessageCache {
                );
        }
 
+       /**
+        * @param string $key A language message cache key that stores blobs
+        * @return null|ScopedCallback
+        */
+       protected function getReentrantScopedLock( $key ) {
+               return $this->mMemc->getScopedLock( $key, self::WAIT_SEC, self::LOCK_TTL, __METHOD__ );
+       }
+
        /**
         * Get a message from either the content language or the user language.
         *