Add terminateDebugger option to disconnect request

[polinasok]

By default dlv debugger terminates itself when the debug session is terminated. However, if it is run with an option to allow multiple client connections, concurrently or sequentially (tracing and debugging the same process), then disconnect issued by a single client does not mean automatic detachment and termination. Instead the cli gives the client a prompt with a choice to terminate the server or keep it running while still attached to the process. This works with both launch and attach, as it is not uncommon for users to launch a remote debugging session and then keep reconnecting to it.

Can such a choice be supported via the DAP protocol? For example, for terminating or not the debuggee, we have terminateDebuggee. Can a similar terminateDebugger option be added as well?

[weinand]

@polinasok in #177 (comment) I tried to explain why "debuggers" are not part of the DAP and are not surfaced in the VS Code UI.

If we would add it to DAP, we would have to consider a corresponding UI in clients. However, today I do not see a strong need for this in clients or debug extensions: I'm not aware of any client or debug extension asking for such a mechanism.

@polinasok Did you consider implementing such a feature in your extension (as an implementation detail of your dlv debugger extension)?

Based on such an implementation it would be much easier to "sell" the feature to other clients and debugger extensions, and - if enough interest is sparked - to move it into DAP.

[polinasok]

Yes, if DAP were to add this option, my next issue request would be for vscode to support this in the UI :)

@hyangah
I don't know how we could do this in the extension. Even if we found a way to prompt the user, we would still need to communicate their choice to the debug adapter and then the debugger. And now that we are trying to deprecate the vscode-go TypeScript adapter that's bundled with the extension in favor of native DAP support in dlv, the only way we can tell delve to terminate or not is via DAP.

Just to document this better, I am hoping to implement an equivalent of this:

# Start debug server
$ dlv debug foo.go --headless --accept-multiclient
API server listening at: 127.0.0.1:53215

# Start debug client and connect to server
$ dlv connect :53215
Type 'help' for list of commands.
(dlv) exit
Would you like to kill the headless instance? [Y/n] <===================================

[hyangah]

Thanks @polinasok for looking into it.

dlv is one of the first attempts that implement DAP natively from the debugger, so we need some creativity.

IMO dlv DAP implementation shouldn't bring down the entire dlv server just because the client (vscode) disconnects from the server by default. Rather, I wish dlv's debug adapter implementation was implemented in a layer separate from the protocol server and represented as an internal state whose lifetime is bound to a single client's connection (let's assume we named it Session here). When the client sends a Disconnect request and disconnects from the server, that terminates only the Session, not the entire dlv server.

Then, whether to exit or not after a Session ends is dlv's implementation detail.

For example, if the dlv server has started with --accept-multiclient, it may choose to keep the debugger and the debugee running even after a Session ends. If the dlv server has started without the --accept-multiclient flag, it can exit after the first and only Session terminates.

According to the spec, Disconnect Request is described as:

It asks the debug adapter to disconnect from the debuggee and to terminate the debug adapter.

I hope my interpretation of the protocol spec isn't too far off from what DAP's Disconnect Request is originally meant for - we will still terminate the debug adapter (Session), strictly speaking 😅 But I may be wrong.

I still don't know how to interpret the Terminate request though - disable it when running in --accept-multiclient mode, maybe?

It would be convenient if users are given a choice to terminate the dlv server even when the server is running with the --accept-multiclient flag, but I hope users can live without such extra convenience. If many users ask for the
feature, I think the dlv dap implementation may consider a custom launch/attach request parameter (maybe exitOnDisconnect?).

For the dlv server started by the extension, the extension maintains the dlv server's lifetime and we can prompt users, but I still want to understand use cases.

Apart from this specific feature request, I think it would be nice if DAP supports a feature equivalent to LSP's ShowMessage Request. For example, in this specific case, upon receiving a Disconnect request, the dlv server can ask the client to display questions like 'do you want to terminate the headless server? do you want to keep running the debugee but terminate only this headless server? do you want to send any specific signal to the debugee process? .... ) and send back the answer from the user.

