1523897 - Doing the same dynamic import in the console twice crashes the content page (or make it unresponsive)

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce

1. Go to https://dynamic-import.glitch.me/
2. Open the console
3. Evaluate the following:

(async () => {
  var {rainbowLog} = await import("./cool-module.js");
  rainbowLog("Dynamic import in console")

  var {rainbowLog} = await import("./cool-module.js");
  rainbowLog("Dynamic import in console")
})()

Expected results

This works fine, and I see 2 "dynamic import in console" logs in the console output.

Actual results

Sometimes, the tab crash, sometimes it looks like it's unresponsive (can't reload for example).
I don't see any meaningful logs about this in the terminal:

Assertion failure: ptr, at /builds/worker/workspace/build/src/js/src/builtin/ModuleObject.cpp:910

###!!! [Parent][MessageChannel] Error: (msgtype=0x1E0087,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv

++DOCSHELL 0x11c6c1800 == 10 [pid = 44982] [id = {7e79321c-ce5b-8243-97f2-9135b2c2e392}]
++DOMWINDOW == 21 (0x13d9b6800) [pid = 44982] [serial = 62] [outer = 0x0]
++DOMWINDOW == 22 (0x132ad2400) [pid = 44982] [serial = 63] [outer = 0x13d9b6800]
++DOMWINDOW == 23 (0x132a90800) [pid = 44982] [serial = 64] [outer = 0x13d9b6800]
++DOMWINDOW == 24 (0x15da54000) [pid = 44982] [serial = 65] [outer = 0x1303cdc00]
++DOCSHELL 0x11c669000 == 11 [pid = 44982] [id = {7256641e-ea3b-0145-8785-7acbd95f9f5a}]
++DOMWINDOW == 25 (0x125ebec00) [pid = 44982] [serial = 66] [outer = 0x0]
++DOMWINDOW == 26 (0x128d9c800) [pid = 44982] [serial = 67] [outer = 0x125ebec00]
++DOMWINDOW == 27 (0x144804400) [pid = 44982] [serial = 68] [outer = 0x125ebec00]
++DOCSHELL 0x11c6cc800 == 12 [pid = 44982] [id = {0ff8764b-2277-064e-a949-c288734c70e9}]
++DOMWINDOW == 28 (0x132accc00) [pid = 44982] [serial = 69] [outer = 0x0]
++DOMWINDOW == 29 (0x144807800) [pid = 44982] [serial = 70] [outer = 0x132accc00]

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Also, the same code seems to work fine in content page.

Jon, would you have an idea what's happening here?

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Here's a crash report: https://crash-stats.mozilla.org/report/index/d5aa9d64-e4fe-477c-8642-0e80e0190130

Note that it's not related to glitch, I reproduce the crash when I'm on https://nchevobbe.github.io/demo/console-test-app.html

Comment 3

[Jon Coppeard (:jonco)]

Thanks for the bug report!

Comment 4

[Jon Coppeard (:jonco)]

Attachment: bug1523897-associate-modules-once

The problem is that ScriptLoader::AssociateSourceElementsForModuleTree never sets the SourceElementAssociated flag if the module request has no element. Modules have their JSScript set to null after their first and only execution, thus reloading an already-executed module with no element causes an assertion when we try to get its JSScript in nsJSUtils::InitModuleSourceElement.

We just need to make sure we set the flag so we don't attempt to call this every time.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Jon, are you going to write a test for that, or do you want me to add one in the console?

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Nicolas Chevobbe from comment #5)
I don't know how to set up a console test, so yes please go ahead.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: bug1523897-init-debugger-data-once-only

Here's a updated patch that is hopefully clearer.

I renamed things to talk about 'initializing debugger data' rather than 'associating source elements' and moved setting of the flag to the end of that method to make it clearer that it reflects the whole of that operation happening rather than the specific call to nsJSUtils::InitModuleSourceElement.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/28f22df25f1b
Only initialize debugger data for modules the first time they are executed r=smaug

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/28f22df25f1b