Fatal error: "Call to a member function clear() on null" from TemplateStylesHooks
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Assigned To

Authored By

	Krinkle
	Dec 5 2019, 12:13 AM

Description

Error

MediaWiki version: 1.35.0-wmf.5

message

Fatal error: Call to a member function clear() on null
in /srv/mediawiki/php-1.35.0-wmf.5/extensions/TemplateStyles/includes/TemplateStylesHooks.php:271

exception.trace

#0 /srv/mediawiki/php-1.35.0-wmf.5/includes/Hooks.php(174): TemplateStylesHooks::onParserClearState(Parser)
#1 /srv/mediawiki/php-1.35.0-wmf.5/includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL)
#2 /srv/mediawiki/php-1.35.0-wmf.5/includes/parser/Parser.php(518): Hooks::run(string, array)
#3 /srv/mediawiki/php-1.35.0-wmf.5/includes/parser/Parser.php(4848): Parser->clearState()
#4 /srv/mediawiki/php-1.35.0-wmf.5/includes/parser/Parser.php(863): Parser->startParse(Title, ParserOptions, integer, boolean)
#5 /srv/mediawiki/php-1.35.0-wmf.5/includes/parser/Parser.php(4874): Parser->preprocess(string, Title, ParserOptions)
#6 /srv/mediawiki/php-1.35.0-wmf.5/includes/cache/MessageCache.php(1206): Parser->transformMsg(string, ParserOptions, Title)
#7 /srv/mediawiki/php-1.35.0-wmf.5/includes/language/Message.php(1284): MessageCache->transform(string, boolean, Language, Title)
#8 /srv/mediawiki/php-1.35.0-wmf.5/includes/language/Message.php(888): Message->transformText(string)
#9 /srv/mediawiki/php-1.35.0-wmf.5/includes/language/Message.php(948): Message->toString(string)
#10 /srv/mediawiki/php-1.35.0-wmf.5/includes/skins/Skin.php(547): Message->text()
#11 /srv/mediawiki/php-1.35.0-wmf.5/includes/skins/Skin.php(619): Skin->getCategoryLinks()
#12 /srv/mediawiki/php-1.35.0-wmf.5/includes/skins/SkinTemplate.php(291): Skin->getCategories()
#13 /srv/mediawiki/php-1.35.0-wmf.5/skins/MinervaNeue/includes/skins/SkinMinerva.php(150): SkinTemplate->prepareQuickTemplate()
#14 /srv/mediawiki/php-1.35.0-wmf.5/includes/skins/SkinTemplate.php(215): SkinMinerva->prepareQuickTemplate()
#15 /srv/mediawiki/php-1.35.0-wmf.5/includes/OutputPage.php(2626): SkinTemplate->outputPage()
#16 /srv/mediawiki/php-1.35.0-wmf.5/includes/exception/MWExceptionRenderer.php(150): OutputPage->output()
#17 /srv/mediawiki/php-1.35.0-wmf.5/includes/exception/MWExceptionRenderer.php(64): MWExceptionRenderer::reportHTML(WMFTimeoutException)
#18 /srv/mediawiki/php-1.35.0-wmf.5/includes/exception/MWExceptionHandler.php(103): MWExceptionRenderer::output(WMFTimeoutException, integer)
#19 /srv/mediawiki/php-1.35.0-wmf.5/includes/exception/MWExceptionHandler.php(177): MWExceptionHandler::report(WMFTimeoutException)
#20 /srv/mediawiki/php-1.35.0-wmf.5/includes/MediaWiki.php(563): MWExceptionHandler::handleException(WMFTimeoutException)
#21 /srv/mediawiki/php-1.35.0-wmf.5/index.php(46): MediaWiki->run()
#22 /srv/mediawiki/w/index.php(3): require(string)

Impact

This fatal error from TemplateStylesHooks is causing some pages to not be viewable due to the rendering step failing to get past this hook.

The specific trace above is from a cases where the bug is triggered whilst rendering the "fatal error" page. Which meant the fatal error page about the other fatal error also couldn't be rendered, thus the system will have had to fall back to a blank page that Varnish replaces with a generic system error / HTTP 500.

Notes

Source of TemplateStylesHooks.php

	public static function onParserClearState( Parser $parser ) {
		/** @phan-suppress-next-line PhanUndeclaredProperty */
		$parser->extTemplateStylesCache->clear();
	}

Details

