From f6b76d3a9e6ddde4fa1f3b403b85a2c0a99d9da5 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 30 Mar 2015 13:37:21 -0700
Subject: [PATCH] Added CAS-style logic to User::saveSettings as a final sanity
 check

* This should prevent lag or race conditions from rolling back data

Change-Id: I5e70975f4e4010fea7af0801bc11dda887df55f4
---
 includes/User.php | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/includes/User.php b/includes/User.php
index 6ac320e795..3f3189d683 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -452,7 +452,7 @@ class User implements IDBAccessObject {
 		// The cache needs good consistency due to its high TTL, so the user
 		// should have been loaded from the master to avoid lag amplification.
 		if ( !( $this->queryFlagsUsed & self::READ_LATEST ) ) {
-			wfWarn( "Cannot save slave-loaded User object data to cache." );
+			wfWarn( "Cannot cache slave-loaded User object with ID '{$this->mId}'." );
 			return;
 		}
 
@@ -3598,16 +3598,21 @@ class User implements IDBAccessObject {
 		$this->load();
 		$this->loadPasswords();
 		if ( 0 == $this->mId ) {
-			return;
+			return; // anon
 		}
 
 		// This method is for updating existing users, so the user should
 		// have been loaded from the master to begin with to avoid problems.
 		if ( !( $this->queryFlagsUsed & self::READ_LATEST ) ) {
-			wfWarn( "Attempting to save slave-loaded User object data." );
+			wfWarn( "Attempting to save slave-loaded User object with ID '{$this->mId}'." );
 		}
 
+		// Get a new user_touched that is higher than the old one.
+		// This will be used for a CAS check as a last-resort safety
+		// check against race conditions and slave lag.
+		$oldTouched = $this->mTouched;
 		$this->mTouched = $this->newTouchedTimestamp();
+
 		if ( !$wgAuth->allowSetLocalPassword() ) {
 			$this->mPassword = self::getPasswordFactory()->newFromCiphertext( null );
 		}
@@ -3628,10 +3633,22 @@ class User implements IDBAccessObject {
 				'user_email_token_expires' => $dbw->timestampOrNull( $this->mEmailTokenExpires ),
 				'user_password_expires' => $dbw->timestampOrNull( $this->mPasswordExpires ),
 			), array( /* WHERE */
-				'user_id' => $this->mId
+				'user_id' => $this->mId,
+				'user_touched' => $dbw->timestamp( $oldTouched ) // CAS check
 			), __METHOD__
 		);
 
+		if ( !$dbw->affectedRows() ) {
+			// User was changed in the meantime or loaded with stale data
+			MWExceptionHandler::logException( new MWException(
+				"CAS update failed on user_touched for user ID '{$this->mId}'."
+			) );
+			// Maybe the problem was a missed cache update; clear it to be safe
+			$this->clearSharedCache();
+
+			return;
+		}
+
 		$this->saveOptions();
 
 		Hooks::run( 'UserSaveSettings', array( $this ) );
-- 
2.20.1