Page MenuHomePhabricator
Create Task
Maniphest T310948

PageImages ParserAfterTidy hook re-enters parser via CommonsMetadata (on Miraheze)
Closed, ResolvedPublic
Actions

Description

It was reported at T300311 that page views such as https://tampvan.miraheze.org/wiki/Benua_Greget are failing with an exception. The backtrace shows an exception thrown from Parser::lock() due to the PageImages ParserAfterTidy handler calling FormatMetadata::fetchExtendedMetadata(), then the CommonsMetadata GetExtendedMetadata handler calls LocalFile::getDescriptionText() which calls the Parser via ContentHandler.

My patch https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageImages/+/743062 is likely the proximate cause since it moved the fetchExtendedMetadata() from LinksUpdate to ParserAfterTidy.

[7361a92e0f43dd1138a5bf26] /wiki/Dunia_Minecraft MWException: Parser state cleared while parsing. Did you call Parser::parse recursively? Lock is held by: #0 /srv/mediawiki/w/includes/parser/Parser.php(673): Parser->lock()
#1 /srv/mediawiki/w/includes/content/WikitextContentHandler.php(294): Parser->parse('{{Halal22}}\n{{M...', Object(Title), Object(ParserOptions), true, true, 14776)
#2 /srv/mediawiki/w/includes/content/ContentHandler.php(1705): WikitextContentHandler->fillParserOutput(Object(WikitextContent), Object(MediaWiki\Content\Renderer\ContentParseParams), Object(ParserOutput))
#3 /srv/mediawiki/w/includes/content/Renderer/ContentRenderer.php(47): ContentHandler->getParserOutput(Object(WikitextContent), Object(MediaWiki\Content\Renderer\ContentParseParams))
#4 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(267): MediaWiki\Content\Renderer\ContentRenderer->getParserOutput(Object(WikitextContent), Object(Title), 14776, Object(ParserOptions), true)
#5 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(238): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(Object(WikitextContent), true)
#6 /srv/mediawiki/w/includes/Revision/RevisionRenderer.php(221): MediaWiki\Revision\RenderedRevision->getSlotParserOutput('main', Array)
#7 /srv/mediawiki/w/includes/Revision/RevisionRenderer.php(158): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(Object(MediaWiki\Revision\RenderedRevision), Array)
#8 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(Object(MediaWiki\Revision\RenderedRevision), Array)
#9 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(200): call_user_func(Object(Closure), Object(MediaWiki\Revision\RenderedRevision), Array)
#10 /srv/mediawiki/w/includes/poolcounter/PoolWorkArticleView.php(137): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#11 /srv/mediawiki/w/includes/poolcounter/PoolCounterWork.php(162): PoolWorkArticleView->doWork()
#12 /srv/mediawiki/w/includes/page/ParserOutputAccess.php(302): PoolCounterWork->execute()
#13 /srv/mediawiki/w/includes/page/Article.php(702): MediaWiki\Page\ParserOutputAccess->getParserOutput(Object(WikiPage), Object(ParserOptions), Object(MediaWiki\Revision\RevisionStoreCacheRecord), 5)
#14 /srv/mediawiki/w/includes/page/Article.php(517): Article->generateContentOutput(Object(User), Object(ParserOptions), 0, Object(OutputPage), Array)
#15 /srv/mediawiki/w/includes/actions/ViewAction.php(80): Article->view()
#16 /srv/mediawiki/w/includes/MediaWiki.php(544): ViewAction->show()
#17 /srv/mediawiki/w/includes/MediaWiki.php(321): MediaWiki->performAction(Object(Article), Object(Title))
#18 /srv/mediawiki/w/includes/MediaWiki.php(910): MediaWiki->performRequest()
#19 /srv/mediawiki/w/includes/MediaWiki.php(564): MediaWiki->main()
#20 /srv/mediawiki/w/index.php(53): MediaWiki->run()
#21 /srv/mediawiki/w/index.php(46): wfIndexMain()
#22 {main}

Backtrace:

