Allow a debuggee to be disconnected gracefully

[yannickowow]

Hello,
I would like to know it if would make sense to add the ability to disconnect (on attach mode) gracefully using the same behaviour as terminateRequest.

My first guess would be if a debuggee has supportsTerminateRequest to true, the first detach command or stop command will trigger terminateRequest (with an optional flag) instead of disconnectRequest. When detaching, a debuggee can emit outputs and would be ignored if a client (such as VS Code) shutdowns immediately a debug session.

Regards.

[weinand]

@yannickowow yes, I think this feature request makes sense (and it was reported by @ericdrobinson too)

[ericdrobinson]

Would this make sense to partially implement as a new feature? The supportsTerminateRequest is sort of specific in its implementation at this point to "termination" as opposed to "disconnection". Perhaps it makes sense to add a supportsGracefulDisconnect or the like?

I believe the issue I reported that @weinand is referring to is this one, but it is VS Code-specific and is specifically about VS Code's implementation of supportsTerminateRequest being ignored when a "Stop" (termination) signal is sent by the user during an "Attach" Debug Session. In recent versions of VS Code, you can hold the Alt/Option key while hovering over the Debug Toolbar to switch between Disconnect/Stop buttons. The button state flip is opposite for Launch and Attach configs, but in both cases both the "Disconnect" and "Stop" buttons are available. My request there is to simply ensure that the "Stop" flow in VS Code works the same for an Attached Debug Session as it does for a Launched Debug Session.

In this case, it seems that the request is to allow a "Graceful Disconnect" option, whereby there would be a two-stage disconnect request. I guess supportsTerminateRequest could be overloaded to take on this meaning, but if that were the case I would recommend deprecating the supportsTerminateRequest capability in favor of something like supportsGracefulDebuggeeStop or something that isn't specific to the TerminateRequest. The supportsTerminateRequest is already confusing enough (#249).

[weinand]

@yannickowow DAP's terminate request is completely orthogonal (independent) to the disconnect request.

A hypothetical client could surface a terminate request as a separate "Shut down debuggee gracefully" command (instead of integrating it into the "Stop Debug" command as VS Code does).
Using that command would trigger the "graceful cleanup code" in the debuggee before the debuggee terminates itself.

There are two possible outcomes of the "Shut down debuggee gracefully" command:

• if everything went well, then the debuggee has terminated itself which ends the debug session and there is no need for a subsequent disconnect request (and the discussion about a "graceful disconnect" is moot).
• if the terminate request did not result in the debuggee terminating itself, then the debug session will just continue and the user can now end the debug session by either disconnecting or by disconnecting and terminating the debuggee. So here a "graceful disconnect" is possible but only in the "vetoed" case.

Bottom line: from a DAP perspective "disconnect gracefully" is possible for the "vetoed" case, but it does not make sense for the un-vetoed case because a "terminate" means "terminate" and not just "disconnect".
So there is nothing to be done here.

---

Now back to VS Code's implementation of this part of the spec:

In VS Code calling the terminate request is integrated into the "Stop Debug" command:
if a debug adapter supports the terminate request, the first run of the "Stop Debug" command calls terminate and if that fails ("is vetoed"), a second run of the "Stop Debug" command calls disconnect.

Since the ultimate goal of the terminate request is to let the debuggee terminate itself gracefully, VS Code made the assumption that terminate needs only to be called in the "launch" case where the debug adapter "owns" the lifecycle of the debuggee and is responsible for terminating the debuggee.
In the "attach" case the terminate request is not called, because the debug adapter does not own the debuggee's life cycle and is not responsible for terminating the debuggee.

However, recently we've added a new argument terminateDebuggee to the disconnect request so that clients can better control whether the debug adapter should terminate the debuggee or not.
But this requires that VS Code does not only call the terminate request in the "launch" case but also when the terminateDebuggee argument is true.

This bug is tracked as microsoft/vscode#143991

[yannickowow]

Correct, thanks for your clarifications.

[ericdrobinson]

@weinand I think you may have misinterpreted the initial request. Specifically that request was for:

the ability to disconnect (on attach mode) gracefully using the same behaviour as terminateRequest.

I read this as a feature request to add a supportsDisconnectRequest that aligns with the supportsTerminateRequest functionality, rather than piggybacks on it. The reason that I think you may have misinterpreted this is because of your explanation as to why this should be closed and not supported (emphasis mine):

There are two possible outcomes of the "Shut down debuggee gracefully" command:
...
Bottom line: from a DAP perspective "disconnect gracefully" is possible for the "vetoed" case, but it does not make sense for the un-vetoed case because a "terminate" means "terminate" and not just "disconnect".

In the bolded part you seem to understand the request as something that depends upon the "terminate" request. Based on the section I quoted above, I think this is a mismatch.

The initial request is for a completely parallel flow that would allow a user to attempt to disconnect an attached debuggee "gracefully". For instance, I send a Debuggee a message that says "I'm disconnecting from you" and the debuggee responds with "wait! Error!" then I can't gracefully disconnect (or I need to show a last-minute error). If the user were to hit the disconnect button again, then it would skip to the standard disconnect logic. This would only be the case if some new supportsDisconnectRequest feature were enabled.

You will notice that my understanding has nothing to do with termination. It is simply a way of saying "if we can support "terminating the host gracefully", why not also offer the opportunity to "disconnect from the host gracefully"?

[ericdrobinson]

Since the ultimate goal of the terminate request is to let the debuggee terminate itself gracefully, VS Code made the assumption that terminate needs only to be called in the "launch" case where the debug adapter "owns" the lifecycle of the debuggee and is responsible for terminating the debuggee.

But you are making an assumption here about how all debuggers will work (I'm working on a debugger for which this doesn't make sense, but another interpretation does). You also allow users to click "Stop" instead of "Disconnect" from the Debug Controls when in an "Attach" session. That "Stop" is sent as a request to terminate (in the disconnectRequest() callback). We handle this in our adapter and it is fantastic. What is very weird is that the terminateRequest works from launch requests when I hit that stop button, but not from attach requests. This does not match my expectations as a user.

In the "attach" case the terminate request is not called, because the debug adapter does not own the debuggee's life cycle and is not responsible for terminating the debuggee.

You are making an assumption about how all debuggers/debuggees work. I'm working with a debuggee protocol that allows me to send a signal to terminate at any time, even if the debug adapter was attached. I cannot "gracefully" handle these situations if a user intentionally uses the "Stop" button in an "Attached" adapter session as a result of this decision.

[weinand]

@ericdrobinson since @yannickowow is the author of this request, I'm taking his comment as a confirmation that my interpretation was correct.

For the other concerns please see microsoft/vscode#143991 (comment)

[yannickowow]

On my side yes I assume that your description of this case is correct, especially since you've linked the related issue in VS Code which suits what was the point of this issue. The issue I mainly faced was the way VS Code implements this, since disconnect does not wait for the disconnect response to close the session.