ashgti wrote:
> 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.
I do think this is an interesting approach. I can take a look at this as trying
to wrap some of this into some helpers or maybe making the RequestHandler
allocate a new instance per request.
My logic behind the `lldb_dap::protocol` types was to keep them as POD data and
to not have much (if any) logic in them so that if we need to move the data
between threads or queues we can be relatively confident that we're not dealing
data races. In swift terms (I'm more familiar with swift than c++), the
`lldb_dap::protocol` types would be `struct`'s that are all `Sendable`.
That may not be as idiomatic in c++ though.
I do like the more simplified `operator()` idea to help deal with the cancelled
/ interrupted, my existing solution was done to support the
LegacyRequestHandlers since they're sending the responses as raw
`json::Value`'s still, but with the more unified approach we could handle the
cancellation in the `operator()` body.
I'll try to see if I can come up with a `PendingRequest` or maybe take a look
at allocating the `RequestHandler`'s in the queue.
https://github.com/llvm/llvm-project/pull/130169
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
