[Lldb-commits] [lldb] [lldb-dap] Adding support for cancelling a request. (PR #130169)

Wed Mar 19 20:18:46 PDT 2025

JDevlieghere wrote:

I left a few small comments but overall this looks good. 

The approach is slightly different from how I expected you to implement this. Note that I don't know if it's actually better, it's more of a "if I had to implement this, that's how I would have started." You've been working on this for a bit, so I'm trusting your judgement. 

Anyway, what I had in mind when I read your RFC and was doing the RequestHandler work, triggered by a question from @labath, is that instead of having one RequestHandler instance per command (which is what we have today), we could instantiate a RequestHandler per incoming command (or a new object that has a reference to the request hander and stores the arguments). 

The read thread could decode its arguments and store away the protocol in the RequestHandler/new object and add it to the queue. Beyond that, things work pretty much how you've implemented them here, but with the logic that checks if the request has been cancelled in the RequestHandler. More concretely the operator doesn't take arguments anymore, and it's implemented something like this:

```
void operator() {
  if (IsCancelled()) // Check if our request ID is in the cancelled request set.
    RespondCancelled();

  llvm::Expected<Body> body = Run(); 
  if (!body) 
    // Handle error

  if (IsInterrupted()) // Check if the SBDebugger was interrupted. 
  	RespondCancelled(); 
}
```

If we go with the new object (`PendingRequest`?) the request handlers can remain what they are today, or if we want to store the data in them we'll need to find a way to dispatch to them (similar to my comment in yesterday's PR). 

Anyway, like I said, not sure if this is actually better, just sharing my thoughts. 

https://github.com/llvm/llvm-project/pull/130169