from /srv/mediawiki/w/includes/parser/Parser.php(6354)
#0 /srv/mediawiki/w/includes/parser/Parser.php(673): Parser->lock()
#1 /srv/mediawiki/w/extensions/Scribunto/includes/common/ScribuntoContentHandler.php(146): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#2 /srv/mediawiki/w/includes/content/ContentHandler.php(1705): ScribuntoContentHandler->fillParserOutput(ScribuntoContent, MediaWiki\Content\Renderer\ContentParseParams, ParserOutput)
#3 /srv/mediawiki/w/includes/content/Renderer/ContentRenderer.php(47): ContentHandler->getParserOutput(ScribuntoContent, MediaWiki\Content\Renderer\ContentParseParams)
#4 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(267): MediaWiki\Content\Renderer\ContentRenderer->getParserOutput(ScribuntoContent, Title, integer, ParserOptions, boolean)
#5 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(238): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(ScribuntoContent, boolean)
#6 /srv/mediawiki/w/includes/Revision/RevisionRenderer.php(221): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string, array)
#7 /srv/mediawiki/w/includes/Revision/RevisionRenderer.php(158): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#8 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#9 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(200): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#10 /srv/mediawiki/w/includes/poolcounter/PoolWorkArticleView.php(137): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#11 /srv/mediawiki/w/includes/poolcounter/PoolCounterWork.php(162): PoolWorkArticleView->doWork()
#12 /srv/mediawiki/w/includes/page/ParserOutputAccess.php(302): PoolCounterWork->execute()
#13 /srv/mediawiki/w/includes/filerepo/file/LocalFile.php(2556): MediaWiki\Page\ParserOutputAccess->getParserOutput(MediaWiki\Page\PageStoreRecord, ParserOptions)
#14 /srv/mediawiki/w/extensions/CommonsMetadata/src/DataCollector.php(258): LocalFile->getDescriptionText(LanguageEn)
#15 /srv/mediawiki/w/extensions/CommonsMetadata/src/DataCollector.php(96): CommonsMetadata\DataCollector->getDescriptionText(LocalFile, LanguageEn)
#16 /srv/mediawiki/w/extensions/CommonsMetadata/src/HookHandler.php(75): CommonsMetadata\DataCollector->collect(array, LocalFile)
#17 /srv/mediawiki/w/includes/HookContainer/HookContainer.php(338): CommonsMetadata\HookHandler::onGetExtendedMetadata(array, LocalFile, DerivativeContext, boolean, integer)
#18 /srv/mediawiki/w/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#19 /srv/mediawiki/w/includes/HookContainer/HookRunner.php(1811): MediaWiki\HookContainer\HookContainer->run(string, array)
#20 /srv/mediawiki/w/includes/media/FormatMetadata.php(1817): MediaWiki\HookContainer\HookRunner->onGetExtendedMetadata(array, LocalFile, DerivativeContext, boolean, integer)
#21 /srv/mediawiki/w/includes/media/FormatMetadata.php(1738): FormatMetadata->getExtendedMetadataFromHook(LocalFile, array, integer)
#22 /srv/mediawiki/w/extensions/PageImages/includes/Hooks/ParserFileProcessingHookHandlers.php(396): FormatMetadata->fetchExtendedMetadata(LocalFile)
#23 /srv/mediawiki/w/extensions/PageImages/includes/Hooks/ParserFileProcessingHookHandlers.php(377): PageImages\Hooks\ParserFileProcessingHookHandlers->fetchFileMetadata(LocalFile)
#24 /srv/mediawiki/w/extensions/PageImages/includes/Hooks/ParserFileProcessingHookHandlers.php(223): PageImages\Hooks\ParserFileProcessingHookHandlers->isImageFree(string)
#25 /srv/mediawiki/w/extensions/PageImages/includes/Hooks/ParserFileProcessingHookHandlers.php(169): PageImages\Hooks\ParserFileProcessingHookHandlers->findBestImages(array)
#26 /srv/mediawiki/w/extensions/PageImages/includes/Hooks/ParserFileProcessingHookHandlers.php(66): PageImages\Hooks\ParserFileProcessingHookHandlers->doParserAfterTidy(Parser, string)
#27 /srv/mediawiki/w/includes/HookContainer/HookContainer.php(338): PageImages\Hooks\ParserFileProcessingHookHandlers::onParserAfterTidy(Parser, string)
#28 /srv/mediawiki/w/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#29 /srv/mediawiki/w/includes/HookContainer/HookRunner.php(2828): MediaWiki\HookContainer\HookContainer->run(string, array)
#30 /srv/mediawiki/w/includes/parser/Parser.php(1721): MediaWiki\HookContainer\HookRunner->onParserAfterTidy(Parser, string)
#31 /srv/mediawiki/w/includes/parser/Parser.php(700): Parser->internalParseHalfParsed(string, boolean, boolean)
#32 /srv/mediawiki/w/includes/content/WikitextContentHandler.php(294): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#33 /srv/mediawiki/w/includes/content/ContentHandler.php(1705): WikitextContentHandler->fillParserOutput(WikitextContent, MediaWiki\Content\Renderer\ContentParseParams, ParserOutput)
#34 /srv/mediawiki/w/includes/content/Renderer/ContentRenderer.php(47): ContentHandler->getParserOutput(WikitextContent, MediaWiki\Content\Renderer\ContentParseParams)
#35 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(267): MediaWiki\Content\Renderer\ContentRenderer->getParserOutput(WikitextContent, Title, integer, ParserOptions, boolean)
#36 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(238): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(WikitextContent, boolean)
#37 /srv/mediawiki/w/includes/Revision/RevisionRenderer.php(221): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string, array)
#38 /srv/mediawiki/w/includes/Revision/RevisionRenderer.php(158): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#39 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#40 /srv/mediawiki/w/includes/Revision/RenderedRevision.php(200): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#41 /srv/mediawiki/w/includes/poolcounter/PoolWorkArticleView.php(137): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#42 /srv/mediawiki/w/includes/poolcounter/PoolCounterWork.php(162): PoolWorkArticleView->doWork()
#43 /srv/mediawiki/w/includes/page/ParserOutputAccess.php(302): PoolCounterWork->execute()
#44 /srv/mediawiki/w/includes/page/Article.php(702): MediaWiki\Page\ParserOutputAccess->getParserOutput(WikiPage, ParserOptions, MediaWiki\Revision\RevisionStoreCacheRecord, integer)
#45 /srv/mediawiki/w/includes/page/Article.php(517): Article->generateContentOutput(User, ParserOptions, integer, OutputPage, array)
#46 /srv/mediawiki/w/includes/actions/ViewAction.php(80): Article->view()
#47 /srv/mediawiki/w/includes/MediaWiki.php(544): ViewAction->show()
#48 /srv/mediawiki/w/includes/MediaWiki.php(321): MediaWiki->performAction(Article, Title)
#49 /srv/mediawiki/w/includes/MediaWiki.php(910): MediaWiki->performRequest()
#50 /srv/mediawiki/w/includes/MediaWiki.php(564): MediaWiki->main()
#51 /srv/mediawiki/w/index.php(53): MediaWiki->run()
#52 /srv/mediawiki/w/index.php(46): wfIndexMain()
#53 {main}

