From 2a4dfa169bd42eb7c68761155d44c450bcdc1423 Mon Sep 17 00:00:00 2001 From: Matthias Mullie Date: Tue, 14 Jan 2014 11:27:58 +0100 Subject: [PATCH] Don't use complex datatypes as CAS tokens For caches where CAS is not natively supported, we have a workaround, where the CAS token is (based on) the stored value. To confirm the data can be written to cache, the CAS token is compared against "whatever is currently in cache", so we pull the cached data and rebuild the value. In the case of objects, we have now pulled & rebuilt (unserialized) 2 objects that are actually the same object (for CAS purpose - it's the correct value to overwrite), but in terms of ===, they're 2 different values. This patch should make sure CAS tokens are always a serialized version of the value we're saving (where no native CAS exists); these serialized versions can reliably be compared. Bug: 59941 Change-Id: I2760416c48f2ceb7a0e0c874dd70ec07b4dccdfc --- includes/objectcache/DBABagOStuff.php | 4 +--- includes/objectcache/HashBagOStuff.php | 4 ++-- includes/objectcache/RedisBagOStuff.php | 7 ++++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/includes/objectcache/DBABagOStuff.php b/includes/objectcache/DBABagOStuff.php index c82b3aa484..a81b5c5bc6 100644 --- a/includes/objectcache/DBABagOStuff.php +++ b/includes/objectcache/DBABagOStuff.php @@ -125,6 +125,7 @@ class DBABagOStuff extends BagOStuff { } $val = dba_fetch( $key, $handle ); + $casToken = $val; list( $val, $expiry ) = $this->decode( $val ); # Must close ASAP because locks are held @@ -139,8 +140,6 @@ class DBABagOStuff extends BagOStuff { $val = false; } - $casToken = $val; - wfProfileOut( __METHOD__ ); return $val; @@ -193,7 +192,6 @@ class DBABagOStuff extends BagOStuff { // DBA is locked to any other write connection, so we can safely // compare the current & previous value before saving new value $val = dba_fetch( $key, $handle ); - list( $val, $exptime ) = $this->decode( $val ); if ( $casToken !== $val ) { dba_close( $handle ); wfProfileOut( __METHOD__ ); diff --git a/includes/objectcache/HashBagOStuff.php b/includes/objectcache/HashBagOStuff.php index d061eff0ec..bc5167d7d3 100644 --- a/includes/objectcache/HashBagOStuff.php +++ b/includes/objectcache/HashBagOStuff.php @@ -64,7 +64,7 @@ class HashBagOStuff extends BagOStuff { return false; } - $casToken = $this->bag[$key][0]; + $casToken = serialize( $this->bag[$key][0] ); return $this->bag[$key][0]; } @@ -88,7 +88,7 @@ class HashBagOStuff extends BagOStuff { * @return bool */ function cas( $casToken, $key, $value, $exptime = 0 ) { - if ( $this->get( $key ) === $casToken ) { + if ( serialize( $this->get( $key ) ) === $casToken ) { return $this->set( $key, $value, $exptime ); } diff --git a/includes/objectcache/RedisBagOStuff.php b/includes/objectcache/RedisBagOStuff.php index adcf762068..3c97480f86 100644 --- a/includes/objectcache/RedisBagOStuff.php +++ b/includes/objectcache/RedisBagOStuff.php @@ -79,12 +79,13 @@ class RedisBagOStuff extends BagOStuff { return false; } try { - $result = $this->unserialize( $conn->get( $key ) ); + $value = $conn->get( $key ); + $casToken = $value; + $result = $this->unserialize( $value ); } catch ( RedisException $e ) { $result = false; $this->handleException( $server, $conn, $e ); } - $casToken = $result; $this->logRequest( 'get', $key, $server, $result ); return $result; @@ -125,7 +126,7 @@ class RedisBagOStuff extends BagOStuff { try { $conn->watch( $key ); - if ( $this->get( $key ) !== $casToken ) { + if ( $this->serialize( $this->get( $key ) ) !== $casToken ) { $conn->unwatch(); return false; } -- 2.20.1