Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expected treatment of threads in debug viewlet UI? #40

Closed
lukehoban opened this issue Nov 18, 2015 · 14 comments
Closed

Expected treatment of threads in debug viewlet UI? #40

lukehoban opened this issue Nov 18, 2015 · 14 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues info-needed Issue requires more information from poster search Search widget and operation issues
Milestone

Comments

@lukehoban
Copy link
Contributor

I’m seeing some odd behavior around how threads are being rendered in the Code debugger for the Go debugger extension. But I’m also not certain what I should be expecting to see, as I don’t think I’ve seen any examples of debuggers which show thread information in Code.

The Go debugger returns a list of threads from DebugSession#threadsRequest, but these are not displayed anywhere. However, it seems that if I’ve ever hit a breakpoint on a different thread before, then later breakpoints will cause a second root node to appear in the Call Stack window showing the stacks associated with the 2 threads I've hit breakpoints on at some point (out of the 10 or so threads I’ve reported as existing).

On the thread that is not the active thread, the call stack displayed appears to be the call stack from the previous breakpoint I was at, not the active call stack on that thread at the current point. And in fact, in both cases, my DebugSession#stackTraceRequest implementation is only being called once (for the thread the breakpoint was hit on), not for any of the other threads I reported.

After hitting first breakpoint:
screen shot 2015-11-17 at 10 23 37 pm
After hitting second breakpoint:
screen shot 2015-11-17 at 10 25 09 pm

@isidorn isidorn added the bug Issue identified by VS Code Team member as probable bug label Nov 18, 2015
@isidorn
Copy link
Contributor

isidorn commented Nov 18, 2015

Yes, this is a bug on the VSCode side. The issue seems to be that we only show the thread in the call stack once a stopped event has happened for that thread.

I can fix the issue but it requires testing so I propose to do it after connect event.

@isidorn isidorn added the info-needed Issue requires more information from poster label Nov 23, 2015
@isidorn
Copy link
Contributor

isidorn commented Nov 23, 2015

After investigating this a bit further, I have some more questions:

  • Do you send a 'thread' event (with the reason 'started') once a new thread gets created in your adapter. This event notifies VSCode that there is a new thread and in the mono case I can see new threads even though no breakpoints got hit on those threads
  • The current implemenation will only call the stackTrace request for the stopped thread, not for all the threads, which I believe is correct behavior. If you stop on Thread A I do not think it is necessery to check the state of thread B, since you have to report first that something changed in B

Let me know what you think

@isidorn isidorn added debug Debug viewlet, configurations, breakpoints, adapter issues and removed bug Issue identified by VS Code Team member as probable bug labels Nov 24, 2015
@isidorn isidorn assigned lukehoban and unassigned isidorn Dec 4, 2015
@isidorn
Copy link
Contributor

isidorn commented Dec 4, 2015

Assigning to @lukehoban so he reads my comment

@lukehoban
Copy link
Contributor Author

Sorry - missed your reply earlier.

Do you send a 'thread' event (with the reason 'started') once a new thread gets created in your adapter. This event notifies VSCode that there is a new thread and in the mono case I can see new threads even though no breakpoints got hit on those threads

No, I don't do this yet, and that is likely the root problem. I see how this is done in the Mono debugger and that makes sense.

Unfortunately, this won't map as easily to the Go debugger as it does in the Mono case. The Delve debugger does not provide any events, only host-initiated RPC commands. I think that's reasonably common for a class of debuggers primarily used through a command line interface.

So to implement this, we will need to get all the threads and compute which are new or stopped to send the events. It feels like the vscode debugger could just be doing that itself since we already provide the ability to list threads.

Is there any reason why you need these events vs. just computing the threads to show based on the ThreadsRequest, just as you do for scopes and variables today (I don't need to fire an event to tell you that the scope changed or that new variables were introduced to a scope)?

The current implemenation will only call the stackTrace request for the stopped thread, not for all the threads, which I believe is correct behavior. If you stop on Thread A I do not think it is necessery to check the state of thread B, since you have to report first that something changed in B

Okay - that makes sense. I think I was just confused because I was seeing multiple threads show stack traces at once in some cases - but probably that was because of not sending the right thread events.

@isidorn
Copy link
Contributor

isidorn commented Dec 4, 2015

Yes I could do something like you proposed.
@weinand what is your take on this. Should we change how we handle threads in the protocol so we support more adapters?

@vadimcn
Copy link
Contributor

vadimcn commented Dec 21, 2015

What I am seeing, is that upon hitting a breakpoint, the adaptor must report all threads in the process as stopped, otherwise vscode seems to think they are still running and won't display their callstacks.
Which may be fine, if you are planning to support something like gdb's 'non-stop' mode, but I feel this behavior could be better documented, 'cause I had to discover it via trial and error.

@vadimcn
Copy link
Contributor

vadimcn commented Dec 23, 2015

Oh, and if I report stops for all threads, the debugger UI seems to select which one to display as current randomly, which can lead to confusion as to which thread has caused the stop.
I've tried varying the order in which StoppedEvent's are sent, but that doesn't seem to change anything.

@isidorn
Copy link
Contributor

isidorn commented Dec 23, 2015

@vadimcn thanks for the feedback! We are aware of some of the issues you mentioned. Currently we are looking into improving/fixing issues around our thread architecture and this will probably be improved for our January release. Will keep you updated.

@vadimcn
Copy link
Contributor

vadimcn commented Jan 29, 2016

Any updates? The next release is nigh, but I am still seeing the same issues with threads display.

@isidorn
Copy link
Contributor

isidorn commented Jan 29, 2016

Unfortunately, this has not been tackled for January. We will look into improving our thread model in our February release.
Sorry about this.

@adelphes
Copy link

adelphes commented Feb 3, 2017

@vadimcn

Oh, and if I report stops for all threads, the debugger UI seems to select which one to display as current randomly, which can lead to confusion as to which thread has caused the stop.
I've tried varying the order in which StoppedEvent's are sent, but that doesn't seem to change anything.

Yes, I'm finding the same issue with my debugger extension. It's making multi-threaded debugging painful with the UI jumping to different sources on each break.

FWIW, I would prefer it if StoppedEvent could take an array of { threadId, reason } entries and use the first as the callstack display priority.

@adelphes
Copy link

adelphes commented Feb 3, 2017

And it would be nice if the threads display prioritised stopped threads over running threads in the order it displays them. I'm finding that on a lot of occasions, the thread that has stopped is the last created thread - which puts it at the bottom of the list requiring the user to scroll down to it.

@vadimcn
Copy link
Contributor

vadimcn commented Feb 3, 2017

@adelphes, I have not seen this issue for many months. VSCode displays the correct thread as long as you pass its id in the stop event.

@isidorn isidorn closed this as completed Feb 6, 2017
@isidorn
Copy link
Contributor

isidorn commented Feb 6, 2017

Closing as this issue is ancient, let's keep the discussion here #19858

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
@vscodebot vscodebot bot added the search Search widget and operation issues label Dec 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues info-needed Issue requires more information from poster search Search widget and operation issues
Projects
None yet
Development

No branches or pull requests

6 participants