Page MenuHomePhabricator

PHP Warning: Recursion detected in RequestContext
Closed, ResolvedPublic8 Story Points

Description

As seen in Logstash under mediawiki-errors for "Recursion detected", via https://logstash.wikimedia.org/goto/415180e45258ab2afdf73972a796d9fe.

Since MediaWiki 1.31.0-wmf.6, on at least ru.wikipedia.org and zh.wikipedia.org:

[{exception_id}] {exception_url} ErrorException from line 319 of /srv/mediawiki/php-1.32.0-wmf.13/includes/context/RequestContext.php: PHP Warning: Recursion detected in RequestContext::getLanguage

Recursion detected:
#0 /srv/mediawiki/php-1.32.0-wmf.13/includes/Message.php(380): RequestContext->getLanguage()
#1 /srv/mediawiki/php-1.32.0-wmf.13/includes/Message.php(1297): Message->getLanguage()
#2 /srv/mediawiki/php-1.32.0-wmf.13/includes/Message.php(849): Message->fetchMessage()
#3 /srv/mediawiki/php-1.32.0-wmf.13/includes/Message.php(953): Message->toString()
#4 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(1888): Message->plain()
#5 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(2274): User->getBlockedStatus()
#6 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(2264): User->getBlock()
#7 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(1370): User->isBlocked()
#8 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(443): User->loadFromSession()
#9 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(5456): User->load()
#10 /srv/mediawiki/php-1.32.0-wmf.13/includes/user/User.php(3146): User->loadOptions()
#11 /srv/mediawiki/php-1.32.0-wmf.13/includes/context/RequestContext.php(336): User->getOption()
#12 /srv/mediawiki/php-1.32.0-wmf.13/includes/StubObject.php(206): RequestContext->getLanguage()
#13 /srv/mediawiki/php-1.32.0-wmf.13/includes/StubObject.php(168): StubUserLang->_newObject()
#14 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/ParserOptions.php(956): StubObject->_unstub()
#15 /srv/mediawiki/php-1.32.0-wmf.13/extensions/TextExtracts/includes/ApiQueryExtracts.php(231): ParserOptions->__construct()
#16 /srv/mediawiki/php-1.32.0-wmf.13/extensions/TextExtracts/includes/ApiQueryExtracts.php(180): TextExtracts\ApiQueryExtracts->parse()
#17 /srv/mediawiki/php-1.32.0-wmf.13/extensions/TextExtracts/includes/ApiQueryExtracts.php(117): TextExtracts\ApiQueryExtracts->getExtract()
#18 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiQuery.php(249): TextExtracts\ApiQueryExtracts->execute()
#19 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiMain.php(1582): ApiQuery->execute()
#20 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiMain.php(535): ApiMain->executeAction()
#21 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiMain.php(506): ApiMain->executeActionWithErrorHandling()
#22 /srv/mediawiki/php-1.32.0-wmf.13/api.php(83): ApiMain->execute()
#23 /srv/mediawiki/w/api.php(3): include()
#24 {main}

Developer notes

From Max:
A quick solution would be to stick DeferredStringifier into block reason and pray that nothing relies too much on it being actual string. A more thorough one would be to refactor the crap from a lot of places: make Block internals hidden and accessible only via getters/setters, then add an option to initialize block reasons with messages that get fetched only when needed.

Event Timeline

Restricted Application added projects: Discovery, Discovery-Search. · View Herald TranscriptNov 8 2017, 5:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
debt added a subscriber: debt.

This doesn't appear to be anything with Discovery-Search as we are asking for a global mediawiki object for the language, and it's been in place since 2015.

The relevant cirrus code here hasn't really changed since at least 2015. The stack trace seems a bit odd, i don't know this path much but it seems asking for the request language shouldn't require rendering i18n messages.

MaxSem added a subscriber: MaxSem.

So, basically, what's going on is:

  • Something needs user language
  • User information is loaded to get their language
  • Because cookie blocks are enabled, User::loadFromSession() retrieves block information
  • When there is a block not caused by something that an admin leaves a block message for (in this particular case, XFF but there are other similar cases around in User::getBlockedStatus()), we initialize the block message with an interface message:
$block->mReason = wfMessage( 'xffblockreason', $block->mReason )->text();
  • Message needs to know user language
  • Infinite loop!

