Moving cancel checking into RequestHandler to try to conslidate the cancellation logic.

Author: ashgti

lldb/tools/lldb-dap/DAP.cpp

@@ -47,6 +47,7 @@
 #include <future>
 #include <memory>
 #include <mutex>
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -70,15 +71,6 @@ const char DEV_NULL[] = "/dev/null";
 #endif
 } // namespace
 
-static Response CancelledResponse(int64_t seq, std::string command) {
-  Response response;
-  response.request_seq = seq;
-  response.command = command;
-  response.success = false;
-  response.message = Response::Message::cancelled;
-  return response;
-}
-
 namespace lldb_dap {
 
 DAP::DAP(llvm::StringRef path, Log *log, const ReplMode default_repl_mode,
@@ -253,11 +245,17 @@ void DAP::SendJSON(const llvm::json::Value &json) {
 }
 
 void DAP::Send(const Message &message) {
+  // FIXME: After all the requests have migrated from LegacyRequestHandler >
+  // RequestHandler<> this should be handled in RequestHandler<>::operator().
   if (auto *resp = std::get_if<Response>(&message);
       resp && debugger.InterruptRequested()) {
     // If the debugger was interrupted, convert this response into a 'cancelled'
     // response because we might have a partial result.
-    Response cancelled = CancelledResponse(resp->request_seq, resp->command);
+    Response cancelled{/*request_seq=*/resp->request_seq,
+                       /*command=*/resp->command,
+                       /*success=*/false,
+                       /*message=*/Response::Message::cancelled,
+                       /*body=*/std::nullopt};
     if (llvm::Error err = transport.Write(cancelled))
       DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
                     transport.GetClientName());
@@ -711,20 +709,9 @@ bool DAP::HandleObject(const Message &M) {
       m_active_request = nullptr;
     });
 
-    {
-      // If there is a pending cancelled request, preempt the request and mark
-      // it cancelled.
-      std::lock_guard<std::mutex> lock(m_cancelled_requests_mutex);
-      if (m_cancelled_requests.find(req->seq) != m_cancelled_requests.end()) {
-        Response cancelled = CancelledResponse(req->seq, req->command);
-        Send(cancelled);
-        return true;
-      }
-    }
-
     auto handler_pos = request_handlers.find(req->command);
     if (handler_pos != request_handlers.end()) {
-      (*handler_pos->second)(*req);
+      handler_pos->second->Run(*req);
       return true; // Success
     }
 
@@ -825,6 +812,11 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
   return ToError(error);
 }
 
+bool DAP::IsCancelled(const protocol::Request &req) {
+  std::lock_guard<std::mutex> lock(m_cancelled_requests_mutex);
+  return m_cancelled_requests.find(req.seq) != m_cancelled_requests.end();
+}
+
 void DAP::ClearCancelRequest(const CancelArguments &args) {
   std::lock_guard<std::mutex> cancalled_requests_lock(
       m_cancelled_requests_mutex);

lldb/tools/lldb-dap/DAP.h

@@ -403,6 +403,9 @@ struct DAP {
 
   InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
 
+  /// Checks if the request is cancelled.
+  bool IsCancelled(const protocol::Request &);
+
   /// Clears the cancel request from the set of tracked cancel requests.
   void ClearCancelRequest(const protocol::CancelArguments &);
 

lldb/tools/lldb-dap/Handler/RequestHandler.cpp

@@ -17,6 +17,8 @@
 #include <unistd.h>
 #endif
 
+using namespace lldb_dap::protocol;
+
 namespace lldb_dap {
 
 static std::vector<const char *>
@@ -159,6 +161,28 @@ static llvm::Error RunInTerminal(DAP &dap,
                                  error.GetCString());
 }
 
+void BaseRequestHandler::Run(const Request &request) {
+  // If this request was cancelled, send a cancelled response.
+  if (dap.IsCancelled(request)) {
+    Response cancelled{/*request_seq=*/request.seq,
+                       /*command=*/request.command,
+                       /*success=*/false,
+                       /*message=*/Response::Message::cancelled,
+                       /*body=*/std::nullopt};
+    dap.Send(cancelled);
+    return;
+  }
+
+  // FIXME: After all the requests have migrated from LegacyRequestHandler >
+  // RequestHandler<> we should be able to move this into
+  // RequestHandler<>::operator().
+  operator()(request);
+
+  // FIXME: After all the requests have migrated from LegacyRequestHandler >
+  // RequestHandler<> we should be able to check `debugger.InterruptRequest` and
+  // mark the response as cancelled.
+}
+
 lldb::SBError
 BaseRequestHandler::LaunchProcess(const llvm::json::Object &request) const {
   lldb::SBError error;

lldb/tools/lldb-dap/Handler/RequestHandler.h

@@ -43,6 +43,8 @@ class BaseRequestHandler {
 
   virtual ~BaseRequestHandler() = default;
 
+  void Run(const protocol::Request &);
+
   virtual void operator()(const protocol::Request &request) const = 0;
 
   virtual llvm::StringMap<bool> GetCapabilities() const { return {}; }