Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DB profiling code breaks on runtime #170

Open
osnard opened this issue Apr 26, 2022 · 1 comment
Open

DB profiling code breaks on runtime #170

osnard opened this issue Apr 26, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@osnard
Copy link
Contributor

osnard commented Apr 26, 2022 •
edited
Loading

DynamicPageList3 version: 3.4.5
MediaWiki version: 1.35.6
PHP version: 7.4.28

List of steps to reproduce
TBH, not sure about how to produce. It was reported to me and I just wanted to share my findings here.

What happens?:
In some cases (sorry, I don't have more info right now) an exception is thrown:
imageimage

I have investigated the code and came across https://github.com/Universal-Omega/DynamicPageList3/blob/3.4.5/includes/Query.php#L366-L367

Apparently a new Parser object is being created using MediaWikiServices::getParser. ParserFactory::create is invoked, which does not set a Title object. In such cases Parser::getTitle can actually return null (https://github.com/wikimedia/mediawiki/blob/1.35.6/includes/parser/Parser.php#L1006-L1008)

But the code does not check for null.

It looks like these actions are only performed to create some marker for DB profiling. Should I provide a patch that removes/alters the code?
@osnard osnard added the bug Something isn't working label Apr 26, 2022
@Universal-Omega
Copy link
Owner

Should I provide a patch that removes/alters the code?

Definitely welcome, if not I can in a little bit also.

osnard pushed a commit to hallowelt/DynamicPageList3 that referenced this issue Apr 29, 2022
Fix profiling context calculation
2236c92 
`ParserFactory::getParser` will create a completely new `Parser` instance.
Based on the context, this instance may have not set a `Title` object.
Thus accessing `Parser::getTitle` without checking for the return value
may break the code.

Issue: Universal-Omega#170
@osnard osnard mentioned this issue Apr 29, 2022
Merged
osnard pushed a commit to hallowelt/mediawiki that referenced this issue Apr 29, 2022
Add fixes for DPL3
30ff0b2 
See Universal-Omega/DynamicPageList3#170

[4.1.x]

ERM28444
@osnard osnard mentioned this issue Apr 29, 2022
Merged
osnard pushed a commit to hallowelt/mediawiki that referenced this issue Apr 29, 2022
Add fixes for DPL3
839185d 
See Universal-Omega/DynamicPageList3#170

[4.1.x]

ERM28444
@osnard osnard mentioned this issue Apr 29, 2022
Merged
it-spiderman pushed a commit to hallowelt/mediawiki that referenced this issue May 30, 2022
@it-spiderman
Add fixes for DPL3
fac8358 
See Universal-Omega/DynamicPageList3#170

[4.1.x]

ERM28444
it-spiderman pushed a commit to hallowelt/mediawiki that referenced this issue May 30, 2022
@it-spiderman
Add fixes for DPL3
11d9533 
See Universal-Omega/DynamicPageList3#170

[4.1.x]

ERM28444
Universal-Omega pushed a commit that referenced this issue Jun 24, 2022
@osnard
Fix profiling context calculation (#174)
64cf6c5 
`ParserFactory::getParser` will create a completely new `Parser` instance.
Based on the context, this instance may have not set a `Title` object.
Thus accessing `Parser::getTitle` without checking for the return value
may break the code.

Issue: #170

Co-authored-by: rvogel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@osnard @Universal-Omega