objectcache: fix race condition in SqlBagOStuff::changeTTL

Also fix --use-bagostuff parameter when given 0

Change-Id: I64971d4ccad2c856cc7688577ae564c8db068819

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index e450212..33b1233 100644 (file)
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -534,20 +534,33 @@ class SqlBagOStuff extends BagOStuff {
                return $ok;
        }
 
-       public function changeTTL( $key, $expiry = 0, $flags = 0 ) {
+       public function changeTTL( $key, $exptime = 0, $flags = 0 ) {
                list( $serverIndex, $tableName ) = $this->getTableByKey( $key );
                $db = null;
                $silenceScope = $this->silenceTransactionProfiler();
                try {
                        $db = $this->getDB( $serverIndex );
+                       if ( $exptime == 0 ) {
+                               $timestamp = $this->getMaxDateTime( $db );
+                       } else {
+                               $timestamp = $db->timestamp( $this->convertToExpiry( $exptime ) );
+                       }
                        $db->update(
                                $tableName,
-                               [ 'exptime' => $db->timestamp( $this->convertToExpiry( $expiry ) ) ],
+                               [ 'exptime' => $timestamp ],
                                [ 'keyname' => $key, 'exptime > ' . $db->addQuotes( $db->timestamp( time() ) ) ],
                                __METHOD__
                        );
                        if ( $db->affectedRows() == 0 ) {
-                               return false;
+                               $exists = (bool)$db->selectField(
+                                       $tableName,
+                                       1,
+                                       [ 'keyname' => $key, 'exptime' => $timestamp ],
+                                       __METHOD__,
+                                       [ 'FOR UPDATE' ]
+                               );
+
+                               return $exists;
                        }
                } catch ( DBError $e ) {
                        $this->handleWriteError( $e, $db, $serverIndex );

diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
index 3d8c9cb..e9b3d62 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
@@ -16,7 +16,7 @@ class BagOStuffTest extends MediaWikiTestCase {
                parent::setUp();
 
                // type defined through parameter
-               if ( $this->getCliArg( 'use-bagostuff' ) ) {
+               if ( $this->getCliArg( 'use-bagostuff' ) !== null ) {
                        $name = $this->getCliArg( 'use-bagostuff' );
 
                        $this->cache = ObjectCache::newFromId( $name );