debug: disconnect should always end the debug session, even if delve is blocked

[polinasok]

It is possible to get delve to block on one of its blocking RPCs if it is issued while the program is running. Once that happens, delve no longer responds to any requests. But that should never block a disconnect, a way out of any debug session. We cannot rely on the editor to force it (e.g. vscode appears to have a time out while Theia does not). Therefore, we should use a timeout ourselves. We already do this with when halt doesn't return in the launch case in close(). But we do not do this for remote debugging.

For example, I can get both launch and remote debug session stuck and unable to disconnect normally with the following sequence.

1. Start debug session
2. Continue on entry (default)
3. Even though continue button is disabled, you can issue another continue with F5
4. Now pause => this triggers the callback from the first continue
5. The adapter will start handling the pause
6. The debugger will then dequeue and process 2nd continue
7. The adapter still thinks we are in stopped state and wants to send a stopped event, but first, it issues blocking RPCServer.ListGoroutines
8. Delve is now blocked and can't accept any new requests

If you try to disconnect the above launch session, you get:

DisconnectRequest
HaltRequest
Killing debug process manually as we could not halt delve in time
killing debugee (pid: 84940)...
DisconnectRequest to parent
DisconnectResponse

If you try to disconnect the above remote session, you get:

DisconnectRequest

Then vscode times out, but Theia does not.

A disconnect should be a way to reset no matter what.

[gopherbot]

Change https://golang.org/cl/257337 mentions this issue: debugAdapter: Fix bugs that cause remote debug to hang

[polinasok]

@quoctruong @hyangah
We had this issue in the past when the old adapter was integrated with Theia that required us to time-out inside of the TypeScript disconnect handler in case delve was unresponsive. Is there anything we need to consider here with dlv-dap?
The test case that I listed (or any of the requests that Theia had issues with when we ran into this) would not block dlv-dap. I can't think of another way to make dlv-dap unresponsive, but in case it does for some other reason we haven't thought of, what should happen on the client side? I believe in vscode-go extension we detect unresponsive delve and kill it from the extension factory code. Would any of that be reused by other editors like Theia? Should we be adding some integration how-to best practices for non-vscode clients that want to try dlv-dap?

[hyangah]

I am not sure what to do about Theia IDE right now. It has been stuck with old VS Code APIs so recent versions of go extension (that has dlv-dap integration) won't work right now.

For remote connect case (where google cloud code may be interested most), we expect the editor & dlv would directly communicate with each other (bypassing the extension completely).

[polinasok]

That makes sense. In the direct interaction case, the only thing we can do is recommend that the client has a timeout for disconnect on their end. We could document what we did in the extension for a smooth integration, for start-up, teardown, output redirection, to serve as a checklist of things to consider and a recommendation for a solution for other clients that might want to integrate. @suzmue