Getting the following error when running maintenance scripts such as update.php:
PHP Fatal error: Uncaught MWException: salvage() must be called immediately after construction in includes/HookContainer/HookContainer.php:94
Stack trace:
#0 includes/MediaWikiServices.php(321): MediaWiki\HookContainer\HookContainer->salvage(Object(MediaWiki\HookContainer\HookContainer))
#1 includes/MediaWikiServices.php(292): MediaWiki\MediaWikiServices->salvage(Object(MediaWiki\MediaWikiServices))
#2 includes/Setup.php(535): MediaWiki\MediaWikiServices::resetGlobalInstance(Object(GlobalVarConfig), 'quick')
#3 maintenance/doMaintenance.php(91): require_once('...')
#4 maintenance/update.php(253): require_once('...')
#5 {main}
thrown in includes/HookContainer/HookContainer.php on line 94As far as I can tell, this is caused by an extension registering a hook for MediaWikiServices using the new 1.35-style HookHandler classes. An extension.json snippet that reproduces the issue is below:
{
"AutoloadClasses": {
"MyExtensionHooks": "MyExtensionHooks.php"
},
"Hooks": {
"MediaWikiServices": "service"
},
"HookHandlers": {
"service": {
"class": "MyExtensionHooks"
}
}
}along with e.g. MyExtensionHooks.php:
<?php class MyExtensionHooks implements MediaWikiServicesHook { public function onMediaWikiServices( $services ) { } }
This seems to only break in maintenance scripts. It works fine for normal web requests and the action API. The culprit seems to be Maintenance::finalSetup() which grabs an instance of MediaWikiServices before services are finished bootstrapping in Setup.php. That function is run as a MW_SETUP_CALLBACK which happens very early on in init.
I'm not sure what the proper fix here is, hence why no gerrit patches as of yet. I'm thinking though that probably we just want HookContainer::salvage to ignore whether or not HookContainer::$handlersByName is already set. Since we can easily re-instantiate any of those handlers whenever we attempt to fetch the appropriate hooks, it should be safe to just erase any existing handlers and re-construct them later as they're needed. I think we still want to throw if any dynamic handlers are registered before salvage() is called since those can't be easily recovered.
This fix should be backported to 1.35 as well once merged.