[Lldb-commits] [lldb] [lldb-dap] Change the launch sequence (PR #138219)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Thu May 1 16:54:55 PDT 2025
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/138219
This PR changes how we treat the launch sequence in lldb-dap.
- Send the initialized event after we finish handling the initialize request, rather than after we finish attaching or launching.
- Delay handling the launch and attach request until we have received the configurationDone request. The latter is now largely a NO-OP and only exists to signal lldb-dap that it can handle the launch and attach requests.
- Add synchronization for ignoring the first stop event. I originally wanted to do all the launching and attaching in asynchronous mode, but that's a little bit trickier than expected. On macOS, I'm getting an additional stop when we hit the dyld breakpoint. This might be something I come back to, but for now I'm keeping the old behavior with the proposed synchronization from #137920.
Background: https://discourse.llvm.org/t/reliability-of-the-lldb-dap-tests/86125
>From 730fdebae55f38e2d1456101807836bd001aec0b Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 1 May 2025 15:11:48 -0700
Subject: [PATCH] [lldb-dap] Change the launch sequence
This PR changes how we treat the launch sequence in lldb-dap.
- Send the initialized event after we finish handling the initialize
request, rather than after we finish attaching or launching.
- Delay handling the launch and attach request until we have received
the configurationDone request. The latter is now largely a NO-OP and
only exists to signal lldb-dap that it can handle the launch and
attach requests.
- Add synchronization for ignoring the first stop event. I originally
wanted to do all the launching and attaching in asynchronous mode,
but that's a little bit trickier than expected. On macOS, I'm getting
an additional stop when we hit the dyld breakpoint. This might be
something I come back to, but for now I'm keeping the old behavior
with the proposed synchronization from #137920.
Background: https://discourse.llvm.org/t/reliability-of-the-lldb-dap-tests/86125
---
lldb/tools/lldb-dap/DAP.cpp | 37 +++++++++++++------
lldb/tools/lldb-dap/DAP.h | 14 ++++++-
lldb/tools/lldb-dap/EventHelper.cpp | 2 +-
.../lldb-dap/Handler/AttachRequestHandler.cpp | 28 +++++++++++---
.../ConfigurationDoneRequestHandler.cpp | 14 +------
.../Handler/InitializeRequestHandler.cpp | 29 +++++++++------
.../lldb-dap/Handler/LaunchRequestHandler.cpp | 2 -
.../tools/lldb-dap/Handler/RequestHandler.cpp | 30 ++++++++++++---
lldb/tools/lldb-dap/Handler/RequestHandler.h | 1 +
9 files changed, 106 insertions(+), 51 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 4cb0d8e49004c..237b16f8c45a9 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -84,8 +84,8 @@ DAP::DAP(Log *log, const ReplMode default_repl_mode,
: log(log), transport(transport), broadcaster("lldb-dap"),
exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
stop_at_entry(false), is_attach(false),
- restarting_process_id(LLDB_INVALID_PROCESS_ID),
- configuration_done_sent(false), waiting_for_run_in_terminal(false),
+ restarting_process_id(LLDB_INVALID_PROCESS_ID), configuration_done(false),
+ waiting_for_run_in_terminal(false),
progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
reverse_request_seq(0), repl_mode(default_repl_mode) {
@@ -892,11 +892,14 @@ llvm::Error DAP::Loop() {
errWrapper.SetErrorString(llvm::toString(std::move(err)).c_str());
return errWrapper;
}
+ bool is_launch_attach = false;
if (const protocol::Request *req =
- std::get_if<protocol::Request>(&*next);
- req && req->command == "disconnect") {
- disconnecting = true;
+ std::get_if<protocol::Request>(&*next)) {
+ if (req->command == "disconnect")
+ disconnecting = true;
+ if (req->command == "launch" || req->command == "attach")
+ is_launch_attach = true;
}
const std::optional<CancelArguments> cancel_args =
@@ -924,7 +927,10 @@ llvm::Error DAP::Loop() {
{
std::lock_guard<std::mutex> guard(m_queue_mutex);
- m_queue.push_back(std::move(*next));
+ if (is_launch_attach)
+ m_launch_attach_queue.push_back(std::move(*next));
+ else
+ m_queue.push_back(std::move(*next));
}
m_queue_cv.notify_one();
}
@@ -938,15 +944,24 @@ llvm::Error DAP::Loop() {
StopEventHandlers();
});
- while (!disconnecting || !m_queue.empty()) {
+ while (true) {
std::unique_lock<std::mutex> lock(m_queue_mutex);
- m_queue_cv.wait(lock, [&] { return disconnecting || !m_queue.empty(); });
+ m_queue_cv.wait(lock, [&] {
+ return disconnecting || !m_queue.empty() ||
+ (configuration_done && !m_launch_attach_queue.empty());
+ });
- if (m_queue.empty())
+ if (disconnecting)
break;
- Message next = m_queue.front();
- m_queue.pop_front();
+ auto &active_queue = configuration_done && !m_launch_attach_queue.empty()
+ ? m_launch_attach_queue
+ : m_queue;
+
+ assert(!active_queue.empty());
+
+ Message next = active_queue.front();
+ active_queue.pop_front();
if (!HandleObject(next))
return llvm::createStringError(llvm::inconvertibleErrorCode(),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 88eedb0860cf1..65c2426aafe40 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -188,7 +188,7 @@ struct DAP {
// shutting down the entire adapter. When we're restarting, we keep the id of
// the old process here so we can detect this case and keep running.
lldb::pid_t restarting_process_id;
- bool configuration_done_sent;
+ bool configuration_done;
llvm::StringMap<std::unique_ptr<BaseRequestHandler>> request_handlers;
bool waiting_for_run_in_terminal;
ProgressEventReporter progress_event_reporter;
@@ -214,6 +214,13 @@ struct DAP {
/// The initial thread list upon attaching.
std::optional<llvm::json::Array> initial_thread_list;
+ /// Synchronization of stop events between the event and request thread.
+ /// @{
+ std::mutex ignore_stop_mutex;
+ std::condition_variable ignore_stop_cv;
+ bool ignore_next_stop;
+ /// @}
+
/// Creates a new DAP sessions.
///
/// \param[in] log
@@ -417,8 +424,11 @@ struct DAP {
lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); }
private:
- std::mutex m_queue_mutex;
+ /// Queue for all incoming messages.
std::deque<protocol::Message> m_queue;
+ /// Dedicated queue for launching and attaching.
+ std::deque<protocol::Message> m_launch_attach_queue;
+ std::mutex m_queue_mutex;
std::condition_variable m_queue_cv;
std::mutex m_cancelled_requests_mutex;
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 2c659f39f4b66..ed2d8700c26b0 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -222,7 +222,7 @@ void SendContinuedEvent(DAP &dap) {
// If the focus thread is not set then we haven't reported any thread status
// to the client, so nothing to report.
- if (!dap.configuration_done_sent || dap.focus_tid == LLDB_INVALID_THREAD_ID) {
+ if (!dap.configuration_done || dap.focus_tid == LLDB_INVALID_THREAD_ID) {
return;
}
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 7084d30f2625b..8ded3c25f0a80 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -128,6 +128,9 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
return;
}
+ std::unique_lock<std::mutex> lock(dap.ignore_stop_mutex);
+ dap.ignore_next_stop = !dap.stop_at_entry;
+
if ((pid == LLDB_INVALID_PROCESS_ID || gdb_remote_port == invalid_port) &&
wait_for) {
char attach_msg[256];
@@ -181,9 +184,24 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
// selected target after these commands are run.
dap.target = dap.debugger.GetSelectedTarget();
- // Make sure the process is attached and stopped before proceeding as the
- // the launch commands are not run using the synchronous mode.
- error = dap.WaitForProcessToStop(std::chrono::seconds(timeout_seconds));
+ }
+
+ // Notify the event thread that it should ignore the next stop event.
+ lock.unlock();
+ dap.ignore_stop_cv.notify_one();
+
+ // Make sure the process is attached and stopped before proceeding.
+ error = dap.WaitForProcessToStop(std::chrono::seconds(timeout_seconds));
+
+ // FIXME: Unify this between launch & attach.
+ if (dap.stop_at_entry) {
+ // 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();
}
if (error.Success() && core_file.empty()) {
@@ -204,10 +222,8 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
}
dap.SendJSON(llvm::json::Value(std::move(response)));
- if (error.Success()) {
+ if (error.Success())
SendProcessEvent(dap, Attach);
- dap.SendJSON(CreateEventObject("initialized"));
- }
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
index f39bbdefdbb95..a10cd0c832782 100644
--- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
@@ -47,21 +47,11 @@ namespace lldb_dap {
void ConfigurationDoneRequestHandler::operator()(
const llvm::json::Object &request) const {
+ dap.configuration_done = true;
+
llvm::json::Object response;
FillResponse(request, response);
dap.SendJSON(llvm::json::Value(std::move(response)));
- dap.configuration_done_sent = true;
- if (dap.stop_at_entry)
- SendThreadStoppedEvent(dap);
- 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/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index ce34c52bcc334..2214c7846217d 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -161,20 +161,22 @@ static void EventThreadFunction(DAP &dap) {
case lldb::eStateSuspended:
break;
case lldb::eStateStopped:
- // We launch and attach in synchronous mode then the first stop
- // event will not be delivered. If we use "launchCommands" during a
- // launch or "attachCommands" during an attach we might some process
- // stop events which we do not want to send an event for. We will
- // manually send a stopped event in request_configurationDone(...)
- // so don't send any before then.
- if (dap.configuration_done_sent) {
- // Only report a stopped event if the process was not
- // automatically restarted.
- if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
- SendStdOutStdErr(dap, process);
+ {
+ std::unique_lock<std::mutex> lock(dap.ignore_stop_mutex);
+ if (dap.ignore_next_stop) {
+ DAP_LOG(dap.log, "Ignoring process stop event");
SendThreadStoppedEvent(dap);
+ dap.ignore_next_stop = false;
+ continue;
}
}
+
+ // Only report a stopped event if the process was not
+ // automatically restarted.
+ if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
+ SendStdOutStdErr(dap, process);
+ SendThreadStoppedEvent(dap);
+ }
break;
case lldb::eStateRunning:
dap.WillContinue();
@@ -284,6 +286,7 @@ llvm::Expected<InitializeResponseBody> InitializeRequestHandler::Run(
// Do not source init files until in/out/err are configured.
dap.debugger = lldb::SBDebugger::Create(false);
dap.debugger.SetInputFile(dap.in);
+ dap.target = dap.debugger.GetDummyTarget();
llvm::Expected<int> out_fd = dap.out.GetWriteFileDescriptor();
if (!out_fd)
@@ -338,4 +341,8 @@ llvm::Expected<InitializeResponseBody> InitializeRequestHandler::Run(
return dap.GetCapabilities();
}
+void InitializeRequestHandler::PostRun() const {
+ dap.SendJSON(CreateEventObject("initialized"));
+}
+
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
index 3e4532e754ec6..ffd61b040e55f 100644
--- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
@@ -72,8 +72,6 @@ void LaunchRequestHandler::PostRun() const {
// Attach happens when launching with runInTerminal.
SendProcessEvent(dap, dap.is_attach ? Attach : Launch);
}
-
- dap.SendJSON(CreateEventObject("initialized"));
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index 7a75cd93abc19..a3e75e35c7dc6 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -238,6 +238,9 @@ llvm::Error BaseRequestHandler::LaunchProcess(
launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
lldb::eLaunchFlagStopAtEntry);
+ std::unique_lock<std::mutex> lock(dap.ignore_stop_mutex);
+ dap.ignore_next_stop = !dap.stop_at_entry;
+
if (arguments.runInTerminal) {
if (llvm::Error err = RunInTerminal(dap, arguments))
return err;
@@ -259,12 +262,27 @@ llvm::Error BaseRequestHandler::LaunchProcess(
// The custom commands might have created a new target so we should use the
// selected target after these commands are run.
dap.target = dap.debugger.GetSelectedTarget();
- // Make sure the process is launched and stopped at the entry point before
- // proceeding as the launch commands are not run using the synchronous
- // mode.
- lldb::SBError error = dap.WaitForProcessToStop(arguments.timeout);
- if (error.Fail())
- return llvm::make_error<DAPError>(error.GetCString());
+ }
+
+ // Notify the event thread that it should ignore the next stop event.
+ lock.unlock();
+ dap.ignore_stop_cv.notify_one();
+
+ // Make sure the process is launched and stopped at the entry point before
+ // proceeding.
+ lldb::SBError error = dap.WaitForProcessToStop(arguments.timeout);
+ if (error.Fail())
+ return llvm::make_error<DAPError>(error.GetCString());
+
+ // FIXME: Unify this between launch & attach.
+ if (!dap.stop_at_entry) {
+ // 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();
}
return llvm::Error::success();
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index fa3d76ed4a125..dec35c5dce1e5 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -282,6 +282,7 @@ class InitializeRequestHandler
static llvm::StringLiteral GetCommand() { return "initialize"; }
llvm::Expected<protocol::InitializeResponseBody>
Run(const protocol::InitializeRequestArguments &args) const override;
+ void PostRun() const override;
};
class LaunchRequestHandler
More information about the lldb-commits
mailing list