Closed Bug 1254617 Opened 4 years ago Closed 4 years ago

browser_dbg_debugger-statement.js has unhandled promise rejections

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: jryans, Assigned: jsnajdr)

References

Details

Attachments

(1 file)

See a partial try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f84c2d387a8.

This one is pretty intermittent.  I have seen it fail on at least OS X debug in the past.

Please note you need to apply the patch in bug 1240804 to see the test failures.

The failure log includes:

DevToolsUtils.assert threw an exception: Error: Assertion failure: Should have an event loop.
Somewhat frequent intermittent, so this should be P2.
Priority: -- → P2
This blocks re-enabling the unhandled promise rejection handling in the test harness for all of DevTools.  Would be good to fix soon, or else I'll like white list the test.
Put this on my todo list for this week, but can't promise I'll be able to get to it.
Bug 1240804 has now landed, re-enabling general unhandled promise rejection handling for DevTools.  This test was whitelisted at the time that landed.

To reproduce the bug here, first remove the "thisTestLeaksUncaughtRejectionsAndShouldBeFixed" call at the top of the test.
Assignee: nobody → jsnajdr
The testDebuggerStatement function doesn't return any promise, so the next then handler is immediately called before the pausing and resuming is finished. And the next handler closes the connection and cleans up.

If the async events happen in the right order, then a race condition can be triggered:

1. "resume" request is received by the ThreadActor.
2. The actor state is "paused" so the resumeLimitHandler.then handler [1] is scheduled.
3. "disconnect" request is received by the actor. The state is still "paused", so onResume is called again [2].
4. A second resumeLimitHandler.then is scheduled.
5. The then-handler from 2. is executed, it calls _resumed() (that sets state back to "running" [3]), then _popThreadPause().
6. The then-handler from 4. is executed, it calls _popThreadPause() once again, but this time there is nothing to pop! The error [4] happens.

I fixed only the test to wait for promises correctly. The race condition in setting the ThreadActor's state is still there.

[1] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/script.js#1024
[2] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/script.js#578
[3] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/script.js#1581
[4] Full message: TypeError: eventLoop is undefined
Full stack: ThreadActor<._popThreadPause@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:548:5
ThreadActor<.onResume/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1033:7
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:454:5
ThreadActor<.onResume@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1024:12
ThreadActor<.disconnect@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:578:7
AP_remove@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:263:7
AP_destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:225:7
DSC_removeActorPool/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1427:32
DSC_removeActorPool@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1427:9
BTA_popContext@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1300:5
BTA_detach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1335:5
BTA_onDetach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1391:10
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1643:15
ChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:742:5
EventLoop.prototype.enter@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:350:5
ThreadActor<._pushThreadPause@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:543:5
ThreadActor<._pauseAndRespond@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:753:7
ThreadActor<.onDebuggerStatement@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1829:9
runDebuggerStatement@http://example.com/browser/devtools/client/debugger/test/mochitest/doc_inline-debugger-statement.html:16:9
this.call@chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/code_frame-script.js:27:10
@chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/code_frame-script.js:80:12
promise callback*@chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/code_frame-script.js:79:3
Comment on attachment 8741375 [details] [diff] [review]
browser_dbg_debugger-statement.js has unhandled promise rejections.

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

Promise bugs can be hard to catch, so thank you very much for figuring this one out!
Attachment #8741375 - Flags: review?(ejpbruel) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d4cd977c44f0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.