STR

• Open Browser Toolbox & Debugger
• Open Content Toolbox and select the Network panel
• Load devtools/client/netmonitor/src/components/App.js
• Try to create BP in the render method (line 64 or 67)
  • It tends to work for the first time, try other files from DevTools code base in case of success. Also try to reload the page where Network panel is opened and consequently try to create the BP again.

There is an exception when trying to set a breakpoint using the BP gutter:
JavaScript error: resource://devtools/client/debugger/src/client/firefox/commands.js, line 201: TypeError: thread is null

This bug probably causes the blank-debugger panel crash

Revised STR:

1. Open Browser Toolbox & Debugger
2. Open Content Toolbox and select the Network panel
3. Add a breakpoint in devtools/client/netmonitor/src/components/App.js's render method (64 or 67)
4. close content devtools and re-open

NOTE: that you'll be paused in the browser toolbox, but wont see a call stack. The reason for this is that the call to fetch frames returns an empty array.

I added a dump statement in ThreadActor.onFrames to see if frame.older would find older frames. This confirmed that older is null and as a result we are not seeing any frames.

diff --git a/devtools/server/actors/thread.js b/devtools/server/actors/thread.js
index 92373850b1a9..a146ad6216aa 100644
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -1262,6 +1262,7 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
   },

   onFrames: function(request) {
@@ -1281,6 +1282,8 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
       i++;
     }

+    dump(`>> onFrames ${i}\n`)
+
     // Return request.count frames, or all remaining
     // frames if count is not defined.
     const frames = [];

Alex, who do you think is the right person to look into this platform issue?

With comment 1's str, I do see the yellow "you are paused" label, but this isn't really paused.
Netmonitor page execution isn't paused. Evaluation isn't against the breakpoint's frame. And as you say, the call stack isn't displayed.

Isn't it something to be figured out by someone from the debugger team having knowledges with the thread actor?
I do not see any "crash" (I mean C++ one), nor any exception. It must be some confusion between targets, processes and the thread actor.

After bisecting, the issue looks to be caused by the recent effort to use content frame. Disabling devtools.toolbox.content-frame fixes the issue.

CC Julian, do you have ideas on some good next steps?

I added a dump statement in ThreadActor.onFrames to see if frame.older would find older frames.
This confirmed that older is null and as a result we are not seeing any frames.

I tried a similar logging but I get >> onFrames 0, which doesn't mean that frame.older was falsy. It means that either this.youngestFrame was null or request.start was 0. In this case, request.start is 0, but that's the case with or without type=content. However this.youngestFrame is indeed falsy only when using a content-frame. This getter relies on this.dbg.getNewestFrame(). Unless you have another reason to suspect an issue with older, I would rather try to look at the C++ implementation of getNewestFrame. https://searchfox.org/mozilla-central/source/js/src/debugger/Debugger.cpp#4383

I am really not great at debugging C++ code, so if anyone in the debugger team can take a look at this, it would be helpful.

Thanks for looking into this. I wonder if Brian might have some ideas.

Did you get a crash report for the problem?

It is JS related, so you see the exception in the terminal.

Changing the title of this bug to make it less confusing as there is no C++ crash.

There is a JS exception on the client side resource://devtools/client/debugger/src/client/firefox/commands.js, line 201: TypeError: thread is null.
But Julian narrowed that down to an issue on the actor side, possibly in youngestFrame / DebuggerAPI.getNewestFrame:
https://searchfox.org/mozilla-central/rev/9bb55ae4d808fc48afcf93f99da6a685265b86c6/devtools/server/actors/thread.js#1264-1283

Also note that this bug isn't related to the omniscient browser toolbox and recent work related to fission as it fails also without the related pref turned on.

The frame accessor is actually behaving correctly here. The problem is that an error occurred while pushing a nested event loop while pausing, so that the server doesn't actually pause, and when the client asks for the frames there is nothing on the stack. The error which caused the event loop to fail does get reported to the console in the browser toolbox, though I didn't notice it until after I tracked down the problem in a more laborious fashion. I'll post a fix which takes care of the problem for me, though I don't know if it's the best fix.

