Skip to content

Conversation

@JDevlieghere
Copy link
Member

Move the event and progress event threads into the DAP class. Currently both are implemented in the InitializeRequestHandler, but it doesn't really belong there because the threads are not tied to that request, they just launch them.

@llvmbot
Copy link
Member

llvmbot commented May 8, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Move the event and progress event threads into the DAP class. Currently both are implemented in the InitializeRequestHandler, but it doesn't really belong there because the threads are not tied to that request, they just launch them.


Patch is 25.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/139167.diff

3 Files Affected:

  • (modified) lldb/tools/lldb-dap/DAP.cpp (+254-7)
  • (modified) lldb/tools/lldb-dap/DAP.h (+12-2)
  • (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+3-250)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 62c60cc3a9b3b..7aa2f03894504 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -8,6 +8,7 @@
 
 #include "DAP.h"
 #include "DAPLog.h"
+#include "EventHelper.h"
 #include "Handler/RequestHandler.h"
 #include "Handler/ResponseHandler.h"
 #include "JSONUtils.h"
@@ -20,6 +21,7 @@
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBEvent.h"
 #include "lldb/API/SBLanguageRuntime.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
@@ -52,6 +54,7 @@
 #include <mutex>
 #include <optional>
 #include <string>
+#include <thread>
 #include <utility>
 #include <variant>
 
