Avoid self-deadlocks in MessageCache::replace()

[lhc/web/wiklou.git] / includes / cache / MessageCache.php

diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php
index 7945c8b..9aac37a 100644 (file)
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@ -33,17 +33,6 @@ define( 'MSG_CACHE_VERSION', 2 );
  */
 define( 'MSG_LOAD_TIMEOUT', 60 );
 
-/**
- * Memcached timeout when locking a key for a writing operation.
- * See MessageCache::lock()
- */
-define( 'MSG_LOCK_TIMEOUT', 30 );
-/**
- * Number of times we will try to acquire a lock from Memcached.
- * This comes in addition to MSG_LOCK_TIMEOUT.
- */
-define( 'MSG_WAIT_TIMEOUT', 30 );
-
 /**
  * Message cache
  * Performs various MediaWiki namespace-related functions
@@ -52,6 +41,11 @@ define( 'MSG_WAIT_TIMEOUT', 30 );
 class MessageCache {
        const FOR_UPDATE = 1; // force message reload
 
+       /** How long memcached locks last */
+       const WAIT_SEC = 30;
+       /** How long to wait for memcached locks */
+       const LOCK_TTL = 30;
+
        /**
         * Process local cache of loaded messages that are defined in
         * MediaWiki namespace. First array level is a language code,
@@ -166,7 +160,7 @@ class MessageCache {
                $this->mExpiry = $expiry;
 
                if ( $wgUseLocalMessageCache ) {
-                       $this->localCache = ObjectCache::newAccelerator( array(), CACHE_NONE );
+                       $this->localCache = ObjectCache::newAccelerator( CACHE_NONE );
                } else {
                        $this->localCache = wfGetCache( CACHE_NONE );
                }
@@ -349,10 +343,10 @@ class MessageCache {
                                                # Disable cache
                                                break;
                                        } else {
-                                               # Wait for the other thread to finish, then retry
+                                               # 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->lock( $cacheKey );
-                                               $this->unlock( $cacheKey );
+                                               $this->getReentrantScopedLock( $cacheKey );
                                        }
                                }
                        }
@@ -385,6 +379,9 @@ class MessageCache {
 
                $memCache = $this->mMemc;
 
+               # Get the non-blocking status key lock. This lets the caller quickly know
+               # to use any stale cache lying around. Otherwise, it may do a blocking
+               # lock to try to obtain the messages.
                $statusKey = wfMemcKey( 'messages', $code, 'status' );
                if ( !$memCache->add( $statusKey, 'loading', MSG_LOAD_TIMEOUT ) ) {
                        return false; // could not acquire lock
@@ -403,13 +400,8 @@ 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.
-               if ( $this->lock( $cacheKey ) ) {
-                       $that = $this;
-                       $mainUnlocker = new ScopedCallback( function () use ( $that, $cacheKey ) {
-                               $that->unlock( $cacheKey );
-                       } );
-               } else {
-                       $mainUnlocker = null;
+               $mainUnlocker = $this->getReentrantScopedLock( $cacheKey );
+               if ( !$mainUnlocker ) {
                        $where[] = 'could not acquire main lock';
                }
 
@@ -556,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 );
-               $this->lock( $cacheKey );
+               $scopedLock = $this->getReentrantScopedLock( $cacheKey );
                $this->load( $code, self::FOR_UPDATE );
 
                $titleKey = wfMemcKey( 'messages', 'individual', $title );
@@ -577,7 +574,7 @@ class MessageCache {
 
                # Update caches
                $this->saveToCaches( $this->mCache[$code], 'all', $code );
-               $this->unlock( $cacheKey );
+               ScopedCallback::consume( $scopedLock );
                $this->wanCache->touchCheckKey( wfMemcKey( 'messages', $code ) );
 
                // Also delete cached sidebar... just in case it is affected
@@ -682,40 +679,11 @@ class MessageCache {
        }
 
        /**
-        * Represents a write lock on the messages key.
-        *
-        * Will retry MessageCache::MSG_WAIT_TIMEOUT times, each operations having
-        * a timeout of MessageCache::MSG_LOCK_TIMEOUT.
-        *
-        * @param string $key
-        * @return bool Success
+        * @param string $key A language message cache key that stores blobs
+        * @return null|ScopedCallback
         */
-       function lock( $key ) {
-               $lockKey = $key . ':lock';
-               $acquired = false;
-               $testDone = false;
-               for ( $i = 0; $i < MSG_WAIT_TIMEOUT && !$acquired; $i++ ) {
-                       $acquired = $this->mMemc->add( $lockKey, 1, MSG_LOCK_TIMEOUT );
-                       if ( $acquired ) {
-                               break;
-                       }
-
-                       # Fail fast if memcached is totally down
-                       if ( !$testDone ) {
-                               $testDone = true;
-                               if ( !$this->mMemc->set( wfMemcKey( 'test' ), 'test', 1 ) ) {
-                                       break;
-                               }
-                       }
-                       sleep( 1 );
-               }
-
-               return $acquired;
-       }
-
-       function unlock( $key ) {
-               $lockKey = $key . ':lock';
-               $this->mMemc->delete( $lockKey );
+       protected function getReentrantScopedLock( $key ) {
+               return $this->mMemc->getScopedLock( $key, self::WAIT_SEC, self::LOCK_TTL, __METHOD__ );
        }
 
        /**