[Lldb-commits] [lldb] [lldb-dap] Correct lldb-dap `seq` handling. (PR #164306)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 20 12:09:21 PDT 2025
https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/164306
We've been treating the `seq` attribute incorrectly in lldb-dap. Previously, we always had `seq=0` for events and responses. We only filled in the `seq` field on reverse requests.
However, looking at the spec and other DAP implementations, we are supposed to fill in the `seq` field for each request we send to the DAP client.
I've updated our usage to fill in `seq` in `DAP::Send` so that all events/requests/responses have a properly filled `seq` value.
>From a29c5c93ac2d1adedd8cce923693c7f5e78b151e Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 20 Oct 2025 10:58:03 -0700
Subject: [PATCH] [lldb-dap] Correct lldb-dap `seq` handling.
We've been treating the `seq` attribute incorrectly in lldb-dap. Previously, we always had `seq=0` for events and responses. We only filled in the `seq` field on reverse requests.
However, looking at the spec and other DAP implementations, we are supposed to fill in the `seq` field for each request we send to the DAP client.
I've updated our usage to fill in `seq` in `DAP::Send` so that all events/requests/responses have a properly filled `seq` value.
---
lldb/tools/lldb-dap/DAP.cpp | 62 +++++++++++--------
lldb/tools/lldb-dap/DAP.h | 25 +++-----
lldb/tools/lldb-dap/EventHelper.cpp | 6 +-
.../tools/lldb-dap/Handler/RequestHandler.cpp | 3 +-
lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp | 29 +++------
lldb/tools/lldb-dap/Protocol/ProtocolBase.h | 20 +++++-
lldb/unittests/DAP/DAPTest.cpp | 2 +-
7 files changed, 80 insertions(+), 67 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index f76656e98ca01..7a960aea115c0 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -266,40 +266,47 @@ void DAP::SendJSON(const llvm::json::Value &json) {
Send(message);
}
-void DAP::Send(const Message &message) {
- if (const protocol::Event *event = std::get_if<protocol::Event>(&message)) {
- if (llvm::Error err = transport.Send(*event))
+Id DAP::Send(const Message &message) {
+ std::lock_guard<std::mutex> guard(call_mutex);
+ if (const protocol::Event *e = std::get_if<protocol::Event>(&message)) {
+ protocol::Event event = *e;
+ event.seq = seq++;
+ if (llvm::Error err = transport.Send(event))
DAP_LOG_ERROR(log, std::move(err), "({0}) sending event failed",
m_client_name);
- return;
+ return event.seq;
}
- if (const Request *req = std::get_if<Request>(&message)) {
- if (llvm::Error err = transport.Send(*req))
+ if (const Request *r = std::get_if<Request>(&message)) {
+ Request req = *r;
+ req.seq = seq++;
+ if (llvm::Error err = transport.Send(req))
DAP_LOG_ERROR(log, std::move(err), "({0}) sending request failed",
m_client_name);
- return;
+ return req.seq;
}
- if (const Response *resp = std::get_if<Response>(&message)) {
+ if (const Response *r = std::get_if<Response>(&message)) {
+ Response resp = *r;
+ resp.seq = seq++;
// FIXME: After all the requests have migrated from LegacyRequestHandler >
// RequestHandler<> this should be handled in RequestHandler<>::operator().
// If the debugger was interrupted, convert this response into a
// 'cancelled' response because we might have a partial result.
- llvm::Error err =
- (debugger.InterruptRequested())
- ? transport.Send({/*request_seq=*/resp->request_seq,
- /*command=*/resp->command,
- /*success=*/false,
- /*message=*/eResponseMessageCancelled,
- /*body=*/std::nullopt})
- : transport.Send(*resp);
- if (err) {
+ llvm::Error err = (debugger.InterruptRequested())
+ ? transport.Send({
+ /*seq=*/resp.seq,
+ /*request_seq=*/resp.request_seq,
+ /*command=*/resp.command,
+ /*success=*/false,
+ /*message=*/eResponseMessageCancelled,
+ /*body=*/std::nullopt,
+ })
+ : transport.Send(resp);
+ if (err)
DAP_LOG_ERROR(log, std::move(err), "({0}) sending response failed",
m_client_name);
- return;
- }
- return;
+ return resp.seq;
}
llvm_unreachable("Unexpected message type");
@@ -1453,17 +1460,20 @@ void DAP::EventThread() {
if (remove_module && module_exists) {
modules.erase(module_id);
Send(protocol::Event{
- "module", ModuleEventBody{std::move(p_module).value(),
- ModuleEventBody::eReasonRemoved}});
+ 0, "module",
+ ModuleEventBody{std::move(p_module).value(),
+ ModuleEventBody::eReasonRemoved}});
} else if (module_exists) {
Send(protocol::Event{
- "module", ModuleEventBody{std::move(p_module).value(),
- ModuleEventBody::eReasonChanged}});
+ 0, "module",
+ ModuleEventBody{std::move(p_module).value(),
+ ModuleEventBody::eReasonChanged}});
} else if (!remove_module) {
modules.insert(module_id);
Send(protocol::Event{
- "module", ModuleEventBody{std::move(p_module).value(),
- ModuleEventBody::eReasonNew}});
+ 0, "module",
+ ModuleEventBody{std::move(p_module).value(),
+ ModuleEventBody::eReasonNew}});
}
}
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index a90ddf59671ee..4acb61dd11f59 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -136,7 +136,7 @@ struct DAP final : public DAPTransport::MessageHandler {
/// unless we send a "thread" event to indicate the thread exited.
llvm::DenseSet<lldb::tid_t> thread_ids;
- uint32_t reverse_request_seq = 0;
+ uint32_t seq = 0;
std::mutex call_mutex;
llvm::SmallDenseMap<int64_t, std::unique_ptr<ResponseHandler>>
inflight_reverse_requests;
@@ -220,8 +220,8 @@ struct DAP final : public DAPTransport::MessageHandler {
/// Serialize the JSON value into a string and send the JSON packet to the
/// "out" stream.
void SendJSON(const llvm::json::Value &json);
- /// Send the given message to the client
- void Send(const protocol::Message &message);
+ /// Send the given message to the client.
+ protocol::Id Send(const protocol::Message &message);
void SendOutput(OutputType o, const llvm::StringRef output);
@@ -353,19 +353,14 @@ struct DAP final : public DAPTransport::MessageHandler {
template <typename Handler>
void SendReverseRequest(llvm::StringRef command,
llvm::json::Value arguments) {
- int64_t id;
- {
- std::lock_guard<std::mutex> locker(call_mutex);
- id = ++reverse_request_seq;
- inflight_reverse_requests[id] = std::make_unique<Handler>(command, id);
- }
-
- SendJSON(llvm::json::Object{
- {"type", "request"},
- {"seq", id},
- {"command", command},
- {"arguments", std::move(arguments)},
+ protocol::Id id = Send(protocol::Request{
+ 0,
+ command.str(),
+ std::move(arguments),
});
+
+ std::lock_guard<std::mutex> locker(call_mutex);
+ inflight_reverse_requests[id] = std::make_unique<Handler>(command, id);
}
/// The set of capabilities supported by this adapter.
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 2b9ed229405a8..0c6a38560e3ae 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -70,7 +70,7 @@ void SendExtraCapabilities(DAP &dap) {
// Only notify the client if supportedFeatures changed.
if (!body.capabilities.supportedFeatures.empty())
- dap.Send(protocol::Event{"capabilities", std::move(body)});
+ dap.Send(protocol::Event{0, "capabilities", std::move(body)});
}
// "ProcessEvent": {
@@ -281,7 +281,7 @@ void SendInvalidatedEvent(
return;
protocol::InvalidatedEventBody body;
body.areas = areas;
- dap.Send(protocol::Event{"invalidated", std::move(body)});
+ dap.Send(protocol::Event{0, "invalidated", std::move(body)});
}
void SendMemoryEvent(DAP &dap, lldb::SBValue variable) {
@@ -292,7 +292,7 @@ void SendMemoryEvent(DAP &dap, lldb::SBValue variable) {
body.count = variable.GetByteSize();
if (body.memoryReference == LLDB_INVALID_ADDRESS)
return;
- dap.Send(protocol::Event{"memory", std::move(body)});
+ dap.Send(protocol::Event{0, "memory", std::move(body)});
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index de63c9d78efd1..0ebd607d8163c 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -157,7 +157,8 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
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,
+ Response cancelled{/*seq=*/0,
+ /*request_seq=*/request.seq,
/*command=*/request.command,
/*success=*/false,
/*message=*/eResponseMessageCancelled,
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
index 9cd9028d879e9..3b39617b981c0 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
@@ -104,7 +104,7 @@ bool operator==(const Request &a, const Request &b) {
json::Value toJSON(const Response &R) {
json::Object Result{{"type", "response"},
- {"seq", 0},
+ {"seq", R.seq},
{"command", R.command},
{"request_seq", R.request_seq},
{"success", R.success}};
@@ -132,8 +132,9 @@ json::Value toJSON(const Response &R) {
return std::move(Result);
}
-bool fromJSON(json::Value const &Params,
- std::variant<ResponseMessage, std::string> &M, json::Path P) {
+static bool fromJSON(json::Value const &Params,
+ std::variant<ResponseMessage, std::string> &M,
+ json::Path P) {
auto rawMessage = Params.getAsString();
if (!rawMessage) {
P.report("expected a string");
@@ -157,8 +158,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) {
return false;
MessageType type;
- int64_t seq;
- if (!O.map("type", type) || !O.map("seq", seq) ||
+ if (!O.map("type", type) || !O.map("seq", R.seq) ||
!O.map("command", R.command) || !O.map("request_seq", R.request_seq))
return false;
@@ -168,12 +168,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) {
}
if (R.command.empty()) {
- P.field("command").report("expected to not be ''");
- return false;
- }
-
- if (R.request_seq == 0) {
- P.field("request_seq").report("expected to not be '0'");
+ P.field("command").report("expected to not be empty");
return false;
}
@@ -219,7 +214,7 @@ bool fromJSON(json::Value const &Params, ErrorMessage &EM, json::Path P) {
json::Value toJSON(const Event &E) {
json::Object Result{
{"type", "event"},
- {"seq", 0},
+ {"seq", E.seq},
{"event", E.event},
};
@@ -235,8 +230,7 @@ bool fromJSON(json::Value const &Params, Event &E, json::Path P) {
return false;
MessageType type;
- int64_t seq;
- if (!O.map("type", type) || !O.map("seq", seq) || !O.map("event", E.event))
+ if (!O.map("type", type) || !O.map("seq", E.seq) || !O.map("event", E.event))
return false;
if (type != eMessageTypeEvent) {
@@ -244,13 +238,8 @@ bool fromJSON(json::Value const &Params, Event &E, json::Path P) {
return false;
}
- if (seq != 0) {
- P.field("seq").report("expected to be '0'");
- return false;
- }
-
if (E.event.empty()) {
- P.field("event").report("expected to not be ''");
+ P.field("event").report("expected to not be empty");
return false;
}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
index 92e41b1dbf595..cf76d506ee054 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
@@ -41,7 +41,7 @@ struct Request {
/// associate requests with their corresponding responses. For protocol
/// messages of type `request` the sequence number can be used to cancel the
/// request.
- Id seq;
+ Id seq = 0;
/// The command to execute.
std::string command;
@@ -58,6 +58,15 @@ bool operator==(const Request &, const Request &);
/// A debug adapter initiated event.
struct Event {
+ /// Sequence number of the message (also known as message ID). The `seq` for
+ /// the first message sent by a client or debug adapter is 1, and for each
+ /// subsequent message is 1 greater than the previous message sent by that
+ /// actor. `seq` can be used to order requests, responses, and events, and to
+ /// associate requests with their corresponding responses. For protocol
+ /// messages of type `request` the sequence number can be used to cancel the
+ /// request.
+ Id seq = 0;
+
/// Type of event.
std::string event;
@@ -77,6 +86,15 @@ enum ResponseMessage : unsigned {
/// Response for a request.
struct Response {
+ /// Sequence number of the message (also known as message ID). The `seq` for
+ /// the first message sent by a client or debug adapter is 1, and for each
+ /// subsequent message is 1 greater than the previous message sent by that
+ /// actor. `seq` can be used to order requests, responses, and events, and to
+ /// associate requests with their corresponding responses. For protocol
+ /// messages of type `request` the sequence number can be used to cancel the
+ /// request.
+ Id seq = 0;
+
/// Sequence number of the corresponding request.
Id request_seq;
diff --git a/lldb/unittests/DAP/DAPTest.cpp b/lldb/unittests/DAP/DAPTest.cpp
index 4fd6cd546e6fa..dd610db299134 100644
--- a/lldb/unittests/DAP/DAPTest.cpp
+++ b/lldb/unittests/DAP/DAPTest.cpp
@@ -21,7 +21,7 @@ using namespace testing;
class DAPTest : public TransportBase {};
TEST_F(DAPTest, SendProtocolMessages) {
- dap->Send(Event{/*event=*/"my-event", /*body=*/std::nullopt});
+ dap->Send(Event{0, /*event=*/"my-event", /*body=*/std::nullopt});
EXPECT_CALL(client, Received(IsEvent("my-event")));
Run();
}
More information about the lldb-commits
mailing list