Page MenuHomePhabricator

mw.loader sometimes executes the module script twice
Closed, ResolvedPublic

Description

In PageTriage, we have a JS file that loads a separate ResourceLoader module if needed. It uses the syntax:
mw.loader.load( 'ext.pageTriage.views.toolbar' )

In some rare circumstances, it loads the ext.pageTriage.views.toolbar module twice (although the load command is only called once), causing the interface to break badly. This seems to only happen in Firefox.

I suspect this is a similar issue to Bug 34696. The steps to reproduce are a bit complicated, so I'm not going to post them unless you need them. Just let me know.


Version: 1.20.x
Severity: critical

Details

Reference
bz37331

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:26 AM
bzimport set Reference to bz37331.

(In reply to comment #0)

[..] The steps to reproduce are a
bit complicated, so I'm not going to post them unless you need them. Just let
me know.

Please do post the steps to reproduce this bug :)

Also, is this new in 1.20alpha? (I guess once the bug is reproducible, git bisect will tell us when it was introduced, or otherwise that it a bug since the beginning).

Yeah, I'm seeing it in 1.20alpha. I first noticed the bug a couple weeks ago, but assumed it was something on our end. It only shows up in conjunction with a new feature that we haven't deployed anywhere, so the fact that I only noticed it recently doesn't rule out the bug existing earlier.

I'll post the repro steps in a bit...

  • Bug 37609 has been marked as a duplicate of this bug. ***

The bug is appearing on en.wiki now, so it should be fairly easy to reproduce...

Steps to reproduce in Firefox:

  1. Log into en.wiki with an autoconfirmed account
  2. Go to http://en.wikipedia.org/wiki/Special:NewPagesFeed
  3. Click 'Review' for one of the unreviewed articles (in red)
  4. On that page add '?curationtoolbar=true' to the URL

The items in the curationtoolbar should load twice if the bug occurs (you'll see duplicate icons)

  • Bug 37605 has been marked as a duplicate of this bug. ***

Bumping to critical since this is blocking turning on the curation toolbar on en.wiki.

Roan and I have been debugging this. It turns out we're hitting a race condition.

When a module is ready to be executed, the module package is expanded (messages loaded, styles inserted, script executed) and then we set 'state' to 'ready'.

However in this case the script itself also has calls to mw.loader.load, which at some point calls handlePending (which executes modules that were waiting for execution due to unresolved dependencies). But this corrupted the other execution flow because it happened during execution. And the state of that module wasn't set to 'ready' until after the module is ready executing.

Fixed by setting the state right before execution begins.

(In reply to comment #9)

Roan and I have been debugging this. It turns out we're hitting a race
condition.

When a module is ready to be executed, the module package is expanded (messages
loaded, styles inserted, script executed) and then we set 'state' to 'ready'.

However in this case the script itself also has calls to mw.loader.load, which
at some point calls handlePending (which executes modules that were waiting for
execution due to unresolved dependencies). But this corrupted the other
execution flow because it happened during execution. And the state of that
module wasn't set to 'ready' until after the module is ready executing.

Fixed by setting the state right before execution begins.

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