Closed Bug 1382306 Opened 7 years ago Closed 7 years ago

async function not working in the module type script

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: nullizer, Assigned: anba)

References

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170630112252

Steps to reproduce:

1. Set dom.moduleScripts.enabled to true

2. Write a html like this:
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <meta http-equiv="X-UA-Compatible" content="ie=edge">
  <title>Document</title>
</head>
<body>
  <script type="module">
    async function run() {
      console.log('console output not working')
    }
    const r = run()
    console.log(r)
  </script>
</body>
</html>

3. Test it with firefox 54.0.1, firefox nightly and chrome dev version


Actual results:

Firefox not execute the async function, the console.log statement not working. Chrome does it correct.


Expected results:

Should display text "console output not working" in the developer console panel
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
The bug is also reproducible on the shell:

  async function run() {
    print('called run()');
  }
  var r = run();
  print("run() returned:", r);

Prints "run() returned: [object Generator]", which indicates the async function's inner generator function is exposed as |run| instead of the actual async function.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch bug1382306.patch (obsolete) — Splinter Review
If I understand the BytecodeEmitter code for async functions correctly, we just need to call WrapAsyncFunction/WrapAsyncGenerator in ModuleObject::instantiateFunctionDeclarations(). And it seems like there was another issue in ModuleObject::instantiateFunctionDeclarations, the method calls Lambda() to clone the function, but then installs the original JSFunction into the module environment (it has |value = ObjectValue(*fun)|, but I think that should be |value = ObjectValue(*obj)|, shouldn't it?). And while touching this code, I've also moved the definition for |RootedObject obj| outside of the loop and removed what looks like unnecessary rooting in js::Lambda() and js::LambdaArrow().
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Attachment #8888043 - Flags: review?(jcoppeard)
Comment on attachment 8888043 [details] [diff] [review]
bug1382306.patch

Review of attachment 8888043 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, but can you add tests for the other cases too?

::: js/src/builtin/ModuleObject.cpp
@@ +904,5 @@
> +                obj = WrapAsyncFunction(cx, obj.as<JSFunction>());
> +            }
> +        }
> +
> +        value = ObjectValue(*obj);

Thanks, you're correct.  This was meant to be |obj| not |fun|.
Attachment #8888043 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #3)
> This looks good, but can you add tests for the other cases too?

Do you mean just adding a test for async generator declarations or something else?
(In reply to André Bargull from comment #4)
Yes, for async generator declarations if that's possible.
(In reply to Jon Coppeard (:jonco) from comment #5)
> (In reply to André Bargull from comment #4)
> Yes, for async generator declarations if that's possible.

Sure thing, update on the way!
Attached patch bug1382306.patchSplinter Review
Updated patch to include test case for async generators (and made the test case compatible for browsers builds by replacing assertEventuallyEq).
Attachment #8888043 - Attachment is obsolete: true
Attachment #8891635 - Flags: review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/81c056ef9e19
Create async function wrapper when instantiating module functions. r=jonco
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/81c056ef9e19
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1402649
You need to log in before you can comment on or make changes to this bug.