1547124 - ChromeUtils.import can return uninitialized exports

Status: NEW

(Core :: XPConnect, defect, P3)

Description

[Dave Townsend [:mossop]]

I seem to be hitting a race condition where two scripts are attempting to import the same module and in this case the second call seems to happen while the first call is still compiling the script ... and then returns before the first call has finished compiling the script.

At this point the variable I declare in the second case gets defined but accessing it complains about "ReferenceError: can't access lexical declaration `foo' before initialization". It I run Object.getOwnPropertyDescriptor(window, "foo") the browser crashes.

I added some logging to mozJSComponentLoader::Import and get the following output (the logs are behind a guard for the main process so this is all in the same process).

*** Importing
*** Building from scratch
*** Importing
*** Returning
JavaScript error: chrome://browser/content/browser.js, line 13: ReferenceError: can't access lexical declaration `foo' before initialization
*** cleanup
*** Build complete
*** Returning

I'm assuming that compiling a module can involve spinning the event loop and so allowing this to happen.

Comment 1

[Neha Kochar [:neha]]

Bobby, do you have any ideas for this?

Comment 2

[Kris Maglione [:kmag]]

There isn't really anything that can be done about this. It's essentially just a circular import scenario. If one script tries to import a module while the top-level of that module is executing for its initial import, the second import attempt is going to get an incomplete state.

The only real solution is "don't do that". Or conceivably, "switch to ES6 imports, which build the entire dependency graph at compile time and will fail eagerly if there are cyclic dependencies that can't be handled"

Comment 3

[Dave Townsend [:mossop]]

(In reply to Kris Maglione [:kmag] from comment #2)

There isn't really anything that can be done about this. It's essentially just a circular import scenario. If one script tries to import a module while the top-level of that module is executing for its initial import, the second import attempt is going to get an incomplete state.

I'm pretty sure there wasn't any circular importing going on here, just two unrelated scripts loading the same module. Seems like you could block the caller until the load is complete or at the very least throw a sane error when hitting this case.

Comment 4

[Kris Maglione [:kmag]]

(In reply to Dave Townsend [:mossop] (he/him) from comment #3)

I'm pretty sure there wasn't any circular importing going on here, just two unrelated scripts loading the same module. Seems like you could block the caller until the load is complete or at the very least throw a sane error when hitting this case.

We do block the caller until the load is complete. Whether it's circular or not, the behavior comes to the same thing. Something spinning the event loop during a module load is going to cause all sorts of undefined behavior.

Throwing an error is probably sensible. We have code that detects in-progress module loads that tries to return a partial set of exports. We could just make it an error instead.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:neha, could you have a look please?

For more information, please visit auto_nag documentation.