MultiWriteBagOStuff: Fix async writes of mutable objects

If someone writes an object into a BagOStuff, they typically expect that
later changes to the object will not affect the value stored.
MultiWriteBagOStuff's async write handling was violating this
expectation, which is potentially causing T168040.

Bug: T168040
Change-Id: Ie897b900befdc8998614af06f9339cd07665703e

diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php
index 687c67c..d94578d 100644 (file)
--- a/includes/libs/objectcache/MultiWriteBagOStuff.php
+++ b/includes/libs/objectcache/MultiWriteBagOStuff.php
@@ -181,6 +181,12 @@ class MultiWriteBagOStuff extends BagOStuff {
                $ret = true;
                $args = array_slice( func_get_args(), 3 );
 
+               if ( $count > 1 && $asyncWrites ) {
+                       // Deep-clone $args to prevent misbehavior when something writes an
+                       // object to the BagOStuff then modifies it afterwards, e.g. T168040.
+                       $args = unserialize( serialize( $args ) );
+               }
+
                foreach ( $this->caches as $i => $cache ) {
                        if ( $i >= $count ) {
                                break; // ignore the lower tiers

diff --git a/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php
index 775709f..4a9f6cc 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php
@@ -81,22 +81,26 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
         */
        public function testSetDelayed() {
                $key = wfRandomString();
-               $value = wfRandomString();
+               $value = (object)[ 'v' => wfRandomString() ];
+               $expectValue = clone $value;
 
                // XXX: DeferredUpdates bound to transactions in CLI mode
                $dbw = wfGetDB( DB_MASTER );
                $dbw->begin();
                $this->cache->set( $key, $value );
 
+               // Test that later changes to $value don't affect the saved value (e.g. T168040)
+               $value->v = 'bogus';
+
                // Set in tier 1
-               $this->assertEquals( $value, $this->cache1->get( $key ), 'Written to tier 1' );
+               $this->assertEquals( $expectValue, $this->cache1->get( $key ), 'Written to tier 1' );
                // Not yet set in tier 2
                $this->assertEquals( false, $this->cache2->get( $key ), 'Not written to tier 2' );
 
                $dbw->commit();
 
                // Set in tier 2
-               $this->assertEquals( $value, $this->cache2->get( $key ), 'Written to tier 2' );
+               $this->assertEquals( $expectValue, $this->cache2->get( $key ), 'Written to tier 2' );
        }
 
        /**