Assertion failure: cx->jobQueue->empty(), at js/src/builtin/Promise.cpp:5182 with async and Debugger

RESOLVED FIXED in Firefox 67

Status

()

defect
--
critical
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: gkw, Assigned: jimb)

Tracking

(Blocks 2 bugs, 4 keywords)

Trunk
mozilla67
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox65 unaffected, firefox66 unaffected, firefox67 fixed)

Details

(Whiteboard: [fuzzblocker][jsbugmon:update])

Attachments

(2 attachments)

Reporter

Description

4 months ago

The following testcase crashes on mozilla-central revision f0ea53f47215 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// Adapted from randomly chosen test: js/src/tests/test262/language/arguments-object/cls-expr-async-gen-func-args-trailing-comma-multiple.js
f = async function*() {};
f().next().then();
// Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1254190.js
var g = newGlobal({ newCompartment: true });
var dbg = new Debugger(g);
oomTest(() => {
    g.eval("(function() {})");
});

Backtrace:

#0 JS::AutoDebuggerJobQueueInterruption::~AutoDebuggerJobQueueInterruption (this=0x7fff89d2bb78) at js/src/builtin/Promise.cpp:5182
#1 0x000055f0ffc12499 in js::Debugger::dispatchHook<js::Debugger::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_11(js::Debugger::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_12)> (cx=0x7f38b7217000, hookIsEnabled=..., fireHook=...) at js/src/vm/Debugger.cpp:2087
#2 js::Debugger::slowPathOnNewScript (cx=0x7f38b7217000, script=...) at js/src/vm/Debugger.cpp:2090
#3 0x000055f10019e87c in js::frontend::BytecodeEmitter::emitScript (this=0x7fff89d2be18, body=0x7f38b72e8220) at js/src/frontend/BytecodeEmitter.cpp:2477
#4 0x000055f1001c9561 in js::frontend::ScriptCompiler<char16_t>::compileScript (this=0x7fff89d2cc70, info=..., environment=..., sc=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:555
/snip

For detailed crash information, see attachment.

Setting [fuzzblocker] as this is being hit a lot.

Reporter

Comment 2

4 months ago

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/3ae9f2b94f97
user: Jim Blandy
date: Tue Feb 12 08:10:54 2019 +0000
summary: Bug 1145201: Use AutoDebuggerJobQueueInterruption in Debugger. r=jorendorff

Jim, is bug 1145201 a likely regressor?

Flags: needinfo?(jimb)
Whiteboard: [jsbugmon:update] → [fuzzblocker][jsbugmon:update]
Assignee

Comment 3

4 months ago

Yes, that is almost certainly the regressor. I can reproduce this.

Assignee: nobody → jimb
Flags: needinfo?(jimb)
Assignee

Comment 4

4 months ago

The assertion is complaining that the Debugger onNewScript hook didn't properly drain its microtask queue before returning. However, the call to AutoDebuggerJobQueueInterruption::runJobs is definitely made unconditionally after running the hook. Since this is an OOM test, I suspect that an OOM is preventing the job queue from draining completely.

Assignee

Comment 5

4 months ago

The AutoDebuggerJobQueueInterruption destructor asserts that the Debugger has
properly managed its hooks' asynchronous jobs. But this assertion clearly only
applies when the AutoDebuggerJobQueueInterruption is properly initialized;
otherwise, the debuggee's job queue is still in place.

Unfortunately, the destructor was using the wrong test to determine whether the
debuggee's queue had been saved. This patch makes it uses the initialized
method, rather that checking the cx field, which is always initialized.

Assignee

Updated

4 months ago
Status: NEW → ASSIGNED

Comment 6

4 months ago
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fb6c0d850fa3
Use proper initialization condition in AutoDebuggerJobQueueInterruption destructor. r=arai
Flags: needinfo?(jimb)

Comment 8

4 months ago
Backout by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e2990d679a3b
Backed out changeset fb6c0d850fa3 for bustage in job-queue-04.js CLOSED TREE

Comment 9

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment 10

4 months ago
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/989d16d0f995
Backed out changeset fb6c0d850fa3 for bustage in job-queue-04.js CLOSED TREE
Assignee

Comment 11

4 months ago

Oh, this is a dumb one: tests that use oomTest need to make sure it's defined.

Flags: needinfo?(jimb)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla67 → ---
Assignee

Comment 12

4 months ago

I've queued a fixed version of the patch to land.

Comment 13

4 months ago
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6dea1af62279
Use proper initialization condition in AutoDebuggerJobQueueInterruption destructor. r=arai

Comment 14

4 months ago
bugherder
Status: REOPENED → RESOLVED
Closed: 4 months ago4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.