@@ -77,6 +80,48 @@ const char DEV_NULL[] = "/dev/null";
 
 namespace lldb_dap {
 
+static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
+                                               const char *key) {
+  lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+  if (!keyValue)
+    return std::string();
+
+  const size_t length = keyValue.GetStringValue(nullptr, 0);
+
+  if (length == 0)
+    return std::string();
+
+  std::string str(length + 1, 0);
+  keyValue.GetStringValue(&str[0], length + 1);
+  return str;
+}
+
+static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
+                                          const char *key) {
+  lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+
+  if (!keyValue.IsValid())
+    return 0;
+  return keyValue.GetUnsignedIntegerValue();
+}
+
+static llvm::StringRef GetModuleEventReason(uint32_t event_mask) {
+  if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded)
+    return "new";
+  if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded)
+    return "removed";
+  assert(event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded ||
+         event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged);
+  return "changed";
+}
+
+/// Return string with first character capitalized.
+static std::string capitalize(llvm::StringRef str) {
+  if (str.empty())
+    return "";
+  return ((llvm::Twine)llvm::toUpper(str[0]) + str.drop_front()).str();
+}
+
 llvm::StringRef DAP::debug_adapter_path = "";
 
 DAP::DAP(Log *log, const ReplMode default_repl_mode,
@@ -94,13 +139,6 @@ DAP::DAP(Log *log, const ReplMode default_repl_mode,
 
 DAP::~DAP() = default;
 
-/// Return string with first character capitalized.
-static std::string capitalize(llvm::StringRef str) {
-  if (str.empty())
-    return "";
-  return ((llvm::Twine)llvm::toUpper(str[0]) + str.drop_front()).str();
-}
-
 void DAP::PopulateExceptionBreakpoints() {
   llvm::call_once(init_exception_breakpoints_flag, [this]() {
     exception_breakpoints = std::vector<ExceptionBreakpoint>{};
@@ -1390,4 +1428,213 @@ protocol::Capabilities DAP::GetCapabilities() {
   return capabilities;
 }
 
+void DAP::StartEventThread() {
+  event_thread = std::thread(&DAP::EventThread, this);
+}
+
+void DAP::StartProgressEventThread() {
+  progress_event_thread = std::thread(&DAP::ProgressEventThread, this);
+}
+
+void DAP::ProgressEventThread() {
+  lldb::SBListener listener("lldb-dap.progress.listener");
+  debugger.GetBroadcaster().AddListener(
+      listener, lldb::SBDebugger::eBroadcastBitProgress |
+                    lldb::SBDebugger::eBroadcastBitExternalProgress);
+  broadcaster.AddListener(listener, eBroadcastBitStopProgressThread);
+  lldb::SBEvent event;
+  bool done = false;
+  while (!done) {
+    if (listener.WaitForEvent(1, event)) {
+      const auto event_mask = event.GetType();
+      if (event.BroadcasterMatchesRef(broadcaster)) {
+        if (event_mask & eBroadcastBitStopProgressThread) {
+          done = true;
+        }
+      } else {
+        lldb::SBStructuredData data =
+            lldb::SBDebugger::GetProgressDataFromEvent(event);
+
+        const uint64_t progress_id =
+            GetUintFromStructuredData(data, "progress_id");
+        const uint64_t completed = GetUintFromStructuredData(data, "completed");
+        const uint64_t total = GetUintFromStructuredData(data, "total");
+        const std::string details =
+            GetStringFromStructuredData(data, "details");
+
+        if (completed == 0) {
+          if (total == UINT64_MAX) {
+            // This progress is non deterministic and won't get updated until it
+            // is completed. Send the "message" which will be the combined title
+            // and detail. The only other progress event for thus
+            // non-deterministic progress will be the completed event So there
+            // will be no need to update the detail.
+            const std::string message =
+                GetStringFromStructuredData(data, "message");
+            SendProgressEvent(progress_id, message.c_str(), completed, total);
+          } else {
+            // This progress is deterministic and will receive updates,
+            // on the progress creation event VSCode will save the message in
+            // the create packet and use that as the title, so we send just the
+            // title in the progressCreate packet followed immediately by a
+            // detail packet, if there is any detail.
+            const std::string title =
+                GetStringFromStructuredData(data, "title");
+            SendProgressEvent(progress_id, title.c_str(), completed, total);
+            if (!details.empty())
+              SendProgressEvent(progress_id, details.c_str(), completed, total);
+          }
+        } else {
+          // This progress event is either the end of the progress dialog, or an
+          // update with possible detail. The "detail" string we send to VS Code
+          // will be appended to the progress dialog's initial text from when it
+          // was created.
+          SendProgressEvent(progress_id, details.c_str(), completed, total);
+        }
+      }
+    }
+  }
+}
+
+// All events from the debugger, target, process, thread and frames are
+// received in this function that runs in its own thread. We are using a
+// "FILE *" to output packets back to VS Code and they have mutexes in them
+// them prevent multiple threads from writing simultaneously so no locking
+// is required.
+void DAP::EventThread() {
+  llvm::set_thread_name(transport.GetClientName() + ".event_handler");
+  lldb::SBEvent event;
+  lldb::SBListener listener = debugger.GetListener();
+  broadcaster.AddListener(listener, eBroadcastBitStopEventThread);
+  debugger.GetBroadcaster().AddListener(
+      listener, lldb::eBroadcastBitError | lldb::eBroadcastBitWarning);
+  bool done = false;
+  while (!done) {
+    if (listener.WaitForEvent(1, event)) {
+      const auto event_mask = event.GetType();
+      if (lldb::SBProcess::EventIsProcessEvent(event)) {
+        lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event);
+        if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) {
+          auto state = lldb::SBProcess::GetStateFromEvent(event);
+          switch (state) {
+          case lldb::eStateConnected:
+          case lldb::eStateDetached:
+          case lldb::eStateInvalid:
+          case lldb::eStateUnloaded:
+            break;
+          case lldb::eStateAttaching:
+          case lldb::eStateCrashed:
+          case lldb::eStateLaunching:
+          case lldb::eStateStopped:
+          case lldb::eStateSuspended:
+            // Only report a stopped event if the process was not
+            // automatically restarted.
+            if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
+              SendStdOutStdErr(*this, process);
+              SendThreadStoppedEvent(*this);
+            }
+            break;
+          case lldb::eStateRunning:
+          case lldb::eStateStepping:
+            WillContinue();
+            SendContinuedEvent(*this);
+            break;
+          case lldb::eStateExited:
+            lldb::SBStream stream;
+            process.GetStatus(stream);
+            SendOutput(OutputType::Console, stream.GetData());
+
+            // When restarting, we can get an "exited" event for the process we
+            // just killed with the old PID, or even with no PID. In that case
+            // we don't have to terminate the session.
+            if (process.GetProcessID() == LLDB_INVALID_PROCESS_ID ||
+                process.GetProcessID() == restarting_process_id) {
+              restarting_process_id = LLDB_INVALID_PROCESS_ID;
+            } else {
+              // Run any exit LLDB commands the user specified in the
+              // launch.json
+              RunExitCommands();
+              SendProcessExitedEvent(*this, process);
+              SendTerminatedEvent();
+              done = true;
+            }
+            break;
+          }
+        } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) ||
+                   (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) {
+          SendStdOutStdErr(*this, process);
+        }
+      } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
+        if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
+            event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
+            event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded ||
+            event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) {
+          llvm::StringRef reason = GetModuleEventReason(event_mask);
+          const uint32_t num_modules =
+              lldb::SBTarget::GetNumModulesFromEvent(event);
+          for (uint32_t i = 0; i < num_modules; ++i) {
+            lldb::SBModule module =
+                lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+            if (!module.IsValid())
+              continue;
+
+            llvm::json::Object body;
+            body.try_emplace("reason", reason);
+            body.try_emplace("module", CreateModule(target, module));
+            llvm::json::Object module_event = CreateEventObject("module");
+            module_event.try_emplace("body", std::move(body));
+            SendJSON(llvm::json::Value(std::move(module_event)));
+          }
+        }
+      } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) {
+        if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
+          auto event_type =
+              lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
+          auto bp = Breakpoint(
+              *this, lldb::SBBreakpoint::GetBreakpointFromEvent(event));
+          // If the breakpoint was set through DAP, it will have the
+          // BreakpointBase::kDAPBreakpointLabel. Regardless of whether
+          // locations were added, removed, or resolved, the breakpoint isn't
+          // going away and the reason is always "changed".
+          if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
+               event_type & lldb::eBreakpointEventTypeLocationsRemoved ||
+               event_type & lldb::eBreakpointEventTypeLocationsResolved) &&
+              bp.MatchesName(BreakpointBase::kDAPBreakpointLabel)) {
+            // As the DAP client already knows the path of this breakpoint, we
+            // don't need to send it back as part of the "changed" event. This
+            // avoids sending paths that should be source mapped. Note that
+            // CreateBreakpoint doesn't apply source mapping and certain
+            // implementation ignore the source part of this event anyway.
+            llvm::json::Value source_bp = CreateBreakpoint(&bp);
+            source_bp.getAsObject()->erase("source");
+
+            llvm::json::Object body;
+            body.try_emplace("breakpoint", source_bp);
+            body.try_emplace("reason", "changed");
+
+            llvm::json::Object bp_event = CreateEventObject("breakpoint");
+            bp_event.try_emplace("body", std::move(body));
+
+            SendJSON(llvm::json::Value(std::move(bp_event)));
+          }
+        }
+      } else if (event_mask & lldb::eBroadcastBitError ||
+                 event_mask & lldb::eBroadcastBitWarning) {
+        lldb::SBStructuredData data =
+            lldb::SBDebugger::GetDiagnosticFromEvent(event);
+        if (!data.IsValid())
+          continue;
+        std::string type = GetStringValue(data.GetValueForKey("type"));
+        std::string message = GetStringValue(data.GetValueForKey("message"));
+        SendOutput(OutputType::Important,
+                       llvm::formatv("{0}: {1}", type, message).str());
+      } else if (event.BroadcasterMatchesRef(broadcaster)) {
+        if (event_mask & eBroadcastBitStopEventThread) {
+          done = true;
+        }
+      }
+    }
+  }
+}
+
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index b581ae759b1bc..afeda8d81efb0 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -167,8 +167,6 @@ struct DAP {
   lldb::SBTarget target;
   Variables variables;
   lldb::SBBroadcaster broadcaster;
-  std::thread event_thread;
-  std::thread progress_event_thread;
   llvm::StringMap<SourceBreakpointMap> source_breakpoints;
   FunctionBreakpointMap function_breakpoints;
   InstructionBreakpointMap instruction_breakpoints;
@@ -418,7 +416,19 @@ struct DAP {
 
   lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); }
 
