browser_dbg_debugger-statement.js has unhandled promise rejections

RESOLVED FIXED in Firefox 48

Status

()

Firefox
Developer Tools: Debugger
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jryans, Assigned: jsnajdr)

Tracking

Trunk
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

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
(Reporter)

Comment 2

2 years ago
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.
(Reporter)

Comment 4

2 years ago
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)

Updated

2 years ago
Assignee: nobody → jsnajdr
(Assignee)

Comment 5

2 years ago
Created attachment 8741375 [details] [diff] [review]
browser_dbg_debugger-statement.js has unhandled promise rejections.
Attachment #8741375 - Flags: review?(ejpbruel)
(Assignee)

Comment 6

2 years ago
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+
(Assignee)

Comment 8

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=832626d30cf4
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 9

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/d4cd977c44f0
Keywords: checkin-needed

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d4cd977c44f0
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.