From 861683b7221bba90a3fe3270dd3073f106ce5f6e Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 26 Mar 2019 17:05:48 -0700
Subject: [PATCH] objectcache: fix race condition in SqlBagOStuff::changeTTL

Also fix --use-bagostuff parameter when given 0

Change-Id: I64971d4ccad2c856cc7688577ae564c8db068819
---
 includes/objectcache/SqlBagOStuff.php         | 19 ++++++++++++++++---
 .../libs/objectcache/BagOStuffTest.php        |  2 +-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index e450212dc6..33b12334aa 100644
--- 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 3d8c9cbb0b..e9b3d62b63 100644
--- 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 );
-- 
2.20.1