debug: provide a way to intercept debug session stop for debuggee to shutdown gracefully #120
Comments
stamblerre
changed the title
stamblerre
added
Debug
|
This year, this extension switched to use the tree-kill's vscode-go/src/debugAdapter/goDebug.ts Line 697 in 9d97bb5 vscode-go/src/utils/processUtils.ts Lines 10 to 27 in 9d97bb5 According to tree-kill's doc, it uses And as discussed in #601 vscode-go/src/debugAdapter/goDebug.ts Lines 721 to 737 in 9d97bb5 I wonder if the current timeout 1000ms is sufficient yet. |
|
@suzmue for reproduce the issue. (for both new/old adapters) |
|
I am able to reproduce this in the old adapter with the same repro case from microsoft/vscode-go#2387 on darwin: main.go: launch.json: Log output: |
|
I posited the idea of a dedicated button for sending specific signals over at microsoft/vscode#110203, so there might be a standardised distinction between stopping debugging and sending SIGINT/SIGTERM, but I might not be explaining myself well-enough over there (and I'm not qualified enough in VSCode internals to validate the advice I am given there). If VSCode offered a different event to indicate an arbitrary signal, and tied the sending of that event to the pressing of a new button on the debug toolbar, could the Go debug provider use that new event to (posix) send SIGINT/SIGTERM, or (win32) invoke If so, perhaps if that request could gain some traction? |
|
FYI -- I added a capture all signals listener to my app - and from VSCode - running through debugger - delve clearly never sends any signals to its debugee ever. Never. Of any kind. There is no solution to this but for that software to be improved or replaced. I'm am very confused reading all of this... Fundamentally, my goal is to respond to the shutdown request - and clean up gracefully - in a golang executable on AMD 64 / Linux hosts. I am getting SIGINT when run from console - but I cannot find any signal that gets sent when I try to "stop" a debug session launched from VSCode (see below). I've taken a few stabs at this - but in the end, my process exists immediately before I ever have a chance to respond to this event (sigterm). Not sure how to improve on this situation? code: var signals = []os.Signal{
syscall.SIGINT,
syscall.SIGQUIT,
syscall.SIGABRT,
syscall.SIGKILL,
syscall.SIGTERM,
syscall.SIGSTOP,
}
osNotify = make(chan os.Signal, 1)
signal.Notify(osNotify, signals...)
sig := <-osNotify
// we never reach here if this is launched from VSCode using debug - and then using the stop button...
signal.Stop(osNotify)Version: 1.52.0-insider |
|
Change https://golang.org/cl/315151 mentions this issue: |
dlv handles SIGINT better than SIGTERM. And, stop using treekill. Treekill sends signal to dlv and its children (debugee and their children, and debugserver). It doesn't look like the debugserver on the mac doesn't seem to finish cleaning up the target before exiting as a response of SIGINT. The target becomes a child of the launchd and remains a zombie. Even when debugserver could handle this gracefully, sending signals to all the children processes may interfere with debug target's attempt to gracefully terminate and doesn't seem like a good idea. Trust dlv's termination handling logic instead. Testing this beyond manual testing is hard. Instead, just check whether the temporary binary is removed that is an indirect sign that delve responded to the signal. Changed DelveDAPOutputAdapter's dispose to take an optional timeout parameter - to use it in testing. And this CL fixes some places in goDebugFactory where null logger object can cause to crash. For go-delve/delve#2445 Updates #120 Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/315151 Trust: Hyang-Ah Hana Kim <hyangah@gmail.com> Trust: Suzy Mueller <suzmue@golang.org> Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> TryBot-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: Suzy Mueller <suzmue@golang.org>
dlv handles SIGINT better than SIGTERM. And, stop using treekill. Treekill sends signal to dlv and its children (debugee and their children, and debugserver). It doesn't look like the debugserver on the mac doesn't seem to finish cleaning up the target before exiting as a response of SIGINT. The target becomes a child of the launchd and remains a zombie. Even when debugserver could handle this gracefully, sending signals to all the children processes may interfere with debug target's attempt to gracefully terminate and doesn't seem like a good idea. Trust dlv's termination handling logic instead. Testing this beyond manual testing is hard. Instead, just check whether the temporary binary is removed that is an indirect sign that delve responded to the signal. Changed DelveDAPOutputAdapter's dispose to take an optional timeout parameter - to use it in testing. And this CL fixes some places in goDebugFactory where null logger object can cause to crash. For go-delve/delve#2445 Updates golang#120 Change-Id: I72de903b27b12ed51079d1c21be89c41d8189d8b Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/315151 Trust: Hyang-Ah Hana Kim <hyangah@gmail.com> Trust: Suzy Mueller <suzmue@golang.org> Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> TryBot-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: Suzy Mueller <suzmue@golang.org>
|
As #120 (comment) and microsoft/vscode#110203 (comment) described, it would be nice if the delve dap implements the Terminate request
That way, in step 4, debugee will have a chance to run the cleanup code, or even users can debug the clean up process. Optionally, it would be nice if users can specify the signal to send to the debugee. |
|
@aarzilli Please see comment on Dec 5, 2020 Is there a way for a user to intercept when debugger detaches and kills debuggee to execute some shutdown/cleanup sequence? Is it always SIGKILL? |
polinasok
changed the title
|
Yes, it's always SIGKILL. Note that the feature requested here is to make the "stop" button send a SIGINT (or SIGTERM, or whatever catchable signal) and then resume the target process and wait for it to finish. Basically to make "stop" into another continue button. At that point you will have the opposite problem: a misbehaving program can not be terminated because it discards the signal we send (or gets stuck cleaning up). |
@aarzilli can you clarify this further? I think DAP clients will send a different request ( If the debuggee doesn't handle the signal, that's ok - the DAP client will send the final
I think it's almost equivalent of resuming the target and running Am I missing something? |
|
Interesting, it does sound like the terminate request should not terminate the debuggee. Seeing that I think the dap server should be changed to respond to a terminate request by doing approximately the same thing it does for a continue request, plus sending a signal. And then it's up to the client to decide what to do if the debuggee does not terminate. |
|
I checked what actually happens in vscode. With Should users be able to specify what signal to send with the TerminateRequest (e.g. SIGTERM or SIGINT)? |
polinasok
added
DA: DlvDAP
NeedsDecision
@polinasok I agree that defaulting to this behavior is not great. How about introducing a new property e.g. Another approach I thought about was just default to
@aarzilli I don't know what to do about Windows either. I think for windows users we ask them to start the process in a separate console, attach to it ( |
No. We don't do anything but we probably should do something. I don't think Ctrl-C works correctly on a non-headless instance of delve on windows, for example. I'm not familiar enough, however. |
|
My dream user scenario - the "debug" bar had a button/key combo to mimic the ctrl-c functionality.
You can mimic the expected behavior with a simple command: |
|
@aronchick -- this is kind of what I was trying to gain traction for with microsoft/vscode#110203, but I was thinker wider than just *nix+SIGINT. The debug bar should offer a unified way to send signals (or other OS-specific messages) to the code being debugged. Debugger implementations should then be able to register the signals it supports (so the debug bar's dropdown/splitbutton could be populated); and then send the signal on to the debugged code when the button is clicked. But I couldn't make my point well enough over there, so it was auto-closed. |
|
Not sure if it belongs here, but it seems related: Doing the same thing with PyCharm works: The test also blocks, but stopping all tests also stops the Python web server that was started in the background as part of the test. My workaround is now to manually look for all Python processes and kill them each time I want to restart the test. A bit cumbersome. |
|
I just lost more time to this than I care to admit (before coming across this issue). It seems that other implementations have found a way: microsoft/vscode-node-debug2#101 It would be great to see something like this for Go! |
|
Almost a year since the last comment. Any update on this? |
|
Looks like supportsTerminateRequest needs to be enabled for non-windows machines. microsoft/vscode-node-debug2@82b0830 |
|
This is still needed! Any idea if it gets on the feature list some time soon? |
|
Perhaps we can use custom options in settings.json/deveConfig/gopls? |
|
We need it! |
and others
As discussed in microsoft/vscode-go#2387, there is a lack of support for graceful shutdown when debugging. This issue is to investigate what can be done to improve this experience.
The text was updated successfully, but these errors were encountered: