Page MenuHomePhabricator

Unable to overwrite services using MediaWikiServices hook
Open, HighPublic

Description

docs/hooks.txt says

'MediaWikiServices': Called when a global MediaWikiServices instance is
initialized. Extensions may use this to define, replace, or wrap services.
However, the preferred way to define a new service is
the $wgServiceWiringFiles array.
$services: MediaWikiServices

However, if I try to do redefine existing service, it does not work. With some debugging I get this:

New container is constructed as 675773727
Service is defined in 675773727
New container is constructed as 974431285
Service is defined in 974431285
My hook on MediaWikiServices is called on 974431285
Service is redefined on 974431285
ServiceContainer::importWiring is ran on 974431285
Wrong (original) service is returned on 974431285

Looking at the code:

	/**
	 * Imports all wiring defined in $container. Wiring defined in $container
	 * will override any wiring already defined locally. However, already
	 * existing service instances will be preserved.
	 *
	 * @since 1.28
	 *
	 * @param ServiceContainer $container
	 * @param string[] $skip A list of service names to skip during import
	 */
	public function importWiring( ServiceContainer $container, $skip = [] ) {
		$newInstantiators = array_diff_key(
			$container->serviceInstantiators,
			array_flip( $skip )
		);

		error_log( "importWiring is ran on " . $this->id );

		$this->serviceInstantiators = array_merge(
			$this->serviceInstantiators,
			$newInstantiators
		);
	}

For array merge:

If the input arrays have the same string keys, then the later value for that key will overwrite the previous one. If, however, the arrays contain numeric keys, the later value will not overwrite the original value, but will be appended.

Based on the documentation, I think the parameters for array_merge are in wrong order.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 14 2016, 9:25 PM
daniel triaged this task as High priority.Dec 14 2016, 11:12 PM
daniel added a project: User-Daniel.

if you are right, that would be bad :) I need to check and think, though. There is a misunderstanding somewhere. Maybe in my head :)

Nemo_bis updated the task description. (Show Details)Dec 22 2016, 8:11 AM
Nemo_bis added a subscriber: Nemo_bis.
daniel moved this task from Inbox to To Do on the User-Daniel board.Jan 5 2017, 4:08 PM
Tgr added a subscriber: Tgr.May 21 2017, 4:24 PM
Tgr added a comment.EditedMay 21 2017, 4:35 PM

MediaWikiServices::importWiring does what it says in the docs. I think the problem is that the first version of the container is set up in ExtensionRegistry::loadFromQueue before hooks have been exported. MediaWikiServices::resetGlobalServices then replaces it with a new object, and even though the MediaWikiServices hook does get called for that new object, the updated service definition gets overwritten by the old one that is copied over from the early container.

Tgr added a comment.May 22 2017, 12:57 AM

So in general the order of things is:

  • extensions which do not use extension registration add their MediaWikiServices hooks
  • loadFromQueue runs, container gets created, those hooks run
  • extensions which use extension registration add their hooks
  • resetGlobalServices runs, all hooks run but those redefining a service have no effect since old service definitions override new ones.

Also, in theory some extension entry point could get the container singleton and thus trigger hook execution, somewhere halfway in the process of LocalSettings.php executing the non-extension-registration entry points.

I ran into this issue with GlobalPreferences, and am working around it by instantiating the service just after redefinition:

	public static function onMediaWikiServices( MediaWikiServices $services ) {
		$services->redefineService(
			'PreferencesFactory',
			function ( MediaWikiServices $services ) {
				return new GlobalPreferencesFactory();
			}
		);
		// Now instantiate the new Preferences.
		$services->getPreferencesFactory();
	}
MaxSem added a subscriber: MaxSem.May 1 2018, 9:43 PM

Change 430012 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/core@master] Don't initialize MediaWikiServices before extensions have been loaded

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

Change 430012 merged by jenkins-bot:
[mediawiki/core@master] Don't initialize MediaWikiServices before extensions have been loaded

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