-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
dlv dap: --accept-multiclient and --continue support #2323
Comments
For |
I understand. I am not even sure it is a good idea to define very different flags for DAP because
Again both are for the case where the debugger is tracking a single process. There is also a proposal for the debugger to be a long-running server that is to stay running on disconnect to accept more client connections, initiating a new debug session to launch/attach to a new process (#2329). |
Instead of adding separate |
Do you want to keep this open because of the issues with synchronizing run status and breakpoints between multiple connected clients? |
I debated closing this and the remote issue yesterday, but decided to do a little bit more testing before I do so. We have not been planning to synchronize the UI, at least not in the short term. What do you think? |
I think it's fine to not do it. |
Is there a way to use the --continue with the dap? I want to use dap but I don't want to wait for a client to connect before continuing the code. Using the dlv exec with --accept-multiclient and --continue works perfectly. |
Yes, there is a way. And we are anxiously waiting for user feedback to iron out any kinks there might be. |
Updates go-delve/delve#2323 Change-Id: I4c89c299ffa569bb3ecbcb0e730f2f7ad04e16d8 GitHub-Last-Rev: 26bb7f9 GitHub-Pull-Request: #1979 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/374615 Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Reviewed-by: Polina Sokolova <polina@google.com>
I am going to close this issue. The feature got released in both dlv and vscode-go, so if users run into problems, they can file new issues. |
With headless dlv, the
--accept-multiclient
feature allows for multiple clients to connect to a server debugging a single process. It is used in 3 scenarios:--continue
(triggered by a quick behind-the-scenes client)This comes into play when a user hits a limitation in vscode UI. In that case, they might also connect with dlv-connect or even GoLand to take advantage of their additional features (e.g. to use
config
command, print variables in flattened text form, etc). We recommend this method when users ask to expose dlv cli features via the Debug Console (which is non-trivial due to the split architecture). And we have been considering streamlining the double client option via the extension to help with this use case (as opposed to reimplementing the dlv cli on the adapter/debugger side). This mode comes with limitations and risks due to lack of client UI synchronization - see accept-multiclient: concurrent clients can get the server and each other stuck #2322.In all cases, delve starts debugging a process as part of server start-up outside of any client. And with this option, the server doesn't automatically exit when any and all client connections are gone, but can continue running, with the target suspended or running, waiting for another client to connect.
Unfortunately, this model was not considered by DAP designers (as recently confirmed with @weinand offline). First of all, DAP clients are supposed to always send a
launch
/attach
request to start the debug session, which doesn't make sense if the debugger already launched/attached to a process. Secondly, there is no DAP request to add a breakpoint to the list of already set breakpoints. There is only asetBreakpoint
request, which clears all previous breakpoints and sets all specified breakpoints. That means that concurrent and sequential clients will clear each other's breakpoints. Thirdly, it will be tricky if not impossible to keep the clients' UIs in sync with what is actually happening, which might lead to untimely requests and issues like in #2322. We might be able to accomplish some UI synchronization with optional DAP events likecontinued
event, but with DAP not actively supporting this use case, there will likely be situations where the right events are not available.We could consider separating support for sequential (covers 1 & 2 above) and parallel (3 above) clients and only support the sequential case to aid remote debugging to begin with. I would even argue that the two cases belong behind two different flags, but since the flag is already in use and comes with certain expectations, we could reject concurrent clients by sending an explicit error message in response to their initialize requests to let the users know that this use case is not (yet?) supported.
The text was updated successfully, but these errors were encountered: