Visual Studio doesn't respect optional on StoppedEvent.threadID

[ben-clayton]

Apologies if this is not the correct place to file this issue. If it is the wrong place, please let me know where I should re-file this.

According to the DAP specification the Stopped event's threadId field is optional. It would seem that threadId is unnecessary when allThreadsStopped is specified.

However, a message using allThreadsStopped and with no threadId causes an InvalidOperationException:

 1> [DebugAdapter] <--   E (stopped): {"body":{"allThreadsStopped":true,"reason":"pause"},"event":"stopped","seq":8,"type":"event"}
 1> ERROR: InvalidOperationException: Stopped event missing ThreadId field!

 1> WARNING: Stopping due to fatal error: InvalidOperationException: Stopped event missing ThreadId field!
 1> ERROR: Unexpected error

Visual Studio version:

Microsoft Visual Studio Community 2019
Version 16.2.1
VisualStudio.16.Release/16.2.1+29201.188
Microsoft .NET Framework
Version 4.8.03752

[andrewcrawley]

Hi, Ben

You're correct - this is a difference between VS and VS Code. The underlying debugging infrastructure in VS requires a thread ID in order to show the correct call stacks, etc, so the debug adapter host enforces that the debug adapter must provide one.

Other such differences are documented here: https://github.com/microsoft/VSDebugAdapterHost/wiki/Differences-between-Visual-Studio-Code-and-the-Visual-Studio-Debug-Adapter-Host

Thanks!

Andrew

[ben-clayton]

Perfect - thank you for taking the time to write up all these differences! This will be very handy.

[ben-clayton]

Sorry to resurrect this - I've had a bit of time to digest the document and I have a few questions.

I understand that Visual Studio will have legacy-limited capabilities and may not be able to fully implement the DAP, but I'm still a little perplexed by VS's field requirements here.

If all threads need to be suspended and resumed together, would it not have made more sense for StoppedEvent.threadID to be ignored and require allThreadsStopped to be true? What purpose does the threadID have to VS?

I'm thinking about the amount of effort placed on the debugger implementors for some of these restrictions. For this particular event, how would a debugger implement a response that keeps both VS and VSCode happy? In our implementation, should we replace the use of allThreadsStopped with a separate per-thread StoppedEvent, or do we just need to add per-IDE conditional logic?

Is there any plan to expose VS's quirks via additional Capabilities?

Many thanks, Ben

[andrewcrawley]

Hi, Ben

VS actually ignores the allThreadsStopped field, since VS doesn't support any other mode of operation. The reason the thread ID is required is so we can show the correct stopped location to the user - if an app stops due to an exception, for instance, we want to show the call stack for the thread that actually hit the exception.

In your case, it looks like you're stopping due to the user hitting the "pause" button in the IDE, so there isn't really a thread responsible for the stop, but you may want to provide one anyway if you can more reliably choose a thread that would be interesting to the user (e.g. because it was the currently scheduled thread, or it's the only thread running user code). I can also tweak the logic in the host to not require an explicit threadId for stopped events where reason=pause - we can just select one arbitrarily to report to the IDE if one isn't provided.

Sending a single stopped event is correct - I think all you need to do to keep both VS and VS Code happy in this case is to specify a threadId value.

I'm not sure what you're looking for in terms of capabilities, but you can tell the difference between hosts via the clientId passed in the initialize request. VS passes "visualstudio", and VS Code passes "vscode". Other values are defined here: https://microsoft.github.io/debug-adapter-protocol/implementors/tools/

[ben-clayton]

The reason the thread ID is required is so we can show the correct stopped location to the user - if an app stops due to an exception, for instance, we want to show the call stack for the thread that actually hit the exception.

Okay, makes sense.

In your case, it looks like you're stopping due to the user hitting the "pause" button in the IDE, so there isn't really a thread responsible for the stop, but you may want to provide one anyway if you can more reliably choose a thread that would be interesting to the user (e.g. because it was the currently scheduled thread, or it's the only thread running user code).

Yes, this was way I hit this issue. IIRC, hitting the pause button sends a PauseRequest to the debugger with a threadID of 0, which I interpreted as a stop-all request and responded with a StoppedEvent with allThreadsStopped set to true and no threadID set. VSCode seemed happy with this. I don't currently have anything in my debugger that prefers one thread over others.

I can also tweak the logic in the host to not require an explicit threadId for stopped events where reason=pause - we can just select one arbitrarily to report to the IDE if one isn't provided.

My personal opinion here is that it would be friendlier to debuggers if VS was more lenient. After all, VS is the one straying from the spec, so being picky because it didn't get all the details it wanted (which is not required by the spec) is just adding burden on every debugger implementation. In this particular case my debugger has to pick a random thread to report the StoppedEvent on which is icky. I feel it's generally more helpful if VS didn't actually require the threadID to be set for any of these events (pause or otherwise). If there's no particular favourite thread in the debugger, a randomly picked thread would be as good picked by the debugger as by the IDE.

I'm not sure what you're looking for in terms of capabilities, but you can tell the difference between hosts via the clientId passed in the initialize request.

Sure, but IDEs come, go and evolve over time. Having the capabilities well-defined in the spec and protocol is significantly nicer than having to refer to an organically changing list of supported IDEs and their ever changing feature sets over time. Take for example your kind offer of 'tweaking the logic' - if this restriction is relaxed, do I now have to start adding conditionals into my debugger for what version of Visual Studio I'm talking to?

Many thanks, Ben