[lldb-dap] Migrating 'threads' request to structured types. (#142510)

Moving `threads` request to structured types. Adding helper types for
this and moving helpers from JSONUtils to ProtocolUtils.

---------

Co-authored-by: Ebuka Ezike <[email protected]>
Co-authored-by: Jonas Devlieghere <[email protected]>

Author: 3 people

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -308,7 +308,6 @@ def _handle_recv_packet(self, packet: Optional[ProtocolMessage]) -> bool:
         return keepGoing
 
     def _process_continued(self, all_threads_continued: bool):
-        self.threads = None
         self.frame_scopes = {}
         if all_threads_continued:
             self.thread_stop_reasons = {}
@@ -1180,6 +1179,9 @@ def request_threads(self):
         with information about all threads"""
         command_dict = {"command": "threads", "type": "request", "arguments": {}}
         response = self.send_recv(command_dict)
+        if not response["success"]:
+            self.threads = None
+            return response
         body = response["body"]
         # Fill in "self.threads" correctly so that clients that call
         # self.get_threads() or self.get_thread_id(...) can get information

lldb/tools/lldb-dap/DAP.cpp

@@ -1240,7 +1240,10 @@ void DAP::EventThread() {
             // automatically restarted.
             if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
               SendStdOutStdErr(*this, process);
-              SendThreadStoppedEvent(*this);
+              if (llvm::Error err = SendThreadStoppedEvent(*this))
+                DAP_LOG_ERROR(log, std::move(err),
+                              "({1}) reporting thread stopped: {0}",
+                              transport.GetClientName());
             }
             break;
           case lldb::eStateRunning:

lldb/tools/lldb-dap/DAP.h

@@ -152,7 +152,7 @@ struct DAP {
   llvm::DenseSet<ClientFeature> clientFeatures;
 
   /// The initial thread list upon attaching.
-  std::optional<llvm::json::Array> initial_thread_list;
+  std::vector<protocol::Thread> initial_thread_list;
 
   /// Keep track of all the modules our client knows about: either through the
   /// modules request or the module events.

lldb/tools/lldb-dap/EventHelper.cpp

@@ -8,10 +8,11 @@
 
 #include "EventHelper.h"
 #include "DAP.h"
-#include "DAPLog.h"
+#include "DAPError.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "lldb/API/SBFileSpec.h"
+#include "llvm/Support/Error.h"
 
 #if defined(_WIN32)
 #define NOMINMAX
@@ -22,6 +23,8 @@
 #endif
 #endif
 
+using namespace llvm;
+
 namespace lldb_dap {
 
 static void SendThreadExitedEvent(DAP &dap, lldb::tid_t tid) {
@@ -116,78 +119,78 @@ void SendProcessEvent(DAP &dap, LaunchMethod launch_method) {
 
 // Send a thread stopped event for all threads as long as the process
 // is stopped.
-void SendThreadStoppedEvent(DAP &dap) {
+llvm::Error SendThreadStoppedEvent(DAP &dap, bool on_entry) {
+  lldb::SBMutex lock = dap.GetAPIMutex();
+  std::lock_guard<lldb::SBMutex> guard(lock);
+
   lldb::SBProcess process = dap.target.GetProcess();
-  if (process.IsValid()) {
-    auto state = process.GetState();
-    if (state == lldb::eStateStopped) {
-      llvm::DenseSet<lldb::tid_t> old_thread_ids;
-      old_thread_ids.swap(dap.thread_ids);
-      uint32_t stop_id = process.GetStopID();
-      const uint32_t num_threads = process.GetNumThreads();
-
-      // First make a pass through the threads to see if the focused thread
-      // has a stop reason. In case the focus thread doesn't have a stop
-      // reason, remember the first thread that has a stop reason so we can
-      // set it as the focus thread if below if needed.
-      lldb::tid_t first_tid_with_reason = LLDB_INVALID_THREAD_ID;
-      uint32_t num_threads_with_reason = 0;
-      bool focus_thread_exists = false;
-      for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
-        lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
-        const lldb::tid_t tid = thread.GetThreadID();
-        const bool has_reason = ThreadHasStopReason(thread);
-        // If the focus thread doesn't have a stop reason, clear the thread ID
-        if (tid == dap.focus_tid) {
-          focus_thread_exists = true;
-          if (!has_reason)
-            dap.focus_tid = LLDB_INVALID_THREAD_ID;
-        }
-        if (has_reason) {
-          ++num_threads_with_reason;
-          if (first_tid_with_reason == LLDB_INVALID_THREAD_ID)
-            first_tid_with_reason = tid;
-        }
-      }
+  if (!process.IsValid())
+    return make_error<DAPError>("invalid process");
+
+  lldb::StateType state = process.GetState();
+  if (!lldb::SBDebugger::StateIsStoppedState(state))
+    return make_error<NotStoppedError>();
+
+  llvm::DenseSet<lldb::tid_t> old_thread_ids;
+  old_thread_ids.swap(dap.thread_ids);
+  uint32_t stop_id = process.GetStopID();
+  const uint32_t num_threads = process.GetNumThreads();
+
+  // First make a pass through the threads to see if the focused thread
+  // has a stop reason. In case the focus thread doesn't have a stop
+  // reason, remember the first thread that has a stop reason so we can
+  // set it as the focus thread if below if needed.
+  lldb::tid_t first_tid_with_reason = LLDB_INVALID_THREAD_ID;
+  uint32_t num_threads_with_reason = 0;
+  bool focus_thread_exists = false;
+  for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+    lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
+    const lldb::tid_t tid = thread.GetThreadID();
+    const bool has_reason = ThreadHasStopReason(thread);
+    // If the focus thread doesn't have a stop reason, clear the thread ID
+    if (tid == dap.focus_tid) {
+      focus_thread_exists = true;
+      if (!has_reason)
+        dap.focus_tid = LLDB_INVALID_THREAD_ID;
+    }
+    if (has_reason) {
+      ++num_threads_with_reason;
+      if (first_tid_with_reason == LLDB_INVALID_THREAD_ID)
+        first_tid_with_reason = tid;
+    }
+  }
 
-      // We will have cleared dap.focus_tid if the focus thread doesn't have
-      // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
-      // then set the focus thread to the first thread with a stop reason.
-      if (!focus_thread_exists || dap.focus_tid == LLDB_INVALID_THREAD_ID)
-        dap.focus_tid = first_tid_with_reason;
-
-      // If no threads stopped with a reason, then report the first one so
-      // we at least let the UI know we stopped.
-      if (num_threads_with_reason == 0) {
-        lldb::SBThread thread = process.GetThreadAtIndex(0);
-        dap.focus_tid = thread.GetThreadID();
+  // We will have cleared dap.focus_tid if the focus thread doesn't have
+  // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
+  // then set the focus thread to the first thread with a stop reason.
+  if (!focus_thread_exists || dap.focus_tid == LLDB_INVALID_THREAD_ID)
+    dap.focus_tid = first_tid_with_reason;
+
+  // If no threads stopped with a reason, then report the first one so
+  // we at least let the UI know we stopped.
+  if (num_threads_with_reason == 0) {
+    lldb::SBThread thread = process.GetThreadAtIndex(0);
+    dap.focus_tid = thread.GetThreadID();
+    dap.SendJSON(CreateThreadStopped(dap, thread, stop_id));
+  } else {
+    for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+      lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
+      dap.thread_ids.insert(thread.GetThreadID());
+      if (ThreadHasStopReason(thread)) {
         dap.SendJSON(CreateThreadStopped(dap, thread, stop_id));
-      } else {
-        for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
-          lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
-          dap.thread_ids.insert(thread.GetThreadID());
-          if (ThreadHasStopReason(thread)) {
-            dap.SendJSON(CreateThreadStopped(dap, thread, stop_id));
-          }
-        }
       }
-
-      for (auto tid : old_thread_ids) {
-        auto end = dap.thread_ids.end();
-        auto pos = dap.thread_ids.find(tid);
-        if (pos == end)
-          SendThreadExitedEvent(dap, tid);
-      }
-    } else {
-      DAP_LOG(
-          dap.log,
-          "error: SendThreadStoppedEvent() when process isn't stopped ({0})",
-          lldb::SBDebugger::StateAsCString(state));
     }
-  } else {
-    DAP_LOG(dap.log, "error: SendThreadStoppedEvent() invalid process");
   }
+
+  for (const auto &tid : old_thread_ids) {
+    auto end = dap.thread_ids.end();
+    auto pos = dap.thread_ids.find(tid);
+    if (pos == end)
+      SendThreadExitedEvent(dap, tid);
+  }
+
   dap.RunStopCommands();
+  return Error::success();
 }
 
 // Send a "terminated" event to indicate the process is done being

lldb/tools/lldb-dap/EventHelper.h

@@ -10,6 +10,7 @@
 #define LLDB_TOOLS_LLDB_DAP_EVENTHELPER_H
 
 #include "DAPForward.h"
+#include "llvm/Support/Error.h"
 
 namespace lldb_dap {
 struct DAP;
@@ -18,7 +19,7 @@ enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
 void SendProcessEvent(DAP &dap, LaunchMethod launch_method);
 
-void SendThreadStoppedEvent(DAP &dap);
+llvm::Error SendThreadStoppedEvent(DAP &dap, bool on_entry = false);
 
 void SendTerminatedEvent(DAP &dap);
 

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

@@ -8,8 +8,9 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
-#include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
+#include "ProtocolUtils.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBDebugger.h"
 
@@ -51,11 +52,9 @@ ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const {
   SendProcessEvent(dap, dap.is_attach ? Attach : Launch);
 
   if (dap.stop_at_entry)
-    SendThreadStoppedEvent(dap);
-  else
-    process.Continue();
+    return SendThreadStoppedEvent(dap, /*on_entry=*/true);
 
-  return Error::success();
+  return ToError(process.Continue());
 }
 
 } // namespace lldb_dap

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

@@ -522,11 +522,14 @@ class StackTraceRequestHandler : public LegacyRequestHandler {
   }
 };
 
-class ThreadsRequestHandler : public LegacyRequestHandler {
+class ThreadsRequestHandler
+    : public RequestHandler<protocol::ThreadsArguments,
+                            llvm::Expected<protocol::ThreadsResponseBody>> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "threads"; }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Expected<protocol::ThreadsResponseBody>
+  Run(const protocol::ThreadsArguments &) const override;
 };
 
 class VariablesRequestHandler : public LegacyRequestHandler {

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

@@ -145,7 +145,11 @@ void RestartRequestHandler::operator()(
   // Because we're restarting, configuration has already happened so we can
   // continue the process right away.
   if (dap.stop_at_entry) {
-    SendThreadStoppedEvent(dap);
+    if (llvm::Error err = SendThreadStoppedEvent(dap, /*on_entry=*/true)) {
+      EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
+      dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
   } else {
     dap.target.GetProcess().Continue();
   }

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

@@ -8,72 +8,45 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "ProtocolUtils.h"
 #include "RequestHandler.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBDefines.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+using namespace lldb_dap::protocol;
 
 namespace lldb_dap {
 
-// "ThreadsRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Thread request; value of command field is 'threads'. The
-//     request retrieves a list of all threads.", "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "threads" ]
-//       }
-//     },
-//     "required": [ "command" ]
-//   }]
-// },
-// "ThreadsResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'threads' request.",
-//     "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "threads": {
-//             "type": "array",
-//             "items": {
-//               "$ref": "#/definitions/Thread"
-//             },
-//             "description": "All threads."
-//           }
-//         },
-//         "required": [ "threads" ]
-//       }
-//     },
-//     "required": [ "body" ]
-//   }]
-// }
-void ThreadsRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
+/// The request retrieves a list of all threads.
+Expected<ThreadsResponseBody>
+ThreadsRequestHandler::Run(const ThreadsArguments &) const {
+  lldb::SBProcess process = dap.target.GetProcess();
+  std::vector<Thread> threads;
 
-  llvm::json::Array threads;
   // Client requests the baseline of currently existing threads after
   // a successful launch or attach by sending a 'threads' request
   // right after receiving the configurationDone response.
   // If no thread has reported to the client, it prevents something
   // like the pause request from working in the running state.
   // Return the cache of initial threads as the process might have resumed
-  if (dap.initial_thread_list) {
-    threads = dap.initial_thread_list.value();
-    dap.initial_thread_list.reset();
+  if (!dap.initial_thread_list.empty()) {
+    threads = dap.initial_thread_list;
+    dap.initial_thread_list.clear();
   } else {
-    threads = GetThreads(dap.target.GetProcess(), dap.thread_format);
-  }
+    if (!lldb::SBDebugger::StateIsStoppedState(process.GetState()))
+      return make_error<NotStoppedError>();
 
-  if (threads.size() == 0) {
-    response["success"] = llvm::json::Value(false);
+    threads = GetThreads(process, dap.thread_format);
   }
-  llvm::json::Object body;
-  body.try_emplace("threads", std::move(threads));
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+
+  if (threads.size() == 0)
+    return make_error<DAPError>("failed to retrieve threads from process");
+
+  return ThreadsResponseBody{threads};
 }
 
 } // namespace lldb_dap