User:Werdna/CentralAuth investigation

I'm looking into it. In my own testing, CentralAuthUser got into an infinite loop when browsing as a non-anonymous unmerged account. Debug log said this:

Global User: cache miss for UnmergedUser, version , expected 1
Loading state for global user UnmergedUser from DB
LoadBalancer::openForeignConnection: reusing connection 0/centralauth
Global User: cache miss for UnmergedUser, version , expected 1
Loading state for global user UnmergedUser from DB
...

until I manually killed the php-cgi process.

I sent the backtrace off to the debug log, and got the following extract:
CentralAuthUser.php line 139 calls wfBacktrace()
CentralAuthUser.php line 266 calls CentralAuthUser::loadState()
CentralAuthUser.php line 289 calls CentralAuthUser::getCacheObject()
CentralAuthUser.php line 158 calls CentralAuthUser::saveToCache()
CentralAuthUser.php line 266 calls CentralAuthUser::loadState()
CentralAuthUser.php line 289 calls CentralAuthUser::getCacheObject()
CentralAuthUser.php line 158 calls CentralAuthUser::saveToCache()
CentralAuthUser.php line 266 calls CentralAuthUser::loadState()

It seems that, in getCacheObject, it wants to run loadState again. Which is all fine and dandy if loadState knows it's already been called, but it doesn't seem to know that. It checks this using the following lines:

		} elseif ( isset( $this->mGlobalId ) ) {
			// Already loaded
			return;
		}

So, obviously, mGlobalId is not being set properly. But it should be set by this line:

$this->mGlobalId = 0; // executed if we can't find a row in loadState();

When I tried dumping the whole object to the debug log before and after this line, I found this:

BEFORE: CentralAuthUser Object
(
    [mName] => UnmergedUser
    [mStateDirty] => 
    [mVersion] => 1
    [mDelayInvalidation] => 0
    [mWasAnon] => 
    [mIsAttached] => 
)
AFTER: CentralAuthUser Object
(
    [mName] => UnmergedUser
    [mStateDirty] => 
    [mVersion] => 1
    [mDelayInvalidation] => 0
    [mWasAnon] => 
    [mIsAttached] => 
    [mGlobalId] => 0
)

Unusual, since mGlobalId should have been set by the previous error. I looked around for anywhere that this might be unset, and the only candidate function was resetState(). So, I added a debug line there with a backtrace to see where and why this was happening.

Resetting state, BACKTRACE:
CentralAuthUser.php line 93 calls wfBacktrace()
CentralAuthUser.php line 294 calls CentralAuthUser::resetState()
CentralAuthUser.php line 160 calls CentralAuthUser::saveToCache()

Deducing from the code, it calls resetState() if mFromMaster is not set to true. Unfortunately, if a row is not found, then the following essential line isn't executed:

$this->mFromMaster = $fromMaster;

And we end up in an infinite loop :-(

Patch to solve this problem

Index: CentralAuthUser.php
===================================================================
--- CentralAuthUser.php (revision 34386)
+++ CentralAuthUser.php (working copy)
@@ -211,6 +211,7 @@
                } else {
                        $this->mGlobalId = 0;
                        $this->mIsAttached = false;
+                       $this->mFromMaster = $fromMaster;
                }
        }