enable debugging multiple process by default
Currently, vscode-go has two debug adapter, the legacy one and DAP one.
- Both do not support setting follow-exec.
- the legacy one goDebug.ts does call RPCServer.FollowExec https://github.com/go-delve/delve/blob/ab208089dea6b9afd73b2a1ea5a10b08821f65ef/service/rpccommon/suitablemethods.go#L32
-
Suspendedconfig is not in interface.- The legacy one does not expose
suspendedin interface, and therefore it's always false. https://github.com/go-delve/delve/blob/f6e0c3e59bb0d0852844ea06685c88b968ea4f0b/service/rpc2/server.go#L243-L249 and https://github.com/go-delve/delve/blob/f6e0c3e59bb0d0852844ea06685c88b968ea4f0b/service/rpc2/server.go#L259-L263 - In the DAP one,
suspendedis hard-coded as false. https://github.com/go-delve/delve/blob/f6e0c3e59bb0d0852844ea06685c88b968ea4f0b/service/dap/server.go#L1523-L1526
- The legacy one does not expose
So the proposal is:
- enable follow-exec by default
- set suspended as true by default
- return a fake breakpoint to vscode-go/vim-go etc.
related issue: https://github.com/golang/vscode-go/issues/3712, https://github.com/golang/vscode-go/issues/3713
code: https://github.com/Lslightly/delve/tree/fixSuspended
The following idea is naive. I underestimate the complexity of adding support to --init flag.
How about enabling --init flag for dlv dap?
I think DAP is hard to change in short time. Besides, using InitFile as pkg/terminal does can decouple the development of DAP client, like vscode-go, and delve itself.
https://github.com/go-delve/delve/blob/1c00fde3d810522c4c30bc9d17cf8f84a5aa6701/pkg/terminal/terminal.go#L339
https://github.com/go-delve/delve/blob/1c00fde3d810522c4c30bc9d17cf8f84a5aa6701/pkg/terminal/command.go#L108-L116
The roadmap is:
- create a mapping from InitFile to DAP call as pkg/terminal does, i.e. adding pairs of (
aliases,cmdFn) - execute InitFile according to the mapping after creating a session and before listening to requests from DAP clients.
- the DAP client(like vscode-go) can add support to other functions gradually.
The benefits are:
- it allows DAP clients to have a fast setup.
- delve DAP layer can debug separately. (But there is mature testing framework in delve, so this could be less beneficial)
If I don't get it wrong, the best place to add support for executing the initial commands is after LaunchRequest or AttachRequest but before other kinds of request like SetBreakpointsRequest. If there is better location, please figure it out. Thanks.
https://github.com/go-delve/delve/blob/1c00fde3d810522c4c30bc9d17cf8f84a5aa6701/service/dap/server.go#L493-L498
I think DAP is hard to change in short time
All of the changed I suggested in the PR are in the DAP implementation in delve.
To solve the problem as https://github.com/go-delve/delve/pull/4078#discussion_r2293981698 mentioned, I think we have to unverify breakpoints of the switched-from process and make breakpoints of the switched-into process verified.
https://github.com/go-delve/delve/blob/49d65b3f40c2a037e36da5b4ce7283f3ee9112ee/pkg/proc/target_exec.go#L202-L203
To achieve that, I think we can record old selected process before the code above. When we detect a newly selected process, we can send event through fn as the following code in #4075 does. The content to be sent is logical breakpoints of old selected process and new selected process. The logical breakpoints are acquired by dbp.Breakpoints().Logical.
https://github.com/go-delve/delve/blob/49d65b3f40c2a037e36da5b4ce7283f3ee9112ee/pkg/proc/target.go#L594-L603
I don't understand what you are saying, but it sounds strange. I also don't understand what is going on in that video that you posted in https://github.com/go-delve/delve/pull/4078#discussion_r2293981698.
I've updated the comment with explanations of the video and the problem. Sorry for the unclear and hope the current version of comments clearly conveys my thoughts. @aarzilli
Ok, I see what you mean now. The problem is that the loop in addTarget in pkg/proc/target_group.go should be sending EventBreakpointMaterialized events.
For the newly spawned target, it seems too early in addTarget to send back EventBreakpointMaterialized events because t.BinInfo().eventsFn has not been set yet.
I try to use grp.Selected.BinInfo().eventsFn to send back EventBreakpointMaterialized. It works.
But another problem arises, when the child process finishes, the verified breakpoints in child process remain verified.
Maybe eventsFn should even be a member of TargetGroup.
Some additional context on multi-process debugging (https://github.com/go-delve/delve/pull/4078#discussion_r2368349589).
The debug adapter protocol has ProcessEvent and StartDebuggingRequest. I discussed the start debugging request in golang/vscode-go#3712, and there's additional context in microsoft/debug-adapter-protocol#79. It's not clear to me exactly what the process event is supposed to do. I believe the start debugging request is the correct choice for handling forked Go processes; as far as I can tell, the process event is simply informational, "Hey, there's a new process". I don't know if vscode does anything with it. I would like to see a ProcessEvent emitted for any non-Go forked process; one of my projects involves a parent Go process that forks a Node process, and if delve emitted a ProcessEvent I believe I could detect that in an extension and start a NodeJS debugger.