Continue for all threads

[vadimcn]

The continue request has threadId attribute to specify which thread should continue execution. However, if this were implemented as spec'ed, how would the user signal the intent to continue running all threads?
Empirically, VSCode always sends a valid thread id, even when I click ▶️ in the top debugger toolbar.

[weinand]

Yes, today DAP does not support an "all threads" mode for the thread oriented requests like continue, next, stepIn, stepOut, reverseContinue, and stepBack.

@vadimcn you said:

VSCode always sends a valid thread id, even when I click ▶️ in the top debugger toolbar.

I do not understand what you mean by "the top debugger toolbar" but are you saying that VS Code should use a "Continue all threads" in that context, which today is impossible because it is not supported by DAP?

[vadimcn]

I do not understand what you mean by "the top debugger toolbar"

I mean the main debugger toolbar that appears at the top of VSCode window when debugging:

My expectation is that when I click the "play" button on that toolbar, all threads should be resumed, not just the currently selected one.

"Continue all threads" in that context, which today is impossible because it is not supported by DAP?

Yes. Or, rather, I'm saying that there should be some way to unpause all threads. When the debug adapter supports per-thread control, unpausing all of them one by one gets rather tedious. Try this with e.g Python extension's debugger.

[int19h]

Note that there's already ContinuedEvent.allThreadsContinued. So in practice, the adapter can treat "continue" requests (and stepping etc) as resuming all threads, if that's the best it can do.

In fact, we make use of this feature of DAP in the Python debug adapter - it's configurable to continue one thread vs all threads on step. This was required to use the adapter with Visual Studio due to this:

Visual Studio requires all threads to enter and leave break mode at the same time. Stepping, stopping, or continuing a single thread is not supported.

Furthermore, even for VSCode, we resume all threads on step by default - it turned out that most users find it less confusing in practice.

We also have a protocol extension in place whereby specifying "threadId": "*" can be used to explicitly continue all threads.

[vadimcn]

Note that there's already ContinuedEvent.allThreadsContinued

Sure, but this is adapter's choice, which user cannot influence. This seems sub-optimal...

The reason I am raising this, is that I've looked into implementing stopping/resuming a specific thread for my debug adapter, because this is useful every once in a while. However, this leads to degradation of the common use-case, so I've decided it isn't worth it. I suppose having this as a config setting is an option, but discoverability would be low.

Furthermore, even for VSCode, we resume all threads on step by default - it turned out that most users find it less confusing in practice.

So what's the purpose of having per-thread execution control buttons in VSCode? I realize this is DAP repo, not VSCode's, but still, there must have been some internal discussion between the teams?

We also have a protocol extension in place whereby specifying "threadId": "*" can be used to explicitly continue all threads.

But which DAP clients support it? VSCode doesn't (unless there's some magic option that enables this?)

In short, I would appreciate some guidance on how this is supposed to be implemented. Thanks!

[int19h]

To clarify, what I wrote above pertains specifically to the Python debug adapter, which is its own thing separate from VSCode proper. Other debug adapters do things differently.

[weinand]

@int19h @vadimcn thanks a lot for your clarifications.

I definitively see the need for improving this both on the Debug Adapter Protocol as well as in VS Code.

So let's start with DAP: what are the options?

It would be nice if we could just make the threadId property optional and introduce the new semantics "missing threadId means 'all threads'". But unfortunately this is not possible because an optional threadId would break all debug adapters that are relying on threadId being mandatory.

Since threadId is of type number, using a wildcard pattern '*' isn't possible either.

So I think the only option we have is to introduce a new boolean property allThreads on the continue, next, stepIn, stepOut, reverseContinue, and stepBack requests with the semantics: "if allThreads is true, the threadId property is ignored and the request applies to all threads. In addition a new debug adapter capability supportsAllThreads indicates to the client that a debug adapter understands the allThreads property.

What do you think?

[vadimcn]

But unfortunately this is not possible because an optional threadId would break all debug adapters that are relying on threadId being mandatory.

If you are adding a new capability, backwards compatibility with existing adapters shouldn't be a problem?

Regarding next, stepIn, stepOut and stepBack: IMO, "step all threads to the next line" does not make much sense; at least I had never needed that. There's always a current thread that you want to step.
On the other hand, some debuggers do make a distinction between stepping the current thread while allowing others to run freely and stepping the current thread with all other frozen. The latter operation is somewhat obscure though, so maybe DAP doesn't need to support this case.

[int19h]

With respect to stepping, yes, the difference is exactly that - whether other threads are frozen or not during the stepdap. Our takeaway from the DAP spec was that stepping while other threads are frozen is the intended behavior. But, in practice, this leads to issues when the stepped thread needs to acquire a lock that is held by one of the frozen threads - and this can be very non-obvious to the user if the lock is somewhere in library code that they are stepping over. If the intent was to unfreeze all threads during the step, I think that might be worth clarifying in the spec.

(Aside from that, there's the issue of VS DAP client being more broadly broken here, in that it expects all threads to unfreeze on step, but does not expect to get "continue" events for any threads other than the one it requested a step on. Conversely, VSCode, and any other properly conforming DAP client, expects "continue" events for all threads that were resumed. Thus, as things stand, there's no way to implement multi-threaded stepping portably across VS and VSCode without relying on "clientID" to special-case VS. This is a VS bug, of course, but they can't easily fix it because all-threads-run during stepping has been a fundamental assumption for VS debugger infrastructure for a very long time. I hope that, if DAP gets a clear way to indicate this semantics in the step request, VS would just adopt it, and we can drop any special treatment in adapters.)

[int19h]

My concern with adding a new flag to the request which directs the adapter to ignore threadId, is that existing adapters will quietly ignore the flag instead, and thus the behavior will not be what the client might have expected - but with no diagnostics. OTOH if threadId is made optional, then any existing adapter that doesn't understand it will fail fast, and the error message should make the problem clear. Since client is obligated to check capabilities before issuing requests that depend on them, an adapter that doesn't advertise supportsAllThreads should never get a request without threadId (and if it does, then it's a client bug - which, again, should be easy to diagnose from error messages).