Merge "Avoid user autocreation race condition caused by repeatable read"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 9 Sep 2016 00:28:52 +0000 (00:28 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 9 Sep 2016 00:28:52 +0000 (00:28 +0000)
includes/auth/AuthManager.php
tests/phpunit/includes/auth/AuthManagerTest.php

index 992e70f..89a22f8 100644 (file)
@@ -1679,14 +1679,12 @@ class AuthManager implements LoggerAwareInterface {
                try {
                        $status = $user->addToDatabase();
                        if ( !$status->isOk() ) {
-                               // double-check for a race condition (T70012)
-                               $localId = User::idFromName( $username, User::READ_LATEST );
-                               if ( $localId ) {
+                               // Double-check for a race condition (T70012). We make use of the fact that when
+                               // addToDatabase fails due to the user already existing, the user object gets loaded.
+                               if ( $user->getId() ) {
                                        $this->logger->info( __METHOD__ . ': {username} already exists locally (race)', [
                                                'username' => $username,
                                        ] );
-                                       $user->setId( $localId );
-                                       $user->loadFromId( User::READ_LATEST );
                                        if ( $login ) {
                                                $this->setSessionDataForUser( $user );
                                        }
index 788d304..f679f63 100644 (file)
@@ -2709,9 +2709,11 @@ class AuthManagerTest extends \MediaWikiTestCase {
                $session->clear();
                $user = $this->getMock( 'User', [ 'addToDatabase' ] );
                $user->expects( $this->once() )->method( 'addToDatabase' )
-                       ->will( $this->returnCallback( function () use ( $username ) {
-                               $status = \User::newFromName( $username )->addToDatabase();
+                       ->will( $this->returnCallback( function () use ( $username, &$user ) {
+                               $oldUser = \User::newFromName( $username );
+                               $status = $oldUser->addToDatabase();
                                $this->assertTrue( $status->isOK(), 'sanity check' );
+                               $user->setId( $oldUser->getId() );
                                return \Status::newFatal( 'userexists' );
                        } ) );
                $user->setName( $username );