Request ID: XehDbApAICwAABH8TTEAAAWJ
Request URL: https://vi.m.wikipedia.org/wiki/Chu%E1%BB%99t_t%C3%BAi_Wallaby

Event Timeline

Krinkle created this task.Dec 5 2019, 12:13 AM

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald TranscriptDec 5 2019, 12:13 AM

How is it managing to not create the member via the 'ParserFirstCallInit' hook? If nothing else, $parser->firstCallInit() is called at the start of clearState().

In T239878#5715823, @Anomie wrote:

How is it managing to not create the member via the 'ParserFirstCallInit' hook? If nothing else, $parser->firstCallInit() is called at the start of clearState().

Yes, although AFAICS firstCallInit can return early if it's not the first call. Maybe the property is correctly created on the first call, but then deleted before clearState() is called?

It was pretty obvious, but I'd like to point out that for the same request we also have

PHP Notice: Undefined property: Parser::$extTemplateStylesCache

In T239878#5718055, @Daimona wrote:

In T239878#5715823, @Anomie wrote:

How is it managing to not create the member via the 'ParserFirstCallInit' hook? If nothing else, $parser->firstCallInit() is called at the start of clearState().

Yes, although AFAICS firstCallInit can return early if it's not the first call. Maybe the property is correctly created on the first call, but then deleted before clearState() is called?

Still, how would either of those things happen?

In T239878#5718391, @Anomie wrote:

Still, how would either of those things happen?

Heh, this is the hard part. I see various paths:

$this->mFirstCall is set by something other than firstCallInit; specifically, something that does not trigger the ParserFirstCallInit hook
firstCallInit is correctly called, but it exits in the middle; specifically, after setting mFirstCall, but before calling the hook (e.g. if an exception is thrown, and the caller catches it) [1]
Something is unsetting the property; Parser::__destruct does unset all properties, but then it's legit to think that the instance doesn't exist anymore. Unless something is calling __destruct manually, but that doesn't seem to be the case.

Then there are exotic possibilities, like:

The ParserFirstCallInit hook not being called/registered (although TemplateStyles registers it correctly)
Some mysterious PHP bug

[1] - In this regard, is there a reason not to have $this->mFirstCall = false; at the bottom of firstCallInit?

In T239878#5718491, @Daimona wrote:

Heh, this is the hard part. I see various paths:

$this->mFirstCall is set by something other than firstCallInit; specifically, something that does not trigger the ParserFirstCallInit hook

https://codesearch.wmflabs.org/search/?q=mFirstCall&i=nope&files=&repos= suggests "no"

firstCallInit is correctly called, but it exits in the middle; specifically, after setting mFirstCall, but before calling the hook (e.g. if an exception is thrown, and the caller catches it) [1]

[1] - In this regard, is there a reason not to have $this->mFirstCall = false; at the bottom of firstCallInit?

That might be a good idea. Such an exception seems likely to break other things too.

We might also change the Hooks::run() to Hooks::runWithoutAbort() for the same reason.

In T239878#5718612, @Anomie wrote:

In T239878#5718491, @Daimona wrote:

Heh, this is the hard part. I see various paths:

$this->mFirstCall is set by something other than firstCallInit; specifically, something that does not trigger the ParserFirstCallInit hook

https://codesearch.wmflabs.org/search/?q=mFirstCall&i=nope&files=&repos= suggests "no"

Heh, yes. And codesearch also excludes __destruct calls, as I forgot to explicitly mention in my previous comment.

firstCallInit is correctly called, but it exits in the middle; specifically, after setting mFirstCall, but before calling the hook (e.g. if an exception is thrown, and the caller catches it) [1]

[1] - In this regard, is there a reason not to have $this->mFirstCall = false; at the bottom of firstCallInit?

That might be a good idea. Such an exception seems likely to break other things too.

We might also change the Hooks::run() to Hooks::runWithoutAbort() for the same reason.

I note that the order was changed in this commit, back in 2008. I wonder whether there'd be any side effect if the methods inside firstCallInit are called twice.

So a caught exception seems the most likely scenario.

At any rate, it'd be good to know what is throwing an exception, and what is catching it. We could wrap the body of firstCallInit in a try{} catch{ log; throw } to discover that (in case it's hiding some serious issue). Do we have consistent repro steps?

LGoto moved this task from Needs triage to Tracking on the Product-Infrastructure-Team-Backlog-Deprecated board.Dec 11 2019, 4:42 PM