Fixed race condition in MessageCache::replace
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 20 May 2015 02:34:20 +0000 (19:34 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 4 Jun 2015 20:53:37 +0000 (13:53 -0700)
* The cache has to reload and *after* locking to avoid
  losing any concurrent changes.
* Also fixed incorrect assumption in MessageCacheTest.
  Message overrides for the content language do not use
  the language suffix.

Change-Id: I98ff158a1575330bc59efe6badb27f8de8717951

includes/cache/MessageCache.php
tests/phpunit/includes/cache/MessageCacheTest.php

index 31ee487..86b073c 100644 (file)
@@ -50,6 +50,8 @@ define( 'MSG_WAIT_TIMEOUT', 30 );
  * @ingroup Cache
  */
 class MessageCache {
+       const FOR_UPDATE = 1; // force message reload
+
        /**
         * Process local cache of loaded messages that are defined in
         * MediaWiki namespace. First array level is a language code,
@@ -236,10 +238,11 @@ class MessageCache {
         * is disabled.
         *
         * @param bool|string $code Language to which load messages
+        * @param integer $mode Use MessageCache::FOR_UPDATE to skip process cache
         * @throws MWException
         * @return bool
         */
-       function load( $code = false ) {
+       function load( $code = false, $mode = null ) {
                global $wgUseLocalMessageCache;
 
                if ( !is_string( $code ) ) {
@@ -250,7 +253,7 @@ class MessageCache {
                }
 
                # Don't do double loading...
-               if ( isset( $this->mLoadedLanguages[$code] ) ) {
+               if ( isset( $this->mLoadedLanguages[$code] ) && $mode != self::FOR_UPDATE ) {
                        return true;
                }
 
@@ -275,7 +278,6 @@ class MessageCache {
                # Hash of the contents is stored in memcache, to detect if local cache goes
                # out of date (e.g. due to replace() on some other server)
                if ( $wgUseLocalMessageCache ) {
-
                        $hash = $this->mMemc->get( wfMemcKey( 'messages', $code, 'hash' ) );
                        if ( $hash ) {
                                $cache = $this->getLocalCache( $hash, $code );
@@ -431,6 +433,7 @@ class MessageCache {
         */
        function loadFromDB( $code ) {
                global $wgMaxMsgCacheEntrySize, $wgLanguageCode, $wgAdaptiveMessageCache;
+
                $dbr = wfGetDB( DB_SLAVE );
                $cache = array();
 
@@ -514,18 +517,17 @@ class MessageCache {
         * @param mixed $text New contents of the page.
         */
        public function replace( $title, $text ) {
-               global $wgMaxMsgCacheEntrySize;
+               global $wgMaxMsgCacheEntrySize, $wgContLang;
 
                if ( $this->mDisable ) {
-
                        return;
                }
 
                list( $msg, $code ) = $this->figureMessage( $title );
 
                $cacheKey = wfMemcKey( 'messages', $code );
-               $this->load( $code );
                $this->lock( $cacheKey );
+               $this->load( $code, self::FOR_UPDATE );
 
                $titleKey = wfMemcKey( 'messages', 'individual', $title );
 
@@ -561,7 +563,6 @@ class MessageCache {
                }
 
                // Update the message in the message blob store
-               global $wgContLang;
                $blobStore = new MessageBlobStore();
                $blobStore->updateMessage( $wgContLang->lcfirst( $msg ) );
 
index 442e9f9..9c59e65 100644 (file)
@@ -52,7 +52,7 @@ class MessageCacheTest extends MediaWikiLangTestCase {
                $this->makePage( 'MessageCacheTest-FullKeyTest', 'ru' );
 
                // In content language -- get base if no derivative
-               $this->makePage( 'FallbackLanguageTest-NoDervContLang', 'de', 'de/none', false );
+               $this->makePage( 'FallbackLanguageTest-NoDervContLang', 'de', 'de/none' );
        }
 
        /**
@@ -61,15 +61,14 @@ class MessageCacheTest extends MediaWikiLangTestCase {
         * @param string $title Title of page to be created
         * @param string $lang Language and content of the created page
         * @param string|null $content Content of the created page, or null for a generic string
-        * @param bool $createSubPage Set to false if a root page should be created
         */
-       protected function makePage( $title, $lang, $content = null, $createSubPage = true ) {
+       protected function makePage( $title, $lang, $content = null ) {
                global $wgContLang;
 
                if ( $content === null ) {
                        $content = $lang;
                }
-               if ( $lang !== $wgContLang->getCode() || $createSubPage ) {
+               if ( $lang !== $wgContLang->getCode() ) {
                        $title = "$title/$lang";
                }