Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/257012
Change subject: Fix get()/getMulti() check key race condition in WANObjectCache
......................................................................
Fix get()/getMulti() check key race condition in WANObjectCache
If a set() happened around the exact same time as check key was
initialized via get(), the curTTL in future get() calls could
sometimes be 0 instead of a negative value, even though hold-off
should apply.
Change-Id: Ide1fd65112aff425a4798e2ec406d71f2a8e84a7
---
M includes/libs/objectcache/WANObjectCache.php
M tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
2 files changed, 38 insertions(+), 1 deletion(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/12/257012/1
diff --git a/includes/libs/objectcache/WANObjectCache.php
b/includes/libs/objectcache/WANObjectCache.php
index 4005abb..95bf743 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -251,6 +251,7 @@
// Fetch all of the raw values
$wrappedValues = $this->cache->getMulti( array_merge(
$valueKeys, $checkKeysFlat ) );
+ // Time used to compare/init "check" keys (derived after
getMulti() to be pessimistic)
$now = microtime( true );
// Collect timestamps from all "check" keys
@@ -282,7 +283,10 @@
foreach ( $purgeValues as $purge ) {
$safeTimestamp = $purge[self::FLD_TIME]
+ $purge[self::FLD_HOLDOFF];
if ( $safeTimestamp >=
$wrappedValues[$vKey][self::FLD_TIME] ) {
- $curTTL = min( $curTTL,
$purge[self::FLD_TIME] - $now );
+ // How long ago this value was
expired by *this* check key
+ $ago = min(
$purge[self::FLD_TIME] - $now, self::TINY_NEGATIVE );
+ // How long ago this value was
expired by *any* known check key
+ $curTTL = min( $curTTL, $ago );
}
}
}
diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
index 1511557..d7babf1 100644
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -366,6 +366,39 @@
}
/**
+ * @covers WANObjectCache::get()
+ * @covers WANObjectCache::processCheckKeys()
+ */
+ public function testCheckKeyInitHoldoff() {
+ $cache = $this->cache;
+
+ for ( $i = 0; $i < 500; ++$i ) {
+ $key = wfRandomString();
+ $checkKey = wfRandomString();
+ // miss, set, hit
+ $cache->get( $key, $curTTL, array( $checkKey ) );
+ $cache->set( $key, 'val', 10 );
+ $curTTL = null;
+ $v = $cache->get( $key, $curTTL, array( $checkKey ) );
+
+ $this->assertEquals( 'val', $v );
+ $this->assertLessThan( 0, $curTTL, "Step $i: CTL < 0
(miss/set/hit)" );
+ }
+
+ for ( $i = 0; $i < 500; ++$i ) {
+ $key = wfRandomString();
+ $checkKey = wfRandomString();
+ // set, hit
+ $cache->set( $key, 'val', 10 );
+ $curTTL = null;
+ $v = $cache->get( $key, $curTTL, array( $checkKey ) );
+
+ $this->assertEquals( 'val', $v );
+ $this->assertLessThan( 0, $curTTL, "Step $i: CTL < 0
(set/hit)" );
+ }
+ }
+
+ /**
* @covers WANObjectCache::delete()
*/
public function testDelete() {
--
To view, visit https://gerrit.wikimedia.org/r/257012
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide1fd65112aff425a4798e2ec406d71f2a8e84a7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
