693589 - The debugger client requests frames after the server has resumed

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

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 (he/him) [:past] (please ni?)]

Attachment: 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 (he/him) [:past] (please ni?)]

https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/4bae4dbabb40

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

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 (he/him) [:past] (please ni?)]

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 (he/him) [:past] (please ni?)]

Attachment: 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 (he/him) [:past] (please ni?)]

https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/bcc0b45b53c8