Closed Bug 1499125 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: decoder, Assigned: ehsan)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update,bisect])

Attachments

(1 file)

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
PromiseObject::copyUserInteractionFlagsFrom was added in bug 1491403, cc'ing :ehsan.
Blocks: 1491403
Flags: needinfo?(ehsan)
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.
Flags: needinfo?(ehsan)
Assignee: nobody → ehsan
Backout by dvarga@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/1298f1a74ae7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
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
Attachment #9020862 - Flags: approval-mozilla-beta?
Flags: qe-verify-
Flags: in-testsuite+
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.
Attachment #9020862 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.