Last Comment Bug 693589 - The debugger client requests frames after the server has resumed
: The debugger client requests frames after the server has resumed
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Panos Astithas [:past] (away until 7/21)
:
Mentors:
Depends on:
Blocks: 697762
  Show dependency treegraph
 
Reported: 2011-10-11 07:25 PDT by Panos Astithas [:past] (away until 7/21)
Modified: 2011-11-18 03:03 PST (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Working patch (4.23 KB, patch)
2011-10-11 08:51 PDT, Panos Astithas [:past] (away until 7/21)
dcamp: review+
Details | Diff | Splinter Review
Working patch v2 (1.58 KB, patch)
2011-11-17 08:11 PST, Panos Astithas [:past] (away until 7/21)
dcamp: review+
Details | Diff | Splinter Review

Description Panos Astithas [:past] (away until 7/21) 2011-10-11 07:25:35 PDT
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.
Comment 1 Panos Astithas [:past] (away until 7/21) 2011-10-11 08:51:38 PDT
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.
Comment 2 Panos Astithas [:past] (away until 7/21) 2011-10-12 00:04:14 PDT
https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/4bae4dbabb40
Comment 3 Panos Astithas [:past] (away until 7/21) 2011-10-12 07:15:41 PDT
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.
Comment 4 Panos Astithas [:past] (away until 7/21) 2011-10-13 12:37:59 PDT
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.
Comment 5 Panos Astithas [:past] (away until 7/21) 2011-11-17 08:11:04 PST
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.
Comment 6 Panos Astithas [:past] (away until 7/21) 2011-11-18 03:03:53 PST
https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/bcc0b45b53c8

Note You need to log in before you can comment on or make changes to this bug.