[puremourning]

I think it would be nice if DAP supports a feature equivalent to LSP's ShowMessage Request. For example, in this specific case, upon receiving a Disconnect request, the dlv server can ask the client to display questions like 'do you want to terminate the headless serve...

I'm strongly against this. In many clients there is no appropriate or convenient way to present this to a user and/or interrupt what they are doing to questions like this. It's better for things to just work consistently rather that ask the user lots of questions at arbitrary times (IMO). Even so, in practice these sorts of interfaces become facsimiles of like a user-interface definition paradigm where we expose the concept of the user interface on top of the protocol. I'm not against specific messages that the client might implement by prompting the user, but I am against allowing the debug adapter to instruct the client to perform user-interface operations; that breaks the encapsulation and isolation that the protocol provides.

An example of a specific such message is the (custom) hotCodeReplace message implemented by the java debug adapter, and the (custom) debugpyAttach message implemented by debugpy. I would wholly like for these to be standardised, but they are at least 'logical' messages which a client may choose to implement with a popup/dialog, or may choose to implement client-side configuration or defaulting. If they were LSP "anything goes" showMessage style things then the client has no choice but to interrupt the user's typing. When you have a hammer, as the saying goes.

[hyangah]

@puremourning Thanks for sharing your thought. I didn't suggest this be required to be implemented in every adapter/client. In LSP, this capability is enabled only when both client and server can support. DAP also has similar capability negotiation. So I expect debug adapters are free to ignore this if it's not useful for them.

There is no concept like hotCodeReplace or debugpyAttach in Go's debug adapter, so, I don't know what you mean by standardizing them and why they are logical but asking Yes/No questions to users during termination phase isn't logical - at least, I see this may help preventing feature requests like #177. Anyhow, sorry if you get distracted. This issue is about the terminateDebugger option, so let's focus on the original issue.

[puremourning]

My point about the other 2 example is that specific requests for specific purposes is good, but generic ‘anything goes’ requests are blunt instruments which are frequently abused.

Regarding whether clients can/should implement things: the fact of the matter is that server authors only test with one client. And if another client doesn’t implement something then either it doesn’t work properly or the experience is sub standard.

I’m simply advocating that we use specific things like originally proposed, not nebulous generic things like LSP introduced.

[weinand]

I tried to explain above that DAP does not (yet) have a concept of "(long running) debuggers" and since there is only a single feature request asking for this, I'm not convinced that we want to introduce another concept to DAP without having a full UI story in DAP clients and DAs.

The dlv debugger is started by the command line (not VS Code!), so IMO a natural way to stop it is via the command line. If there is a strong need to provide VS Code UI for "debuggers" then the dlv/go extension can implement this first and if this is successful and other debug extensions want to do the same, we can promote the private implementation into DAP (and the VS Code client).

For now I will move this request to the backlog.
Please speak up if this doesn't work for you.

[polinasok]

If the DA server/debugger is started from the command-line, not VS code (or some other DAP-compatible editor), then, yes, it is a natural way to stop it via the command-line. Having an option to stop it from the client (such as VS code) via a user UI control translated into a setting in a DAP request would be for convenience. And yes, we have considered specifying this as part of the launch/attach request arguments (as @hyangah mentioned above), but that is not the most convenient option because the user might not know if they want to stop the debug server before they completed their debugging task and found what they were looking for.

The next question is if we want to allow a user to be able to rely on editor (such as VS Code) to start-up a reusable DA server/debugger for them (instead of starting it externally). Sometimes the users start the server manually not because they have to (e.g. due to remote system permissions), but because that gives them access to additional features. So ultimately we might want to streamline this for them and cut out the extra steps to improve the overall experience. But if they specify at start-up that they want us to launch a reusable server that will accept multiple connections from vscode or other clients, then it would be natural for them to expect different disconnect behavior and different disconnect controls than what they would in other cases. This is, of course, a corner case, and it is very possible that it is unique to Go. I can see how it makes sense to push this to the Backlog and see if more users and implementors would be interested in this feature.

