[go: up one dir, main page]
Page MenuHomePhabricator
Create Task
Maniphest T282320

Recursive service instantiation: Circular dependency when creating service! MainWANObjectCache
Closed, ResolvedPublic
Actions

Description

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/685544
https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-noselenium-docker/82325/console

20:17:44 1) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set "MultiLock" ('MultiLock')
20:17:44 Wikimedia\Services\RecursiveServiceDependencyException: Recursive service instantiation: Circular dependency when creating service! GrowthExperimentsMultiConfig -> GrowthExperimentsWikiPageConfig -> GrowthExperimentsWikiPageConfigLoader -> RevisionLookup -> RevisionStore -> RevisionStoreFactory -> BlobStoreFactory -> MainWANObjectCache -> UserNameUtils -> TitleParser -> _MediaWikiTitleCodec -> InterwikiLookup -> MainWANObjectCache

Core CI seems fine, so I assume this is Growth-specific, although no idea how.

Stack trace:

20:49:51 1) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set "MultiLock" ('MultiLock')
20:49:51 Wikimedia\Services\RecursiveServiceDependencyException: Recursive service instantiation: Circular dependency when creating service! GrowthExperimentsMultiConfig -> GrowthExperimentsWikiPageConfig -> GrowthExperimentsWikiPageConfigLoader -> RevisionLookup -> RevisionStore -> RevisionStoreFactory -> BlobStoreFactory -> MainWANObjectCache -> UserNameUtils -> TitleParser -> _MediaWikiTitleCodec -> InterwikiLookup -> MainWANObjectCache
20:49:51 
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:437
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1063
20:49:51 /workspace/src/includes/ServiceWiring.php:578
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:908
20:49:51 /workspace/src/includes/ServiceWiring.php:1722
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/ServiceWiring.php:1487
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1499
20:49:51 /workspace/src/includes/ServiceWiring.php:1568
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1579
20:49:51 /workspace/src/includes/user/User.php:2070
20:49:51 /workspace/src/includes/user/User.php:2938
20:49:51 /workspace/src/includes/user/User.php:2956
20:49:51 /workspace/src/includes/session/SessionBackend.php:669
20:49:51 /workspace/src/includes/session/SessionBackend.php:234
20:49:51 /workspace/src/includes/session/Session.php:73
20:49:51 /workspace/src/includes/ServiceWiring.php:810
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1063
20:49:51 /workspace/src/includes/ServiceWiring.php:210
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:611
20:49:51 /workspace/src/includes/ServiceWiring.php:1266
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1338
20:49:51 /workspace/src/includes/ServiceWiring.php:1250
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1330
20:49:51 /workspace/src/includes/ServiceWiring.php:1236
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/includes/MediaWikiServices.php:1314
20:49:51 /workspace/src/extensions/GrowthExperiments/ServiceWiring.php:479
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:424
20:49:51 /workspace/src/extensions/GrowthExperiments/includes/GrowthExperimentsServices.php:184
20:49:51 /workspace/src/extensions/GrowthExperiments/ServiceWiring.php:99
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:424
20:49:51 /workspace/src/extensions/GrowthExperiments/includes/GrowthExperimentsServices.php:180
20:49:51 /workspace/src/extensions/GrowthExperiments/ServiceWiring.php:89
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:447
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:416
20:49:51 /workspace/src/includes/MediaWikiServices.php:266
20:49:51 /workspace/src/vendor/wikimedia/services/src/ServiceContainer.php:424
20:49:51 /workspace/src/vendor/wikimedia/object-factory/src/ObjectFactory/ObjectFactory.php:209
20:49:51 /workspace/src/vendor/wikimedia/object-factory/src/ObjectFactory/ObjectFactory.php:150
20:49:51 /workspace/src/includes/HookContainer/HookContainer.php:465
20:49:51 /workspace/src/includes/HookContainer/HookContainer.php:156
20:49:51 /workspace/src/includes/HookContainer/HookRunner.php:3677
20:49:51 /workspace/src/includes/specialpage/SpecialPageFactory.php:1099
20:49:51 /workspace/src/includes/specialpage/SpecialPageFactory.php:1114
20:49:51 /workspace/src/includes/specialpage/SpecialPageFactory.php:1172
20:49:51 /workspace/src/includes/specialpage/SpecialPageFactory.php:1202
20:49:51 /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php:40
20:49:51 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:448

Details

SubjectRepoBranchLines +/-
[DNM] CI testmediawiki/extensions/GrowthExperimentsmaster+0 -0
User::getID: Be lazier when fetching UserNameUtilsmediawiki/coremaster+8 -8
[DNM] Force stack trace from Session constructormediawiki/coremaster+15 -0
Customize query in gerrit

Related Objects

Event Timeline

Tgr created this task.May 8 2021, 7:10 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 8 2021, 7:10 PM
Tgr added a comment.May 8 2021, 7:25 PM

No idea what's happening here. The error seems to imply that the service factory function for MainWANObjectCache requests UserNameUtils from the service container, but it doesn't (and there is no reason why it would). The rest of the dependencies seem legit.

Tgr added a comment.May 8 2021, 7:41 PM

MultiLock is a CentralAuth special page so probably this is some conflict with that extension (which does use UserNameUtils a lot).

Tgr added a comment.May 8 2021, 7:43 PM

Locally, there's no issue, with CentralAuth and GrowthExperiments installed.

vagrant@growth:~$ mwscript /vagrant/mediawiki/tests/phpunit/phpunit.php --wiki=wiki --filter MultiLock /vagrant/mediawiki/tests/phpunit/structure/SpecialPageFatalTest.php 
#!/usr/bin/env php
Using PHP 7.2.31-1+0~20200514.41+debian9~1.gbpe2a56b+wmf1
PHPUnit 8.5.15 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 3.49 seconds, Memory: 56.25 MB