Got an exception during TA__pauseAndRespond: can't access dead object:
getAllWindowDebuggees/<@resource://devtools/server/actors/utils/event-loop.js:136:13
getAllWindowDebuggees@resource://devtools/server/actors/utils/event-loop.js:134:10
preNest@resource://devtools/server/actors/utils/event-loop.js:152:31
enter@resource://devtools/server/actors/utils/event-loop.js:75:30
_pushThreadPause@resource://devtools/server/actors/thread.js:248:15
_pauseAndRespond@resource://devtools/server/actors/thread.js:795:12
hit@resource://devtools/server/actors/breakpoint.js:269:29
render@resource://devtools/client/netmonitor/src/components/App.js:62:9
finishClassComponent@resource://devtools/client/shared/vendor/react-dom.js:10638:31
updateClassComponent@resource://devtools/client/shared/vendor/react-dom.js:10601:44
beginWork@resource://devtools/client/shared/vendor/react-dom.js:11419:16
performUnitOfWork@resource://devtools/client/shared/vendor/react-dom.js:14702:12
workLoop@resource://devtools/client/shared/vendor/react-dom.js:14720:24
renderRoot@resource://devtools/client/shared/vendor/react-dom.js:14803:15
performWorkOnRoot@resource://devtools/client/shared/vendor/react-dom.js:15655:17
performWork@resource://devtools/client/shared/vendor/react-dom.js:15567:24
performSyncWork@resource://devtools/client/shared/vendor/react-dom.js:15541:14
requestWork@resource://devtools/client/shared/vendor/react-dom.js:15410:5
scheduleWork@resource://devtools/client/shared/vendor/react-dom.js:15224:16
scheduleRootUpdate@resource://devtools/client/shared/vendor/react-dom.js:15865:15
updateContainerAtExpirationTime@resource://devtools/client/shared/vendor/react-dom.js:15881:10
updateContainer@resource://devtools/client/shared/vendor/react-dom.js:15908:10
ReactRoot.prototype.render@resource://devtools/client/shared/vendor/react-dom.js:16133:18
legacyRenderSubtreeIntoContainer/<@resource://devtools/client/shared/vendor/react-dom.js:16242:14
unbatchedUpdates@resource://devtools/client/shared/vendor/react-dom.js:15772:10
legacyRenderSubtreeIntoContainer@resource://devtools/client/shared/vendor/react-dom.js:16238:21
render@resource://devtools/client/shared/vendor/react-dom.js:16289:12
bootstrap@resource://devtools/client/netmonitor/src/app.js:72:11
open@resource://devtools/client/netmonitor/panel.js:20:15
async*onLoad@resource://devtools/client/framework/toolbox.js:2328:27

Thanks. I feel silly, i've seen that too, but assumed that was from the client trying to communicate with the server after the frame accessor failed....

(In reply to Brian Hackett (:bhackett) from comment #10)

The error which caused the event loop to fail does get reported to the console in the browser toolbox, though I didn't notice it until after I tracked down the problem in a more laborious fashion.

+1, I also miss this exception while testing the STR.

It comes from this dumpn() call over here, which only logs on stdout if devtools.debugger.logging is set to true, which isn't the case by default.
Should we update this to use console.error(msg) instead? I don't see why we would hide any such error/exception this is very misleading.

Thanks for finding a fix!
Could we uplift the patch to Firefox Beta after the patch reaches Nightly?

https://hg.mozilla.org/mozilla-central/rev/7fabd271c116

Does this need a Beta approval request?

Comment on attachment 9091199 [details]
Bug 1577881 - Watch out for dead windows in getAllWindowDebuggees, r=ochameau.

Beta/Release Uplift Approval Request

• User impact if declined: The devtools debugger can be broken / unusable in some circumstances.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: No
• 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): This is a simple patch to avoid throwing an exception due to operating on dead windows.
• String changes made/needed:

(In reply to Alexandre Poirot [:ochameau] from comment #13)

(In reply to Brian Hackett (:bhackett) from comment #10)

The error which caused the event loop to fail does get reported to the console in the browser toolbox, though I didn't notice it until after I tracked down the problem in a more laborious fashion.

+1, I also miss this exception while testing the STR.

It comes from this dumpn() call over here, which only logs on stdout if devtools.debugger.logging is set to true, which isn't the case by default.
Should we update this to use console.error(msg) instead? I don't see why we would hide any such error/exception this is very misleading.

Yeah, I agree, and just filed bug 1581232 to fix this.

Comment on attachment 9091199 [details]
Bug 1577881 - Watch out for dead windows in getAllWindowDebuggees, r=ochameau.

Simple fix for an issue in debugger, let's try it for beta 7.

https://hg.mozilla.org/releases/mozilla-beta/rev/1f9a07afed3e

Confirmed issue with 70.0a1 (2019-08-30) on Windows 10.
Fix verified with 71.0a1 (2019-09-18), 70.0b7 on Windows 10, macOS 10.12, Ubuntu 16.04.