-
Notifications
You must be signed in to change notification settings - Fork 2.2k
dlv dap: stop debug do not close app session #2445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
cc @polinasok @hyangah @suzmue I think this is because of asynchronous requests. |
@zzjin Thank you for giving dlv-dap a try! Please keep your feedback coming and stay tuned as we work on improving it! There are could be a couple of reasons why your app might outlive your debug session.
It always helps to make sure that you have the most recent version of the Go extension (we release as often as once a week). You can even check Go Nightly to try out the latest features. If you want a more detailed analysis of your issue, please also include the following in your report:
|
Hi, thanks for point this,update version info here:
note: I do not attach process but always let dlv-dap start it, here is lanch.json contents: {
"version": "0.2.0",
"configurations": [
{
"name": "DebugGin",
"type": "go",
"request": "launch",
"mode": "auto",
"program": "${workspaceFolder}",
"env": {
"CGO_CFLAGS_ALLOW": "-Xpreprocessor"
},
"buildFlags": "-tags='jsoniter nomsgpack'",
"showLog": true,
"logOutput": "dap",
}
]
} steps to reproduce
|
Thank you for additional details. How do you know that dlv-dap stopped? Does pgrep no longer show such a process? |
after add
Click Stop red button and use
no more logs eleswhere.output dropdown have no filter shows select |
I was under a false impression that logging changes (that would write to OUTPUT) were already in the Nightly, but looks like they are not because we haven't refreshed Nightly for a while. Sorry about the false instructions. @hyangah has more experience with the code in the extension that kills dlv when it is unresponsive. We discussed a some point that when dlv is force-killed, the target (that its child) becomes he child of another process (extension? init?) and survives, so that's probably what you are observing. With some of the ongoing work, we will soon be able to shut things down gracefully in your particular use case, so this should stop happening. I will update this issue once the delve changes are in. But for other cases where we still need to shut things down forcefully, we need a better approach. I am not sure what exact interrupt gets sent now, but the server should shut things down cleanly when killed with Ctrl-C (SIGINT). @hyangah Can we trigger that from the extension instead? |
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 <[email protected]> Trust: Suzy Mueller <[email protected]> Run-TryBot: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Suzy Mueller <[email protected]>
The latest Nightly version contains the change that sends SIGINT instead of SIGTERM. Please try if that helps while we are waiting for the PR2423 to be merged. FYI, the client side tracing mentioned in #2445 (comment) is available in Nightly (golang.go-nightly, not golang.go) and requires |
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 <[email protected]> Trust: Suzy Mueller <[email protected]> Run-TryBot: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Suzy Mueller <[email protected]>
Latest Nightly version do the close properly. |
dlv-dap version
go version
When change vscode to use dlv-dap start debugging one web project works fine,but click stop button dlv-dap stopped but web app session do not killed at all.
prevs dlv works fine.
PS: Its'a simple gin web api service.
The text was updated successfully, but these errors were encountered: