[Lldb-commits] [lldb] [lldb] Make SBProcess thread related actions listen to StopLocker (PR #134339)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 14 18:51:29 PDT 2025
https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/134339
>From 4f0ac25e9e7f93d64940e8af2949569bf9898c4e Mon Sep 17 00:00:00 2001
From: Wanyi Ye <wanyi at meta.com>
Date: Thu, 3 Apr 2025 22:29:13 -0700
Subject: [PATCH 1/3] [lldb] Make SBProcess thread related actions listen to
StopLocker
---
lldb/source/API/SBProcess.cpp | 20 ++++++++++---------
.../tools/lldb-dap/attach/TestDAP_attach.py | 2 +-
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index 23ea449b30cca..ba77b2beed5ea 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -193,10 +193,11 @@ uint32_t SBProcess::GetNumThreads() {
if (process_sp) {
Process::StopLocker stop_locker;
- const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
- num_threads = process_sp->GetThreadList().GetSize(can_update);
+ if (stop_locker.TryLock(&process_sp->GetRunLock())) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
+ num_threads = process_sp->GetThreadList().GetSize();
+ }
}
return num_threads;
@@ -393,11 +394,12 @@ SBThread SBProcess::GetThreadAtIndex(size_t index) {
ProcessSP process_sp(GetSP());
if (process_sp) {
Process::StopLocker stop_locker;
- const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
- thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, can_update);
- sb_thread.SetThread(thread_sp);
+ if (stop_locker.TryLock(&process_sp->GetRunLock())) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
+ thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, false);
+ sb_thread.SetThread(thread_sp);
+ }
}
return sb_thread;
diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
index 9df44cc454d5d..b9fbf2c8d14f9 100644
--- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
+++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
@@ -1,5 +1,5 @@
"""
-Test lldb-dap setBreakpoints request
+Test lldb-dap attach request
"""
>From 13f72497031f3933625e4ad845e231354959f6bf Mon Sep 17 00:00:00 2001
From: Wanyi Ye <wanyi at meta.com>
Date: Tue, 8 Apr 2025 22:52:19 -0700
Subject: [PATCH 2/3] [lldb-dap] Client expects initial threads request not
empty
---
.../test/tools/lldb-dap/dap_server.py | 4 ++++
lldb/tools/lldb-dap/DAP.h | 3 +++
.../ConfigurationDoneRequestHandler.cpp | 10 +++++++++-
.../lldb-dap/Handler/ThreadsRequestHandler.cpp | 18 ++++++++++++------
lldb/tools/lldb-dap/JSONUtils.cpp | 10 ++++++++++
lldb/tools/lldb-dap/JSONUtils.h | 2 ++
6 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 45403e9df8525..61d7fa94479b8 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -649,6 +649,10 @@ def request_configurationDone(self):
response = self.send_recv(command_dict)
if response:
self.configuration_done_sent = True
+ # Client requests the baseline of currently existing threads after
+ # a successful launch or attach.
+ # Kick off the threads request that follows
+ self.request_threads()
return response
def _process_stopped(self):
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 8d32a18fb711e..f126ec5cf2dd8 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -211,6 +211,9 @@ struct DAP {
/// The set of features supported by the connected client.
llvm::DenseSet<ClientFeature> clientFeatures;
+ /// The initial thread list upon attaching
+ std::optional<llvm::json::Array> initial_thread_list;
+
/// Creates a new DAP sessions.
///
/// \param[in] log
diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
index cd120e1fdfaba..f39bbdefdbb95 100644
--- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
@@ -44,6 +44,7 @@ namespace lldb_dap {
// just an acknowledgement, so no body field is required."
// }]
// },
+
void ConfigurationDoneRequestHandler::operator()(
const llvm::json::Object &request) const {
llvm::json::Object response;
@@ -52,8 +53,15 @@ void ConfigurationDoneRequestHandler::operator()(
dap.configuration_done_sent = true;
if (dap.stop_at_entry)
SendThreadStoppedEvent(dap);
- else
+ else {
+ // 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.
+ // Obtain the list of threads before we resume the process
+ dap.initial_thread_list =
+ GetThreads(dap.target.GetProcess(), dap.thread_format);
dap.target.GetProcess().Continue();
+ }
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp
index 2b857f7f6a02b..f4ddf8b90685c 100644
--- a/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp
@@ -50,16 +50,22 @@ namespace lldb_dap {
// }
void ThreadsRequestHandler::operator()(
const llvm::json::Object &request) const {
- lldb::SBProcess process = dap.target.GetProcess();
llvm::json::Object response;
FillResponse(request, response);
- const uint32_t num_threads = process.GetNumThreads();
llvm::json::Array threads;
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
- threads.emplace_back(CreateThread(thread, dap.thread_format));
- }
+ // 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();
+ } else
+ threads = GetThreads(dap.target.GetProcess(), dap.thread_format);
+
if (threads.size() == 0) {
response["success"] = llvm::json::Value(false);
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 7660403666150..4d317974cf2b2 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -870,6 +870,16 @@ llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format) {
return llvm::json::Value(std::move(object));
}
+llvm::json::Array GetThreads(lldb::SBProcess process, lldb::SBFormat &format) {
+ llvm::json::Array threads;
+ const uint32_t num_threads = process.GetNumThreads();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
+ threads.emplace_back(CreateThread(thread, format));
+ }
+ return threads;
+}
+
// "StoppedEvent": {
// "allOf": [ { "$ref": "#/definitions/Event" }, {
// "type": "object",
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index da91797290ff0..b8c53353bf42d 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -414,6 +414,8 @@ llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread,
/// definition outlined by Microsoft.
llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format);
+llvm::json::Array GetThreads(lldb::SBProcess process, lldb::SBFormat &format);
+
/// Create a "StoppedEvent" object for a LLDB thread object.
///
/// This function will fill in the following keys in the returned
>From 37340cb0201fc0309b092e50a884f44634aab47c Mon Sep 17 00:00:00 2001
From: Wanyi <kusmour at gmail.com>
Date: Fri, 11 Apr 2025 16:13:05 -0400
Subject: [PATCH 3/3] Apply suggestions from code review
Co-authored-by: Jonas Devlieghere <jonas at devlieghere.com>
---
lldb/tools/lldb-dap/DAP.h | 2 +-
lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp | 3 ++-
lldb/tools/lldb-dap/JSONUtils.cpp | 3 +++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f126ec5cf2dd8..b79a0d9d0f25c 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -211,7 +211,7 @@ struct DAP {
/// The set of features supported by the connected client.
llvm::DenseSet<ClientFeature> clientFeatures;
- /// The initial thread list upon attaching
+ /// The initial thread list upon attaching.
std::optional<llvm::json::Array> initial_thread_list;
/// Creates a new DAP sessions.
diff --git a/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp
index f4ddf8b90685c..16d797c2ab327 100644
--- a/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp
@@ -63,8 +63,9 @@ void ThreadsRequestHandler::operator()(
if (dap.initial_thread_list) {
threads = dap.initial_thread_list.value();
dap.initial_thread_list.reset();
- } else
+ } else {
threads = GetThreads(dap.target.GetProcess(), dap.thread_format);
+ }
if (threads.size() == 0) {
response["success"] = llvm::json::Value(false);
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4d317974cf2b2..33f10c93d2ada 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -871,6 +871,9 @@ llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format) {
}
llvm::json::Array GetThreads(lldb::SBProcess process, lldb::SBFormat &format) {
+ lldb::SBMutex lock = process.GetTarget().GetAPIMutex();
+ std::lock_guard<lldb::SBMutex> guard(lock);
+
llvm::json::Array threads;
const uint32_t num_threads = process.GetNumThreads();
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
More information about the lldb-commits
mailing list