1254617 - browser_dbg_debugger-statement.js has unhandled promise rejections

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 1

[Eddy Bruel [:ejpbruel]]

Somewhat frequent intermittent, so this should be P2.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 3

[Eddy Bruel [:ejpbruel]]

Put this on my todo list for this week, but can't promise I'll be able to get to it.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 5

[Jarda Snajdr [:jsnajdr]]

Attachment: browser_dbg_debugger-statement.js has unhandled promise rejections.

Comment 6

[Jarda Snajdr [: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 7

[Eddy Bruel [:ejpbruel]]

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!

Comment 8

[Jarda Snajdr [:jsnajdr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=832626d30cf4

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d4cd977c44f0

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d4cd977c44f0