-
Notifications
You must be signed in to change notification settings - Fork 133
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
Add support for Checkpoint requests #379
base: main
Are you sure you want to change the base?
Conversation
- Add supportsCheckpointRequests capability. - Add createCheckpoint, deleteCheckpoint and loadCheckpoint Requests.
@iAbadia please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
@microsoft-github-policy-service agree |
I think representing checkpointed processes as Threads might make sense. That allows individual addressability and lifetime management (and UI reuse for consumers) without a ton of new checkpoint-specific calls. What do you think? |
Thank for the reply @connor4312 ! I love the idea of getting UI figured out "for free" but I'm concerned it may not be the best solution in this situation given the fundamental difference between Threads and Checkpoints. Since my use case is GDB in Linux, all this is Linux-specific: While C++ threads are created as Linux threads (they all share a PID and are given individual SPIDs or TIDs, can share resources like memory and file descriptors), GDB handles Checkpoints as different Processes (they each have their own PID, men space, file descriptors, etc.). This makes it very easy to spawn and kill Checkpoints, while with Threads is not so trivial and rarely desired. On the other hand it's true that Checkpoints and Threads are mutually exclusive (See bug 12628), which would work nicely UI-wise (If Threads: Show thread options; else show Checkpoint options) but Checkpoints seems like too much of a niche use case (C/C++ on Linux) to make a convincing argument for increasing the complexity of the common case (Any lang on any platform). |
Yea, that's why I'm trying to see if we can roll it into Threads so client's don't have to do a lot of work (or, realistically, choose not to do the work) to support the niche.
The resource usage and sharing is not something we really care about in the DAP specification, as far as I know. For example, while there is representation of memory, this is given on a variable-by-variable basis and not required to be contiguous for a program. And we do support thread termination already. My question is, are there things that DAP can't express with threads that we'd want to do with checkpoints, and are there things DAP can do with threads that wouldn't be applicable for checkpoints? |
The only thing that can't be done with the current DAP spec is the creation of checkpoints. Checkpoint deletion can be done as Thread termination (a limitation would be that the main or Checkpoint 0 can't be terminated). Checkpoints can be represented as threads (they both have ID and a stack trace). Checkpoint resume can be done as Thread resume (thanks to I'm thinking that an option would be to add Checkpoint Creation as Thread Spawn, and to go with the thread support. |
I'm okay keeping the verbiage as "Create Checkpoint", but it sounds like it's okay for the return value of that to be a thread ID, and then for clients to interact with that thread normally (or, if they want to, they could have special UI handling.) Then we only need a single new request for that. cc @roblourens for thoughts too |
@iAbadia, in case this isn't obvious -- if your goal is just to support this in MIEngine with UI the C++ tools VS Code extension, it doesn't make sense for this to be in the standard, and doing so probably makes your task more difficult. The reasons to add this to the standard are:
|
I would be interested in adding support to Vimspector, but using the threads abstraction seems on the face of it odd to me. It feels like a lot of special casing would be required client side to surface this set of functions in a reasonable and discoverable way. I would be happy perhaps to work with the cpptools team on a prototype to see how the UX turns out. But I suspect that something more like breakpoints is a better model, though I could be wrong. |
I have never used checkpoints in GDB, but in the general concept, checkpoints can have multiple threads, so I would agree with @puremourning that I don't think it makes sense to treat them like threads. |
I have an almost finished version of this. It adds a view like the Breakpoints View on the debug tab. The last bit pending is a way to ask the Watch and Variables view to refresh after loading a Checkpoint. Unfortunately the VSCode Extension API doesn't offer a way to do this and I was going to add support on MIEngine for a InvalidateRequest that would trigger an InvalidateEvent (which those Views listen for and refresh). The API exposes the DebugSession so te extension can issue the InvalidateRequest. I opened this PR as it's tangent to that other piece of work and I thought: why not? |
I think I don't follow why the invalidation is driven by the view, wouldn't the Checkpoint be loaded by the debug adapter anyway, and it would know that it needs to then fire an |
@roblourens After loading the checkpoint the debugger state has changed but the UI views don't know it has so they won't refresh. I asked in vscode-discussions#535 and Extensions can't just reach into the Views and ask them to refresh, which would be ideal. This is why initially I decided to go down to the debug adapter and fire an event from there since these views will receive it. From your comment I see how I could bundle sending an InvalidateEvent after receiving a LoadCheckpoint, which would save having to send an InvalidateRequest. I like this |
Hi, I would like to present this PR to add support in the DAP for Checkpoints. The use case I find for this feature are GDB's Checkpoints.
I'm working on a PR for microsoft/vscode-cpptools to add GUI support for Checkpoints in GDB. I have it working using an
EvaluateRequest
but it would be ideal to use a standard DAP request instead. If this PR goes ahead I'll add the subsequent PR on microsoft/MIEngine to make use of it 🙂