In the meantime, I am wondering about the following:

1. Besides the objections expressed by @puremourning, what other objections are there to adding optional implementation-specific arguments similar to those in launch/attach to disconnect requests, so each debugger/adapter can customize not only how sessions are started, but also how they are stopped?
2. What is the best way support prompting for additional user inputs for disconnecting and channeling that to the DA in the Go extension only? How would the extension intercept user disconnect requests to special handle them and prompt? Would we need to define a completely separate UI element to disconnect? Would we need a thin adapter to implement custom disconnect logic? I am not sure how we could support this in the extension while communicating to dlv-dap via DAP only (our ultimate goal) without a thin adapter in-between. Ideally we would want to let the server know to shut itself down gracefully (or not) as opposed to force-killing it from the extension. And if the server only uses DAP to communicate, how would we tell it to do this without sending one of the DAP requests?

[puremourning]

For 2, I dont know about vscode, but my client vimspector already provides an out-of-band way to start debuggers (such as gdbserver, debugpy, lldb-server, delve) as part of its remote debugging marshalling features. DAP itself doesn’t need to be involved in this part, it’s done outside the Debugger client-debug adapter interaction. It’s effectively doing the ‘manually from the command line’ parts above, just not manually. Presumably other clients can do the same.

[polinasok]

Couple more thoughts on long-running debuggers.

We have the following component with a debugger that is also a DA server, communicating via the DAP protocol:

1. The server - listens and accepts to client connection(s)
   This is the umbrella component that consists of the two components below.
2. The server-client connection - receives requests and sends back events and responses
   This component is the DA piece that consists of connect+initialize+ ...+disconnect session. When the session ends and the DA terminates, this connection goes away, but the the server (1) can go on.
3. The server-debuggee connection - debugs a process
   This component is the debugger piece that can attach to the process and control its execution. Is (2) a prerequisite for (3)? We can choose to create (3) as part of starting up (1) the server (via command-line arguments) or only via launch/attach via (2) client connection for a debug session.

A long-running debugger/DA can therefore mean:

• (A) server that can accept more than one client connection, but can only create a single server-debuggee connection reused among all client connections
• (B) server that can accept more than one client connection, and can shut down and start up new server-debuggee connections to the same program/process for every new connection
• (C) server that can accept more than one client connection, and can shut down and start up new server-debuggee connections to the different program/process

(B) and (C) are implementation details. The end user nor the client should not normally care if the server survives between the sessions because each session is independent. But (A) is a special case, a feature that the end user might want to request explicitly, so they can rely on the debugger being long-running and connect to it in other ways. So I think in this use case exposing the debugger concept is inevitable and comes with the territory. But, of course, if this is only something that Go debugging cares about, then it is not a critical use case for DAP to support.

[weinand]

@polinasok I've tried to answer your two questions from above:

yes, we can easily add implementation-specific arguments to the disconnect request (as we already did for the launch and attach requests). The biggest difference between launch/attach and disconnect is that the "option bag" of the former was surfaced to end users as a "launch.json" (or a "debug configuration" in general) from day one. An "option bag" for the disconnect request was never requested as a feature and hence no established UI exists for it.

Introducing the disconnect "option bag" in DAP today would mean that we have to find and establish a universal UI for it. Since our discussion has not sparked much interest from other debuggers, I'm reluctant to start adding some features to DAP and corresponding UI to VS Code that is only used by a single debug extension.

I think an alternative exists: the dlv/go extension can introduce "private" properties on DAP's disconnect request and provide UI in the extension to manage the disconnect configuration options. E.g. an extension can intercept the disconnect request in order to inject the private disconnect configuration options. If we find out that there are problems with this approach or limitations with debug's extension API, then we can try to address them by improving the extension API.

[weinand]

@hyangah above you said:

I still don't know how to interpret the Terminate request though - disable it when running in --accept-multiclient mode, maybe?

We have tried to improve the documentation for terminate, please see https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Terminate

If you do not want to support terminate, then please don't enable the capability.