+  void StartEventThread();
+  void StartProgressEventThread();
+
 private:
+  /// Event threads.
+  /// @{
+  void EventThread();
+  void ProgressEventThread();
+
+  std::thread event_thread;
+  std::thread progress_event_thread;
+  /// @}
+
   /// Queue for all incoming messages.
   std::deque<protocol::Message> m_queue;
   std::deque<protocol::Message> m_pending_queue;
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index aa947d3cb5ab9..cdcbdfa7f48e3 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -12,255 +12,11 @@
 #include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
-#include "lldb/API/SBEvent.h"
-#include "lldb/API/SBListener.h"
-#include "lldb/API/SBStream.h"
 #include "lldb/API/SBTarget.h"
-#include <cstdint>
 
-using namespace lldb;
+using namespace lldb_dap;
 using namespace lldb_dap::protocol;
 
-namespace lldb_dap {
-
-static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
-                                               const char *key) {
-  lldb::SBStructuredData keyValue = data.GetValueForKey(key);
-  if (!keyValue)
-    return std::string();
-
-  const size_t length = keyValue.GetStringValue(nullptr, 0);
-
-  if (length == 0)
-    return std::string();
-
-  std::string str(length + 1, 0);
-  keyValue.GetStringValue(&str[0], length + 1);
-  return str;
-}
-
-static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
-                                          const char *key) {
-  lldb::SBStructuredData keyValue = data.GetValueForKey(key);
-
-  if (!keyValue.IsValid())
-    return 0;
-  return keyValue.GetUnsignedIntegerValue();
-}
-
-void ProgressEventThreadFunction(DAP &dap) {
-  lldb::SBListener listener("lldb-dap.progress.listener");
-  dap.debugger.GetBroadcaster().AddListener(
-      listener, lldb::SBDebugger::eBroadcastBitProgress |
-                    lldb::SBDebugger::eBroadcastBitExternalProgress);
-  dap.broadcaster.AddListener(listener, eBroadcastBitStopProgressThread);
-  lldb::SBEvent event;
-  bool done = false;
-  while (!done) {
-    if (listener.WaitForEvent(1, event)) {
-      const auto event_mask = event.GetType();
-      if (event.BroadcasterMatchesRef(dap.broadcaster)) {
-        if (event_mask & eBroadcastBitStopProgressThread) {
-          done = true;
-        }
-      } else {
-        lldb::SBStructuredData data =
-            lldb::SBDebugger::GetProgressDataFromEvent(event);
-
-        const uint64_t progress_id =
-            GetUintFromStructuredData(data, "progress_id");
-        const uint64_t completed = GetUintFromStructuredData(data, "completed");
-        const uint64_t total = GetUintFromStructuredData(data, "total");
-        const std::string details =
-            GetStringFromStructuredData(data, "details");
-
-        if (completed == 0) {
-          if (total == UINT64_MAX) {
-            // This progress is non deterministic and won't get updated until it
-            // is completed. Send the "message" which will be the combined title
-            // and detail. The only other progress event for thus
-            // non-deterministic progress will be the completed event So there
-            // will be no need to update the detail.
-            const std::string message =
-                GetStringFromStructuredData(data, "message");
-            dap.SendProgressEvent(progress_id, message.c_str(), completed,
-                                  total);
-          } else {
-            // This progress is deterministic and will receive updates,
-            // on the progress creation event VSCode will save the message in
-            // the create packet and use that as the title, so we send just the
-            // title in the progressCreate packet followed immediately by a
-            // detail packet, if there is any detail.
-            const std::string title =
-                GetStringFromStructuredData(data, "title");
-            dap.SendProgressEvent(progress_id, title.c_str(), completed, total);
-            if (!details.empty())
-              dap.SendProgressEvent(progress_id, details.c_str(), completed,
-                                    total);
-          }
-        } else {
-          // This progress event is either the end of the progress dialog, or an
-          // update with possible detail. The "detail" string we send to VS Code
-          // will be appended to the progress dialog's initial text from when it
-          // was created.
-          dap.SendProgressEvent(progress_id, details.c_str(), completed, total);
-        }
-      }
-    }
-  }
-}
-
-static llvm::StringRef GetModuleEventReason(uint32_t event_mask) {
-  if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded)
-    return "new";
-  if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded)
-    return "removed";
-  assert(event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded ||
-         event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged);
-  return "changed";
-}
-
-// All events from the debugger, target, process, thread and frames are
-// received in this function that runs in its own thread. We are using a
-// "FILE *" to output packets back to VS Code and they have mutexes in them
-// them prevent multiple threads from writing simultaneously so no locking
-// is required.
-static void EventThreadFunction(DAP &dap) {
-  llvm::set_thread_name(dap.transport.GetClientName() + ".event_handler");
-  lldb::SBEvent event;
-  lldb::SBListener listener = dap.debugger.GetListener();
-  dap.broadcaster.AddListener(listener, eBroadcastBitStopEventThread);
-  dap.debugger.GetBroadcaster().AddListener(listener, eBroadcastBitError |
-                                                          eBroadcastBitWarning);
-  bool done = false;
-  while (!done) {
-    if (listener.WaitForEvent(1, event)) {
-      const auto event_mask = event.GetType();
-      if (lldb::SBProcess::EventIsProcessEvent(event)) {
-        lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event);
-        if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) {
-          auto state = lldb::SBProcess::GetStateFromEvent(event);
-
-          DAP_LOG(dap.log, "State = {0}", state);
-          switch (state) {
-          case lldb::eStateConnected:
-          case lldb::eStateDetached:
-          case lldb::eStateInvalid:
-          case lldb::eStateUnloaded:
-            break;
-          case lldb::eStateAttaching:
-          case lldb::eStateCrashed:
-          case lldb::eStateLaunching:
-          case lldb::eStateStopped:
-          case lldb::eStateSuspended:
-            // Only report a stopped event if the process was not
-            // automatically restarted.
-            if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
-              SendStdOutStdErr(dap, process);
-              SendThreadStoppedEvent(dap);
-            }
-            br...
[truncated]

@github-actions
Copy link

github-actions bot commented May 8, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Move the event and progress event threads into the DAP class. Currently
both are implemented in the InitializeRequestHandler, but it doesn't
really belong there because the threads are not tied to that request,
they just launch them.
@JDevlieghere JDevlieghere force-pushed the lldb-dap-event-thread branch from de98615 to 724a9e9 Compare May 8, 2025 22:47
Copy link
Contributor

@ashgti ashgti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me

@JDevlieghere JDevlieghere merged commit 60b62c6 into llvm:main May 9, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants