1532265 - Assertion failure: !generator->isClosed() (closed generator when resuming async function), at js/src/vm/AsyncFunction.cpp:91 or Crash [@ ??] with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 06f0c5e35c3a (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager --ion-offthread-compile=off):

let g = newGlobal({newCompartment: true});
g.eval(`
    async function timeout(n) {
        for (let i = 0; i < n; i++)
            await Promise.resolve(i);
    }
    let racer = timeout(5);
`);
let dbg = Debugger(g);
dbg.onEnterFrame = frame => {
    frame.onPop = async each => {};
};

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  AsyncFunctionResume (cx=<optimized out>, generator=..., kind=kind@entry=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:90
#1  0x00005555559ce54f in js::AsyncFunctionAwaitedFulfilled (cx=<optimized out>, generator=..., generator@entry=..., value=..., value@entry=...) at js/src/vm/AsyncFunction.cpp:128
#2  0x0000555555987a1b in AsyncFunctionPromiseReactionJob (rval=..., reaction=..., cx=<optimized out>) at js/src/builtin/Promise.cpp:1491
#3  PromiseReactionJob (cx=<optimized out>, cx@entry=0x7ffff5f17000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Promise.cpp:1612
#4  0x00005555558fa6a9 in CallJSNative (cx=0x7ffff5f17000, native=0x555555986d50 <PromiseReactionJob(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:440
[...]
#12 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:10966
rax	0x555557c3d280	93825033032320
rbx	0x555556b396c0	93825015191232
rcx	0x7ffff6c1c2dd	140737333281501
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffd070	140737488343152
rsp	0x7fffffffcee0	140737488342752
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6cc0	140737354034368
r10	0x0	0
r11	0x0	0
r12	0x0	0
r13	0xfffa000000000000	-1688849860263936
r14	0x7fffffffcf30	140737488342832
r15	0x7fffffffcf10	140737488342800
rip	0x5555559ce309 <AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::HandleValue)+1433>
=> 0x5555559ce309 <AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::HandleValue)+1433>:	movl   $0x0,0x0
   0x5555559ce314 <AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::HandleValue)+1444>:	ud2

This crashes without symbols on the heap, so marking as fuzzblocker because we can't easily differentiate this from other issues.

Comment 1

[André Bargull [:anba]]

The assertion was added in bug 1530324, but it looks like it actually uncovered an existing issue.

let g = newGlobal({newCompartment: true});
let dbg = new Debugger(g);

g.eval(`
    async function f() {
        print("START")
        await Promise.resolve(0);
        await Promise.resolve(0);
        print("DONE")
    }
`);

g.f();

dbg.onEnterFrame = frame => {
    print("enter")
    frame.onPop = () => {
        print("pop")
        return null;
    };
};

When run with --no-baseline (and without bug 1530324), it prints:

START
enter
DONE
pop

But this with --baseline-eager (and also without bug 1530324):

START
enter
pop

So somehow baseline doesn't exit the function properly and by that leaves things in an inconsistent state, which leads to the assertion described in comment #0.

Comment 2

[André Bargull [:anba]]

And when the onEnterFrame handler is changed to:

dbg.onEnterFrame = frame => {
    print("enter")
};

--no-baseline prints:

START
enter
DONE

whereas --baseline-eager prints:

START
enter
enter
DONE

(This behaviour is reproducible with and without bug 1530324.)

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1532265 - Part 1: Skip over InterpretGeneratorResume in IsTopMostAsyncFunctionCall. r=arai!

• Remove nullptr checks for calleeTemplate because it can't be null for
  function frames.
• Always skip over InterpretGeneratorResume to ensure baseline sees the same
  number of debugger enter-frame events as interpreter.

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1532265 - Part 2: Handle closed generators in async function resume. r=arai!

Debugger or OOM errors can close the generator after JSOP_ASYNCAWAIT enqueued
a promise job for AsyncFunctionResume. Change AsyncFunctionResume to handle
this case and also try to reject the result promise with the pending OOM error
if possible.

Depends on D22299

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21c52ca0266e
Part 1: Skip over InterpretGeneratorResume in IsTopMostAsyncFunctionCall. r=arai
https://hg.mozilla.org/integration/autoland/rev/413190773025
Part 2: Handle closed generators in async function resume. r=arai

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/21c52ca0266e
https://hg.mozilla.org/mozilla-central/rev/413190773025