OK (1 test, 1 assertion)
Tgr triaged this task as Unbreak Now! priority.May 9 2021, 8:25 PM
RhinosF1 subscribed.May 9 2021, 8:25 PM
Tgr added a comment.EditedMay 9 2021, 8:32 PM

Started happening some time yesterday (last good check is 0:42 UTC, first bad check is 20:25).

Tgr added a comment.May 9 2021, 8:44 PM

No merges in the relevant time window in CentralAuth. None of the core merges look relevant. rEGREb4b9beb392da: Use static closures where safe to use seems a bit suspicious (in theory it should be a no-op but it changes ServiceWiring and might trigger some obscure bug) but it's a bit too early, it was merged at 18h the day before yesterday, with several patches merged afterwards. The last successful merge was rEGRE2f2d084f5ae9: Tracker: Untrack pages after link recommendation edit which is definitely unrelated. A CI config change, maybe?

gerritbot added a comment.May 9 2021, 8:47 PM

Change 687415 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] [DNM] CI test

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

gerritbot added a project: Patch-For-Review.May 9 2021, 8:47 PM
Tgr added a project: Continuous-Integration-Config.May 9 2021, 8:55 PM
kostajh moved this task from Incoming to Ready for Development on the Growth-Team (Sprint 0 (Growth Team)) board.May 10 2021, 7:50 AM
daniel subscribed.May 10 2021, 11:34 AM

Hm, got a fully stack trace?

I suspect this involves ObjectCache::newFromParams. It seems that one of the ObjectCaches used by WANObjectCache apparently needs UserNameUtils...

daniel updated the task description. (Show Details)May 10 2021, 11:36 AM
daniel added a comment.May 10 2021, 11:42 AM
In T282320#7074304, @daniel wrote:

Hm, got a fully stack trace?

I suspect this involves ObjectCache::newFromParams. It seems that one of the ObjectCaches used by WANObjectCache apparently needs UserNameUtils...

Found the stack trace. ObjectCache is not on the stack. Instead, the recursion happens via the destructor of a Session, which is triggered when the scope of the wiring method for WANObjectCache ends. It's not obvious why a (new local) Session instance would be created in that scope.

gerritbot added a comment.May 10 2021, 12:26 PM

Change 688264 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] [DNM] Force stack trace from Session constructor

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

Tgr added a comment.May 10 2021, 1:17 PM

For the time being, we could work around the issue by making UserNameUtils::isIP static.

gerritbot added a comment.May 10 2021, 1:29 PM

Change 688308 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] User::getID: Be lazier when fetching UserNameUtils

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

daniel added a comment.May 10 2021, 1:45 PM

So the trigger for this problem is probably >https://gerrit.wikimedia.org/r/c/mediawiki/core/+/683726/14/includes/session/SessionManager.php#229>, which causes SessionManager to ask the service container for a UserNameUtils object in the constructor. We could do that elsewhere to fix the problem for now. But at some point, we'll want to convert to proper DI, and we'll have the same chicke-and-egg issue agin.

I still don't see how we are accessing a SessionManager from the wiring of WANObjectCache anyway.
I made https://gerrit.wikimedia.org/r/c/mediawiki/core/+/688264 to force the Session constructor to throw when called from inside WANObjectCahe's wiring, assuming that, if the destructor is called from that scope, the constructor was also called from that scope somewhow, and we'd get a stack trace. But I can't make it trigger....

Tgr added a comment.EditedMay 10 2021, 2:15 PM

UserNameUtils is loaded in User::getID, which is called in the SessionBackend destructors (it's one of the few calls that are made even if no session operation happened). https://gerrit.wikimedia.org/r/c/mediawiki/core/+/688308 works around that (at least for anonymous users, but there shouldn't be any other kind of user during setup). It does fix the test error, which would imply this was caused by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/678604 but the timing does not line up...

In any case, as you say, the bigger mystery is why sessions are involved at all at this point. Something in that ObjectCache logic must have created and disposed a SessionBackend object, but I have no idea what that is. Nor how it could be related to GrowthExperimentsMultiConfig (presumably the reason why this error only shows up in GrowthExtensions) or why MultiLock is the single special page for which the test fails. It does depend on UserNameUtils, but it's not the only special page that does. And in any case the loading of that service wasn't triggered via that dependency.

gerritbot added a comment.May 10 2021, 4:52 PM

Change 688308 merged by jenkins-bot:

[mediawiki/core@master] User::getID: Be lazier when fetching UserNameUtils

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

gerritbot added a comment.May 11 2021, 12:00 AM

Change 687415 abandoned by Gergő Tisza:

[mediawiki/extensions/GrowthExperiments@master] [DNM] CI test

Reason:

not needed anymore

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

Tgr added a comment.May 11 2021, 12:04 AM

The GrowthExperiments CI error is fixed (PHPUnit tests never have a valid session so limiting the UserNameUtils dependency to non-anonymous users will prevent the error from occuring in tests). But the underlying issue of the WANObjectCache service factory creating a session for no obvious reason is not fixed, and seems like the kind of thing that can cause trouble in the future.

• Ramtin2021 closed this task as Declined.May 11 2021, 1:23 AM
Krinkle changed the task status from Declined to Resolved.May 11 2021, 1:42 AM
daniel mentioned this in T282533: WANObjectCache is causing a Session to be constructed for no obvious reason.May 11 2021, 10:43 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 · Credits