A quick solution would be to stick DeferredStringifier into block reason and pray that nothing relies too much on it being actual string. A more thorough one would be to refactor the crap from a lot of places: make Block internals hidden and accessible only via getters/setters, then add an option to initialize block reasons with messages that get fetched only when needed.

Anomie added a subscriber: Anomie.Nov 9 2017, 3:41 PM

The proximate cause here seems to be rMW1cc3a57296ff: Send a cookie with autoblocks to prevent vandalism., which added attempted checking of the blocked status into User::loadFromSession().

Nikerabbit added a subscriber: Nikerabbit.

There is no Language-Team. Besides, the cause has been diagnosed above and i18n is only tangentially related.

Jdforrester-WMF added a subscriber: Jdforrester-WMF.

No justification in this task that this should block the release of MediaWiki 1.31.

Krinkle renamed this task from [Russian] [Chinese] PHP Warning: Recursion detected in RequestContext::getLanguage MW v1.31.0-wmf.6 to [Russian] [Chinese] PHP Warning: Recursion detected in RequestContext.Jul 31 2018, 6:24 PM
Krinkle updated the task description. (Show Details)
Restricted Application added a subscriber: Cosine02. · View Herald TranscriptJul 31 2018, 6:24 PM
Krinkle moved this task from Unsorted to Needs research on the Technical-Debt board.EditedJul 31 2018, 6:26 PM
Krinkle added a subscriber: Krinkle.

All instances of this error seem to have in common that they are from /w/api.php requesting extracts.

Marking as production impact because it can cause incorrect results to be returned, given the user's language will not be considered during the request and likely other parser options may be incorrect as well, possibly causing cache poisoning.

pmiazga added a subscriber: pmiazga.Aug 1 2018, 4:50 PM

I think that should try to apply the first proposed solution (use the DeferredStringifier) and hope it fixes the problem.

ovasileva triaged this task as Normal priority.Aug 6 2018, 10:53 AM
pmiazga claimed this task.Aug 6 2018, 2:34 PM
Jdlrobson updated the task description. (Show Details)Aug 9 2018, 9:43 PM
Jdlrobson set the point value for this task to 8.Aug 14 2018, 4:07 PM
Jdlrobson lowered the priority of this task from Normal to Low.Nov 15 2018, 7:00 PM
Jdlrobson added a subscriber: Jdlrobson.

reflecting reality.

debt removed a subscriber: debt.Nov 28 2018, 7:14 PM
Krinkle renamed this task from [Russian] [Chinese] PHP Warning: Recursion detected in RequestContext to [Russian] [Chinese] PHP Warning: Recursion detected in RequestContext in TextExtracts API.Jan 22 2019, 1:53 AM
MaxSem raised the priority of this task from Low to High.Jun 26 2019, 5:58 AM

Due to BlockManager changes, this has generated 256 errors in the last 15 minutes. Needs fixed ASAP. Also, removing irrelevant tags not related to the actual problem. It's neither restricted to ru: and zh:, nor does it have TextExtracts in its stacktraces.

Moral of the story: we should take care of our tech debt before it bites us in the ass.

MaxSem renamed this task from [Russian] [Chinese] PHP Warning: Recursion detected in RequestContext in TextExtracts API to PHP Warning: Recursion detected in RequestContext in TextExtracts API.Jun 26 2019, 5:59 AM

Change 519177 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/core@master] WIP: defer cookie block checks to resolve a circular dependency

https://gerrit.wikimedia.org/r/519177

MaxSem renamed this task from PHP Warning: Recursion detected in RequestContext in TextExtracts API to PHP Warning: Recursion detected in RequestContext.Jun 26 2019, 7:02 AM
Restricted Application added a subscriber: MGChecker. · View Herald TranscriptJun 27 2019, 10:06 AM

Change 519177 merged by jenkins-bot:
[mediawiki/core@master] Defer cookie block checks to resolve a circular dependency

https://gerrit.wikimedia.org/r/519177

Does https://gerrit.wikimedia.org/r/519177 fix this? I'm still catching up on a bunch of things, so am curious what the current state of this is..

@Jdlrobson Yes that should fix this - it hasn't been deployed yet, which is why the errors are still being logged.

Jdlrobson closed this task as Resolved.Jul 15 2019, 6:18 PM

These appear to have tailed off. \o/

mmodell changed the subtype of this task from "Task" to "Production Error".Wed, Aug 28, 11:09 PM