Matthias Mullie has uploaded a new change for review. https://gerrit.wikimedia.org/r/107348
Change subject: Don't use complex datatypes as CAS tokens ...................................................................... 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 --- M includes/objectcache/DBABagOStuff.php M includes/objectcache/HashBagOStuff.php M includes/objectcache/RedisBagOStuff.php 3 files changed, 7 insertions(+), 8 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/48/107348/1 diff --git a/includes/objectcache/DBABagOStuff.php b/includes/objectcache/DBABagOStuff.php index c82b3aa..a81b5c5 100644 --- a/includes/objectcache/DBABagOStuff.php +++ b/includes/objectcache/DBABagOStuff.php @@ -125,6 +125,7 @@ } $val = dba_fetch( $key, $handle ); + $casToken = $val; list( $val, $expiry ) = $this->decode( $val ); # Must close ASAP because locks are held @@ -138,8 +139,6 @@ wfDebug( __METHOD__ . ": $key expired\n" ); $val = false; } - - $casToken = $val; wfProfileOut( __METHOD__ ); @@ -193,7 +192,6 @@ // 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 d061eff..bc5167d 100644 --- a/includes/objectcache/HashBagOStuff.php +++ b/includes/objectcache/HashBagOStuff.php @@ -64,7 +64,7 @@ return false; } - $casToken = $this->bag[$key][0]; + $casToken = serialize( $this->bag[$key][0] ); return $this->bag[$key][0]; } @@ -88,7 +88,7 @@ * @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 adcf762..3c97480 100644 --- a/includes/objectcache/RedisBagOStuff.php +++ b/includes/objectcache/RedisBagOStuff.php @@ -79,12 +79,13 @@ 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 @@ try { $conn->watch( $key ); - if ( $this->get( $key ) !== $casToken ) { + if ( $this->serialize( $this->get( $key ) ) !== $casToken ) { $conn->unwatch(); return false; } -- To view, visit https://gerrit.wikimedia.org/r/107348 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2760416c48f2ceb7a0e0c874dd70ec07b4dccdfc Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: wmf/1.23wmf10 Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits