Don't use complex datatypes as CAS tokens
authorMatthias Mullie <git@mullie.eu>
Tue, 14 Jan 2014 10:27:58 +0000 (11:27 +0100)
committerMatthias Mullie <git@mullie.eu>
Tue, 14 Jan 2014 15:45:32 +0000 (16:45 +0100)
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
includes/objectcache/HashBagOStuff.php
includes/objectcache/RedisBagOStuff.php

index c82b3aa..a81b5c5 100644 (file)
@@ -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__ );
index d061eff..bc5167d 100644 (file)
@@ -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 );
                }
 
index adcf762..3c97480 100644 (file)
@@ -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;
                        }