Details

SubjectRepoBranchLines +/-
Deprecate Parser::getFreshParser()mediawiki/coremaster+74 -27
In ScribuntoContentHandler use the new ParserFactory::getInstance()mediawiki/extensions/Scribuntomaster+2 -2
Migrate risky callers of MediaWikiServices::getParser()mediawiki/coremaster+15 -14
Customize query in gerrit

Related Objects

Event Timeline

tstarling created this task.Jun 18 2022, 11:15 PM
Restricted Application added a project: User-RhinosF1. · View Herald TranscriptJun 18 2022, 11:15 PM
Restricted Application added subscribers: Reception123, Aklapper. · View Herald Transcript
tstarling added a comment.Jun 18 2022, 11:21 PM

PageImages only needs the file metadata to determine whether an image is non-free, a minor feature which is probably not relevant for most wikis. So a temporary workaround is to disable that feature.

diff --git a/includes/Hooks/ParserFileProcessingHookHandlers.php b/includes/Hooks/ParserFileProcessingHookHandlers.php
index 8bdbd95..25f760c 100644
--- a/includes/Hooks/ParserFileProcessingHookHandlers.php
+++ b/includes/Hooks/ParserFileProcessingHookHandlers.php
@@ -370,6 +370,7 @@ class ParserFileProcessingHookHandlers {
 	 * @return bool
 	 */
 	protected function isImageFree( $fileName ) {
+		return true;
 		$file = MediaWikiServices::getInstance()->getRepoGroup()->findFile( $fileName );
 		if ( $file ) {
 			// Process copyright metadata from CommonsMetadata, if present.
ChioGaming2007 added a comment.Jun 18 2022, 11:42 PM

So what's the solution?
and what do you mean "disable that feature"
What should I remove

Universal_Omega added a comment.EditedJun 19 2022, 12:46 AM
In T310948#8013280, @ChioGaming2007 wrote:

So what's the solution?
and what do you mean "disable that feature"
What should I remove

Please let people handle it, any action that is needed from you in particular, will be mentioned to you. If you want to fix it on your wiki on Miraheze, visit Special:ManageWiki/extensions, and disable PageImages for now.

ChioGaming2007 added a comment.Jun 19 2022, 1:44 AM
In T310948#8013300, @Universal_Omega wrote:

Please let people handle it, any action that is needed from you in particular, will be mentioned to you. If you want to fix it on your wiki on Miraheze, visit Special:ManageWiki/extensions, and disable PageImages for now.

So if I turn it off will the pictures in the article become invisible

Universal_Omega added a comment.Jun 19 2022, 1:47 AM
In T310948#8013331, @ChioGaming2007 wrote:

So if I turn it off will the pictures in the article become invisible

No.

ChioGaming2007 added a comment.Jun 19 2022, 3:07 AM
In T310948#8013332, @Universal_Omega wrote:

No.

I have turned it off
But the result is still the same

Universal_Omega added a comment.Jun 19 2022, 3:15 AM
In T310948#8013344, @ChioGaming2007 wrote:
In T310948#8013332, @Universal_Omega wrote:

No.

I have turned it off
But the result is still the same

According to https://tampvan.miraheze.org/wiki/Special:Version, you still have it turned on.

RhinosF1 triaged this task as High priority.Jun 19 2022, 7:50 PM
RhinosF1 moved this task from Radar to Miraheze-Linked on the User-RhinosF1 board.

@tstarling: would a patch to allow disable with config be acceptable & suitable for backport?

If so, feel free to assign to CosmicAlpha who can take that.

RhinosF1 moved this task from Backlog to Maintenance / bugs on the PageImages board.Jun 19 2022, 7:50 PM
tstarling added a project: Scribunto.Jun 20 2022, 1:09 AM

A patch that optionally turns off the free image feature should be acceptable.

But I think my preferred solution is to call getFreshParser() at the relevant place, and I see now that the relevant place in the backtrace is actually in the Scribunto extension. This explains why we're not seeing this error at Wikimedia -- core's WikitextContentHandler already calls getFreshParser() and so is safe to call in this context. Somehow the image description page has the Lua content type.

tstarling added a project: MediaWiki-extensions-MsUpload.Jun 20 2022, 2:06 AM

I confirmed with an API query that many images on this wiki have the content model set to "Scribunto", for example Berkas:0710-4-11.jpg. The comments say they were uploaded with MsUpload. Looking at the source of MsUpload, I don't think it can be at fault, since API action=upload does not allow the client to specify the content model.

I confirmed that the default content model for NS_FILE is wikitext. I don't know how those image description pages end up getting created with their content model set to Scribunto.

tstarling added a comment.Jun 20 2022, 3:44 AM

I'm doing a patch involving getFreshParser() which won't be suitable for backport.

gerritbot added a comment.Jun 20 2022, 3:58 AM

Change 806567 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Deprecate Parser::getFreshParser()

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

gerritbot added a project: Patch-For-Review.Jun 20 2022, 3:58 AM
gerritbot added a comment.Jun 20 2022, 6:46 AM

Change 806849 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Migrate risky callers of MediaWikiServices::getParser()

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

gerritbot added a comment.Jun 20 2022, 6:53 AM

Change 806850 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/Scribunto@master] In ScribuntoContentHandler use the new ParserFactory::getInstance()

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

tstarling removed a project: MediaWiki-extensions-MsUpload.Jun 20 2022, 10:49 AM
gerritbot added a comment.Jul 5 2022, 4:53 AM

Change 806567 merged by jenkins-bot:

[mediawiki/core@master] Deprecate Parser::getFreshParser()

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

gerritbot added a comment.Jul 5 2022, 4:57 AM

Change 806849 merged by jenkins-bot:

[mediawiki/core@master] Migrate risky callers of MediaWikiServices::getParser()

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

gerritbot added a comment.Jul 5 2022, 5:16 AM

Change 806850 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] In ScribuntoContentHandler use the new ParserFactory::getInstance()

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

Maintenance_bot removed a project: Patch-For-Review.Jul 5 2022, 5:30 AM
ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.21; 2022-07-18).Jul 12 2022, 1:01 AM
tstarling closed this task as Resolved.Jul 21 2022, 6:00 AM
tstarling claimed this task.
Maintenance_bot moved this task from Miraheze-Linked to Done on the User-RhinosF1 board.Jul 21 2022, 6:29 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL