Page MenuHomePhabricator

Special pages are constructed twice
Closed, DeclinedPublic

Description

When a Special:... page is loaded, MediaWiki::performRequest() will create the corresponding special page object twice. This can cause weird bugs if the constructor does nontrivial work. Introduced in T109724.

Event Timeline

Tgr created this task.Jan 19 2016, 1:33 AM
Tgr raised the priority of this task from to Needs Triage.
Tgr updated the task description. (Show Details)
Tgr added a project: MediaWiki-Special-pages.
Tgr added a subscriber: Tgr.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptJan 19 2016, 1:33 AM
Florian claimed this task.Apr 2 2016, 2:36 PM

I hope I understand the problem correctly :P

Change 281174 had a related patch set uploaded (by Florianschmidtwelzow):
Don't construct SpecialPages twice

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

Florian triaged this task as Normal priority.Apr 2 2016, 2:44 PM
Florian moved this task from To triage to SpecialPage system on the MediaWiki-Special-pages board.

Change 281174 merged by jenkins-bot:
Don't construct SpecialPages twice

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

Florian closed this task as Resolved.Apr 2 2016, 5:54 PM
Florian removed a project: Patch-For-Review.
Tgr added a comment.Apr 2 2016, 6:11 PM

Thanks Florian!

Based on T132545, I disagree with "This can cause weird bugs if the constructor does nontrivial work." as caching SpecialPage instances also causes weird bugs, and am in favor of reverting this.

@PleaseStand asks on https://gerrit.wikimedia.org/r/#/c/283326/:

Why are we caching SpecialPage objects? T123995 says that "[creating] the corresponding special page object twice [...] can cause weird bugs if the constructor does nontrivial work." Which special pages are affected? Can they be fixed to work correctly in this case?
Alternatively, can the code in MediaWiki and SpecialPageFactory be refactored to avoid the construction of a separate SpecialPage instance, without resorting to caching in getPage()?

Tgr added a comment.Nov 8 2016, 6:46 AM

I don't know if any special page is affected at the moment; I worked around the double-creation when I ran into it. IIRC it had to do with registering hooks in the constructor.

Legoktm changed the task status from Resolved to Declined.Nov 10 2016, 5:52 AM