The debugger client requests frames after the server has resumed

RESOLVED FIXED

Status

()

Firefox
Developer Tools
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: past, Assigned: past)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Opening the script debugger in a tab results in this output:

DBG-SERVER: Got: {"to":"root","type":"listTabs"}
DBG-SERVER: Got: {"from":"root","selected":0,"tabs":[{"actor":"conn0.tab2","title":"Debugger test page","url":"http://htmlpad.org/debugger/"}]}
DBG-SERVER: Got: {"to":"conn0.tab2","type":"attach"}
DBG-SERVER: Got: {"type":"tabAttached","threadActor":"conn0.context3","from":"conn0.tab2"}
DBG-SERVER: Got: {"to":"conn0.context3","type":"attach"}
DBG-SERVER: Got: {"from":"conn0.context3","type":"paused","actor":"conn0.pause4","poppedFrames":[],"why":{"type":"attached"}}
DBG-SERVER: Got: {"to":"conn0.context3","type":"resume"}
DBG-SERVER: Got: {"from":"conn0.context3","type":"resumed"}
JavaScript error: , line 0: uncaught exception: Debugger command sent while not paused.


This was made visible by the patch in bug 690615. The cause is probably a race in startDebuggingTab: when the debugger attaches to the thread, the three helpers don't run their connect() methods to completion before the thread is resumed.
Assignee: nobody → past
Status: NEW → ASSIGNED
Created attachment 566240 [details] [diff] [review]
Working patch

This patch fixes the race in the most straightforward way. The re-architecture of the client based on Jim's ideas that we discussed might also fix this, but this should suffice until that time.
Attachment #566240 - Flags: review?(dcamp)

Updated

6 years ago
Attachment #566240 - Flags: review?(dcamp) → review+
https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/4bae4dbabb40
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
I just saw the same message again. This fix apparently narrowed the time window of the race, but didn't eliminate it. Will try adding callbacks to the client methods that result in protocol requests.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The bug stems from the fact that resume() must be called after both the framesadded and scriptsadded events are handled, which cannot be accomplished with simple callbacks. I'm still gauging whether to use another synchronization approach or rewrite the client API to eliminate this race in attachThread.
Created attachment 575196 [details] [diff] [review]
Working patch v2

(In reply to Panos Astithas [:past] from comment #4)
> The bug stems from the fact that resume() must be called after both the
> framesadded and scriptsadded events are handled, which cannot be
> accomplished with simple callbacks. I'm still gauging whether to use another
> synchronization approach or rewrite the client API to eliminate this race in
> attachThread.

Another way to look at it, is that when connecting to the debugger the UI tries to enumarate frames and scripts without asking for the caches to fill first. Frames will not be available at connection time, because the debugger is paused by the client, not due to a debugger statement or breakpoint/watchpoint. And as for scripts, they will be loaded via the unsolicited newScript notification anyway.
Attachment #575196 - Flags: review?(dcamp)
Blocks: 697762

Updated

6 years ago
Attachment #575196 - Flags: review?(dcamp) → review+
https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/bcc0b45b53c8
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.