Skip to content

Commit

Permalink
Fix profiling context calculation
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
rvogel committed Apr 29, 2022
1 parent 376da33 commit 2236c92
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
8 changes: 7 additions & 1 deletion includes/Parse.php
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,13 @@ public function parse( $input, Parser $parser, &$reset, &$eliminate, $isParserTa
$query = new Query( $this->parameters );

$foundRows = null;
$rows = $query->buildAndSelect( $calcRows, $foundRows );
$profilingContext = '';
$currentTitle= $parser->getTitle();
if ( $currentTitle instanceof Title ) {

This comment has been minimized.

Copy link
@osnard

osnard Apr 29, 2022

Member

No idea why Parser::getTitle is defined to never return null. I have followed the path:

  1. MediaWikiServices::getParserFactory
  2. ParserFactory::__construct
  3. ParserFactory::create
  4. Parser::__construct

Parser::setTitle is nowhere called. Neither is Parser::mTitle initialised somewhere else in this callstack. Therefore there must be cases of when Parser::getTitle returns null.

$profilingContext
= str_replace( [ '*', '/' ], '-', $currentTitle->getPrefixedDBkey() );
}
$rows = $query->buildAndSelect( $calcRows, $foundRows, $profilingContext );
if ( $rows === false ) {
// This error path is very fast (We exit immediately if poolcounter is full)
// Thus it should be safe to try again in ~5 minutes.
Expand Down
11 changes: 6 additions & 5 deletions includes/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,10 @@ public function __construct( Parameters $parameters ) {
*
* @param bool $calcRows
* @param ?int &$foundRows
* @param string $profilingContext Used to see the origin of a query in the profiling
* @return array|bool
*/
public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null ) {
public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null, $profilingContext = '' ) {
global $wgNonincludableNamespaces, $wgDebugDumpSql;

$options = [];
Expand Down Expand Up @@ -363,10 +364,10 @@ public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null
$options['MAX_EXECUTION_TIME'] = $maxQueryTime;
}

$parser = MediaWikiServices::getInstance()->getParser();
$pageName = str_replace( [ '*', '/' ], '-', $parser->getTitle()->getPrefixedDBkey() );

$qname = __METHOD__ . ' - ' . $pageName;
$qname = __METHOD__;
if ( !empty( $profilingContext ) ) {
$qname .= ' - ' . $profilingContext;
}
$where = $this->where;
$join = $this->join;
$db = $this->dbr;
Expand Down

0 comments on commit 2236c92

Please sign in to comment.