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.