1499125 - Assertion failure: state() == JS::PromiseState::Pending, at js/src/builtin/Promise.cpp:4479

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 4a230b07f0cb (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):

function newPromiseCapability() {
    return {};
}
function neverCalled() {}
function resolveCapability(dIs) {}
class P extends Promise {
    constructor(executor) {
        executor(resolveCapability, neverCalled);
        var p = async function() {}();
        p.constructor = {
            [Symbol.species]: P
        };
        return p;
    }
}
var {
    promise: alwaysPending
} = newPromiseCapability();
P.race([alwaysPending]).then(neverCalled, neverCalled);


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555559db288 in js::PromiseObject::copyUserInteractionFlagsFrom (this=0x7ffff4e011d0, rhs=...) at js/src/builtin/Promise.cpp:4479
#1  0x00005555559e4067 in PromiseThenNewPromiseCapability (cx=<optimized out>, promiseObj=..., createDependent=createDependent@entry=js::CreateDependentPromise::SkipIfCtorUnobservable, resultCapability=resultCapability@entry=...) at js/src/builtin/Promise.cpp:3315
#2  0x00005555559e45eb in js::OriginalPromiseThen (cx=<optimized out>, promiseObj=promiseObj@entry=..., onFulfilled=..., onFulfilled@entry=..., onRejected=..., onRejected@entry=..., dependent=..., createDependent=js::CreateDependentPromise::SkipIfCtorUnobservable) at js/src/builtin/Promise.cpp:3333
#3  0x00005555559e4855 in Promise_then_impl (cx=<optimized out>, cx@entry=0x7ffff5f18000, promiseVal=..., promiseVal@entry=..., onFulfilled=..., onRejected=..., rval=..., rvalUsed=<optimized out>) at js/src/builtin/Promise.cpp:4080
#4  0x00005555559e4990 in Promise_then_noRetVal (cx=0x7ffff5f18000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Promise.cpp:4100
#5  0x000055555596a795 in CallJSNative (cx=0x7ffff5f18000, native=0x5555559e4910 <Promise_then_noRetVal(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:461
[...]
#19 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:10727
rax	0x0	0
rbx	0x7ffff4e011d0	140737301713360
rcx	0x7ffff6c1c2dd	140737333281501
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffc7c0	140737488340928
rsp	0x7fffffffc790	140737488340880
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6cc0	140737354034368
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7ffff4e00760	140737301710688
r13	0x7ffff4e00760	140737301710688
r14	0x7fffffffc8d8	140737488341208
r15	0x7fffffffc820	140737488341024
rip	0x5555559db288 <js::PromiseObject::copyUserInteractionFlagsFrom(js::PromiseObject&)+328>
=> 0x5555559db288 <js::PromiseObject::copyUserInteractionFlagsFrom(js::PromiseObject&)+328>:	movl   $0x0,0x0
   0x5555559db293 <js::PromiseObject::copyUserInteractionFlagsFrom(js::PromiseObject&)+339>:	ud2

Comment 1

[André Bargull [:anba]]

PromiseObject::copyUserInteractionFlagsFrom was added in bug 1491403, cc'ing :ehsan.

Comment 2

[(no longer active)]

The bug here is rather obvious.  We're hitting the assertion because our promise is fullfilled when we run copyUserInteractionFlagsFrom().  Thinking more about this, the assertion is actually bogus IMO, since even if the promise _is_ fullfilled we still want to copy the user interaction flags over to it to ensure the resolve handler will be called with the user activation flag.

Comment 3

[(no longer active)]

Attachment: Bug 1499125 - Remove the bogus assertion

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a379f71dac8
Remove the bogus assertion r=arai

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/399cd7842104
Backed out changeset 6a379f71dac8 for spidermonkey bustage at /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1298f1a74ae7
Remove the bogus assertion r=arai

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1298f1a74ae7

Comment 8

[(no longer active)]

Comment on attachment 9020862 [details]
Bug 1499125 - Remove the bogus assertion

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1491403

User impact if declined: This is just fixing an incorrect debug-only assertion.  There is no impact to our users.  It may be helpful to take this fix for the sake of people who run tests with debug builds on beta (not sure how many such folks exist).

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It's NPOTB (not part of the build) for non-debug builds.

String changes made/needed: None

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9020862 [details]
Bug 1499125 - Remove the bogus assertion

[Triage Comment]
Removes a bogus assert to make life better for our fuzzers. Approved for 64.0b6.

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b2f18c10db56