[Lldb-commits] [lldb] [lldb-dap] Adding support for cancelling a request. (PR #130169)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 21 14:03:22 PDT 2025
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/130169
>From c183231db80d6c97bdd5e9bd0b21d041189146e8 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 18 Mar 2025 14:05:38 -0700
Subject: [PATCH 01/10] [lldb-dap] Adding support for cancelling a request.
Adding support for cancelling requests.
There are two forms of request cancellation.
* Preemptively cancelling a request that is in the queue.
* Actively cancelling the in progress request as a best effort attempt using `SBDebugger.RequestInterrupt()`.
---
lldb/test/API/tools/lldb-dap/cancel/Makefile | 3 +
.../tools/lldb-dap/cancel/TestDAP_cancel.py | 101 ++++++++++++
lldb/test/API/tools/lldb-dap/cancel/main.c | 6 +
.../tools/lldb-dap/launch/TestDAP_launch.py | 1 +
lldb/tools/lldb-dap/CMakeLists.txt | 1 +
lldb/tools/lldb-dap/DAP.cpp | 145 ++++++++++++++++--
lldb/tools/lldb-dap/DAP.h | 3 +
.../lldb-dap/Handler/CancelRequestHandler.cpp | 55 +++++++
lldb/tools/lldb-dap/Handler/RequestHandler.h | 10 ++
.../lldb-dap/Protocol/ProtocolRequests.cpp | 7 +
.../lldb-dap/Protocol/ProtocolRequests.h | 20 +++
lldb/tools/lldb-dap/Transport.cpp | 37 +++--
lldb/tools/lldb-dap/Transport.h | 3 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 5 +-
14 files changed, 377 insertions(+), 20 deletions(-)
create mode 100644 lldb/test/API/tools/lldb-dap/cancel/Makefile
create mode 100644 lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
create mode 100644 lldb/test/API/tools/lldb-dap/cancel/main.c
create mode 100644 lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
diff --git a/lldb/test/API/tools/lldb-dap/cancel/Makefile b/lldb/test/API/tools/lldb-dap/cancel/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/cancel/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
new file mode 100644
index 0000000000000..f3b2f9fcb7a92
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
@@ -0,0 +1,101 @@
+"""
+Test lldb-dap cancel request
+"""
+
+import time
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+
+
+class TestDAP_launch(lldbdap_testcase.DAPTestCaseBase):
+ def send_async_req(self, command: str, arguments={}) -> int:
+ seq = self.dap_server.sequence
+ self.dap_server.send_packet(
+ {
+ "type": "request",
+ "command": command,
+ "arguments": arguments,
+ }
+ )
+ return seq
+
+ def async_blocking_request(self, duration: float) -> int:
+ """
+ Sends an evaluate request that will sleep for the specified duration to
+ block the request handling thread.
+ """
+ return self.send_async_req(
+ command="evaluate",
+ arguments={
+ "expression": '`script import time; print("starting sleep", file=lldb.debugger.GetOutputFileHandle()); time.sleep({})'.format(
+ duration
+ ),
+ "context": "repl",
+ },
+ )
+
+ def async_cancel(self, requestId: int) -> int:
+ return self.send_async_req(command="cancel", arguments={"requestId": requestId})
+
+ def test_pending_request(self):
+ """
+ Tests cancelling a pending request.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program, stopOnEntry=True)
+ self.continue_to_next_stop()
+
+ # Use a relatively short timeout since this is only to ensure the
+ # following request is queued.
+ blocking_seq = self.async_blocking_request(duration=1.0)
+ # Use a longer timeout to ensure we catch if the request was interrupted
+ # properly.
+ pending_seq = self.async_blocking_request(duration=self.timeoutval)
+ cancel_seq = self.async_cancel(requestId=pending_seq)
+
+ blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
+ self.assertEqual(blocking_resp["request_seq"], blocking_seq)
+ self.assertEqual(blocking_resp["command"], "evaluate")
+ self.assertEqual(blocking_resp["success"], True)
+
+ pending_resp = self.dap_server.recv_packet(filter_type=["response"])
+ self.assertEqual(pending_resp["request_seq"], pending_seq)
+ self.assertEqual(pending_resp["command"], "evaluate")
+ self.assertEqual(pending_resp["success"], False)
+ self.assertEqual(pending_resp["message"], "cancelled")
+
+ cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
+ self.assertEqual(cancel_resp["request_seq"], cancel_seq)
+ self.assertEqual(cancel_resp["command"], "cancel")
+ self.assertEqual(cancel_resp["success"], True)
+ self.continue_to_exit()
+
+ def test_inflight_request(self):
+ """
+ Tests cancelling an inflight request.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program, stopOnEntry=True)
+ self.continue_to_next_stop()
+
+ blocking_seq = self.async_blocking_request(duration=self.timeoutval / 2)
+ # Wait for the sleep to start to cancel the inflight request.
+ self.collect_stdout(
+ timeout_secs=self.timeoutval,
+ pattern="starting sleep",
+ )
+ cancel_seq = self.async_cancel(requestId=blocking_seq)
+
+ blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
+ self.assertEqual(blocking_resp["request_seq"], blocking_seq)
+ self.assertEqual(blocking_resp["command"], "evaluate")
+ self.assertEqual(blocking_resp["success"], False)
+ self.assertEqual(blocking_resp["message"], "cancelled")
+
+ cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
+ self.assertEqual(cancel_resp["request_seq"], cancel_seq)
+ self.assertEqual(cancel_resp["command"], "cancel")
+ self.assertEqual(cancel_resp["success"], True)
+ self.continue_to_exit()
diff --git a/lldb/test/API/tools/lldb-dap/cancel/main.c b/lldb/test/API/tools/lldb-dap/cancel/main.c
new file mode 100644
index 0000000000000..ecc0d99ec8db7
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/cancel/main.c
@@ -0,0 +1,6 @@
+#include <stdio.h>
+
+int main(int argc, char const *argv[]) {
+ printf("Hello world!\n");
+ return 0;
+}
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index 0c92e5bff07c6..5fb088dc51cbb 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -27,6 +27,7 @@ def test_default(self):
lines = output.splitlines()
self.assertIn(program, lines[0], "make sure program path is in first argument")
+ @skipIfWindows
def test_termination(self):
"""
Tests the correct termination of lldb-dap upon a 'disconnect'
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 545212d8cfb74..da41896bf8cc1 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -41,6 +41,7 @@ add_lldb_tool(lldb-dap
Handler/ResponseHandler.cpp
Handler/AttachRequestHandler.cpp
Handler/BreakpointLocationsHandler.cpp
+ Handler/CancelRequestHandler.cpp
Handler/CompileUnitsRequestHandler.cpp
Handler/CompletionsHandler.cpp
Handler/ConfigurationDoneRequestHandler.cpp
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 65de0488729e5..30e282cc7b95a 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -39,9 +39,11 @@
#include <algorithm>
#include <cassert>
#include <chrono>
+#include <condition_variable>
#include <cstdarg>
#include <cstdio>
#include <fstream>
+#include <future>
#include <memory>
#include <mutex>
#include <string>
@@ -240,6 +242,21 @@ void DAP::SendJSON(const llvm::json::Value &json) {
}
void DAP::Send(const protocol::Message &message) {
+ if (auto *resp = std::get_if<protocol::Response>(&message);
+ resp && debugger.InterruptRequested()) {
+ // If the debugger was interrupted, convert this response into a 'cancelled'
+ // response.
+ protocol::Response cancelled;
+ cancelled.command = resp->command;
+ cancelled.request_seq = resp->request_seq;
+ cancelled.success = false;
+ cancelled.message = protocol::Response::Message::cancelled;
+ if (llvm::Error err = transport.Write(cancelled))
+ DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
+ transport.GetClientName());
+ return;
+ }
+
if (llvm::Error err = transport.Write(message))
DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
transport.GetClientName());
@@ -673,6 +690,10 @@ void DAP::SetTarget(const lldb::SBTarget target) {
bool DAP::HandleObject(const protocol::Message &M) {
if (const auto *req = std::get_if<protocol::Request>(&M)) {
+ // Clear interrupt marker prior to handling the next request.
+ if (debugger.InterruptRequested())
+ debugger.CancelInterruptRequest();
+
auto handler_pos = request_handlers.find(req->command);
if (handler_pos != request_handlers.end()) {
(*handler_pos->second)(*req);
@@ -777,28 +798,134 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
return ToError(error);
}
+template <typename T>
+static std::optional<T> getArgumentsIfRequest(const protocol::Message &pm,
+ llvm::StringLiteral command) {
+ auto *const req = std::get_if<protocol::Request>(&pm);
+ if (!req || req->command != command)
+ return std::nullopt;
+
+ T args;
+ llvm::json::Path::Root root;
+ if (!fromJSON(req->arguments, args, root)) {
+ return std::nullopt;
+ }
+
+ return std::move(args);
+}
+
llvm::Error DAP::Loop() {
- auto cleanup = llvm::make_scope_exit([this]() {
+ std::deque<protocol::Message> queue;
+ std::condition_variable queue_cv;
+ std::mutex queue_mutex;
+ std::future<llvm::Error> queue_reader = std::async([&]() -> llvm::Error {
+ llvm::set_thread_name(transport.GetClientName() + ".transport_handler");
+ auto cleanup = llvm::make_scope_exit([&]() {
+ // Ensure we're marked as disconnecting when the reader exits.
+ disconnecting = true;
+ queue_cv.notify_all();
+ });
+
+ while (!disconnecting) {
+ llvm::Expected<std::optional<protocol::Message>> next =
+ transport.Read(std::chrono::seconds(1));
+ bool timeout = false;
+ if (llvm::Error Err = llvm::handleErrors(
+ next.takeError(),
+ [&](std::unique_ptr<llvm::StringError> Err) -> llvm::Error {
+ if (Err->convertToErrorCode() == std::errc::timed_out) {
+ timeout = true;
+ return llvm::Error::success();
+ }
+ return llvm::Error(std::move(Err));
+ }))
+ return Err;
+
+ // If the read timed out, continue to check if we should disconnect.
+ if (timeout)
+ continue;
+
+ // nullopt is returned on EOF.
+ if (!*next)
+ break;
+
+ {
+ std::lock_guard<std::mutex> lock(queue_mutex);
+
+ // If a cancel is requested for the active request, make a best
+ // effort attempt to interrupt.
+ if (const auto cancel_args =
+ getArgumentsIfRequest<protocol::CancelArguments>(**next,
+ "cancel");
+ cancel_args && active_seq == cancel_args->requestId) {
+ DAP_LOG(log, "({0}) interrupting inflight request {1}",
+ transport.GetClientName(), active_seq);
+ debugger.RequestInterrupt();
+ debugger.GetCommandInterpreter().InterruptCommand();
+ }
+
+ queue.push_back(std::move(**next));
+ }
+ queue_cv.notify_one();
+ }
+
+ return llvm::Error::success();
+ });
+
+ auto cleanup = llvm::make_scope_exit([&]() {
out.Stop();
err.Stop();
StopEventHandlers();
});
+
while (!disconnecting) {
- llvm::Expected<std::optional<protocol::Message>> next = transport.Read();
- if (!next)
- return next.takeError();
+ protocol::Message next;
+ {
+ std::unique_lock<std::mutex> lock(queue_mutex);
+ queue_cv.wait(lock, [&] { return disconnecting || !queue.empty(); });
- // nullopt on EOF
- if (!*next)
- break;
+ if (queue.empty())
+ break;
+
+ next = queue.front();
+ queue.pop_front();
+
+ if (protocol::Request *req = std::get_if<protocol::Request>(&next)) {
+ active_seq = req->seq;
+
+ // Check if we should preempt this request from a queued cancel.
+ bool cancelled = false;
+ for (const auto &message : queue) {
+ if (const auto args =
+ getArgumentsIfRequest<protocol::CancelArguments>(message,
+ "cancel");
+ args && args->requestId == req->seq) {
+ cancelled = true;
+ break;
+ }
+ }
- if (!HandleObject(**next)) {
+ // Preempt the request and immeidately respond with cancelled.
+ if (cancelled) {
+ protocol::Response response;
+ response.request_seq = req->seq;
+ response.command = req->command;
+ response.success = false;
+ response.message = protocol::Response::Message::cancelled;
+ Send(response);
+ continue;
+ }
+ } else
+ active_seq = 0;
+ }
+
+ if (!HandleObject(next)) {
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"unhandled packet");
}
}
- return llvm::Error::success();
+ return queue_reader.get();
}
lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 4b4d471161137..770f82c017fcf 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -397,6 +397,9 @@ struct DAP {
InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id);
InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
+
+private:
+ std::atomic<int64_t> active_seq;
};
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
new file mode 100644
index 0000000000000..e6746316e6e74
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
@@ -0,0 +1,55 @@
+//===-- CancelRequestHandler.cpp ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Handler/RequestHandler.h"
+#include "Protocol/ProtocolRequests.h"
+#include "llvm/Support/Error.h"
+
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
+
+namespace lldb_dap {
+
+/// The `cancel` request is used by the client in two situations:
+///
+/// - to indicate that it is no longer interested in the result produced by a
+/// specific request issued earlier
+/// - to cancel a progress sequence.
+///
+/// Clients should only call this request if the corresponding capability
+/// `supportsCancelRequest` is true.
+///
+/// This request has a hint characteristic: a debug adapter can only be
+/// expected to make a 'best effort' in honoring this request but there are no
+/// guarantees.
+///
+/// The `cancel` request may return an error if it could not cancel
+/// an operation but a client should refrain from presenting this error to end
+/// users.
+///
+/// The request that got cancelled still needs to send a response back.
+/// This can either be a normal result (`success` attribute true) or an error
+/// response (`success` attribute false and the `message` set to `cancelled`).
+///
+/// Returning partial results from a cancelled request is possible but please
+/// note that a client has no generic way for detecting that a response is
+/// partial or not.
+///
+/// The progress that got cancelled still needs to send a `progressEnd` event
+/// back.
+///
+/// A client should not assume that progress just got cancelled after sending
+/// the `cancel` request.
+llvm::Expected<CancelResponseBody>
+CancelRequestHandler::Run(const CancelArguments &arguments) const {
+ // Cancel support is built into the DAP::Loop handler for detecting
+ // cancellations of pending or inflight requests.
+ return CancelResponseBody();
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index d327820224a30..113b1f6d39bf7 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -423,6 +423,16 @@ class ReadMemoryRequestHandler : public LegacyRequestHandler {
void operator()(const llvm::json::Object &request) const override;
};
+class CancelRequestHandler
+ : public RequestHandler<protocol::CancelArguments,
+ protocol::CancelResponseBody> {
+public:
+ using RequestHandler::RequestHandler;
+ static llvm::StringLiteral getCommand() { return "cancel"; }
+ llvm::Expected<protocol::CancelResponseBody>
+ Run(const protocol::CancelArguments &args) const override;
+};
+
/// A request used in testing to get the details on all breakpoints that are
/// currently set in the target. This helps us to test "setBreakpoints" and
/// "setFunctionBreakpoints" requests to verify we have the correct set of
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 5cc5429227439..9b36c8329b0fc 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -15,6 +15,13 @@ using namespace llvm;
namespace lldb_dap::protocol {
+bool fromJSON(const llvm::json::Value &Params, CancelArguments &CA,
+ llvm::json::Path P) {
+ llvm::json::ObjectMapper O(Params, P);
+ return O && O.mapOptional("requestId", CA.requestId) &&
+ O.mapOptional("progressId", CA.progressId);
+}
+
bool fromJSON(const json::Value &Params, DisconnectArguments &DA,
json::Path P) {
json::ObjectMapper O(Params, P);
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 5dc4a589178d2..8acdcd322f526 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -29,6 +29,26 @@
namespace lldb_dap::protocol {
+/// Arguments for `cancel` request.
+struct CancelArguments {
+ /// The ID (attribute `seq`) of the request to cancel. If missing no request
+ /// is cancelled.
+ ///
+ /// Both a `requestId` and a `progressId` can be specified in one request.
+ std::optional<int64_t> requestId;
+
+ /// The ID (attribute `progressId`) of the progress to cancel. If missing no
+ /// progress is cancelled.
+ ///
+ /// Both a `requestId` and a `progressId` can be specified in one request.
+ std::optional<int64_t> progressId;
+};
+bool fromJSON(const llvm::json::Value &, CancelArguments &, llvm::json::Path);
+
+/// Response to `cancel` request. This is just an acknowledgement, so no body
+/// field is required.
+using CancelResponseBody = VoidResponse;
+
/// Arguments for `disconnect` request.
struct DisconnectArguments {
/// A value of true indicates that this `disconnect` request is part of a
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 50f5a4399782a..5a359c190f87d 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -10,6 +10,7 @@
#include "DAPLog.h"
#include "Protocol/ProtocolBase.h"
#include "lldb/Utility/IOObject.h"
+#include "lldb/Utility/SelectHelper.h"
#include "lldb/Utility/Status.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringExtras.h"
@@ -27,23 +28,39 @@ using namespace lldb_dap::protocol;
/// ReadFull attempts to read the specified number of bytes. If EOF is
/// encountered, an empty string is returned.
-static Expected<std::string> ReadFull(IOObject &descriptor, size_t length) {
+static Expected<std::string>
+ReadFull(IOObject &descriptor, size_t length,
+ const std::chrono::microseconds &timeout) {
+ if (!descriptor.IsValid())
+ return createStringError("transport output is closed");
+
+#ifndef _WIN32
+ // FIXME: SelectHelper does not work with NativeFile on Win32.
+ SelectHelper sh;
+ sh.SetTimeout(timeout);
+ sh.FDSetRead(descriptor.GetWaitableHandle());
+ Status status = sh.Select();
+ if (status.Fail())
+ return status.takeError();
+#endif
+
std::string data;
data.resize(length);
- auto status = descriptor.Read(data.data(), length);
+ status = descriptor.Read(data.data(), length);
if (status.Fail())
return status.takeError();
// Return the actual number of bytes read.
return data.substr(0, length);
}
-static Expected<std::string> ReadUntil(IOObject &descriptor,
- StringRef delimiter) {
+static Expected<std::string>
+ReadUntil(IOObject &descriptor, StringRef delimiter,
+ const std::chrono::microseconds &timeout) {
std::string buffer;
buffer.reserve(delimiter.size() + 1);
while (!llvm::StringRef(buffer).ends_with(delimiter)) {
Expected<std::string> next =
- ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1);
+ ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1, timeout);
if (auto Err = next.takeError())
return std::move(Err);
// Return "" if EOF is encountered.
@@ -68,13 +85,14 @@ Transport::Transport(StringRef client_name, Log *log, IOObjectSP input,
: m_client_name(client_name), m_log(log), m_input(std::move(input)),
m_output(std::move(output)) {}
-Expected<std::optional<Message>> Transport::Read() {
+Expected<std::optional<Message>>
+Transport::Read(const std::chrono::microseconds &timeout) {
if (!m_input || !m_input->IsValid())
return createStringError("transport output is closed");
IOObject *input = m_input.get();
Expected<std::string> message_header =
- ReadFull(*input, kHeaderContentLength.size());
+ ReadFull(*input, kHeaderContentLength.size(), timeout);
if (!message_header)
return message_header.takeError();
// '' returned on EOF.
@@ -85,7 +103,8 @@ Expected<std::optional<Message>> Transport::Read() {
kHeaderContentLength, *message_header)
.str());
- Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
+ Expected<std::string> raw_length =
+ ReadUntil(*input, kHeaderSeparator, timeout);
if (!raw_length)
return raw_length.takeError();
if (raw_length->empty())
@@ -96,7 +115,7 @@ Expected<std::optional<Message>> Transport::Read() {
return createStringError(
formatv("invalid content length {0}", *raw_length).str());
- Expected<std::string> raw_json = ReadFull(*input, length);
+ Expected<std::string> raw_json = ReadFull(*input, length, timeout);
if (!raw_json)
return raw_json.takeError();
// If we got less than the expected number of bytes then we hit EOF.
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 78b1a3fb23832..b3849903dd431 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -43,7 +43,8 @@ class Transport {
/// Reads the next Debug Adater Protocol message from the input stream.
///
/// \returns Returns the next protocol message or nullopt if EOF is reached.
- llvm::Expected<std::optional<protocol::Message>> Read();
+ llvm::Expected<std::optional<protocol::Message>>
+ Read(const std::chrono::microseconds &timeout);
/// Returns the name of this transport client, for example `stdin/stdout` or
/// `client_1`.
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 062c3a5f989f3..951c34c8b8c34 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -118,6 +118,7 @@ class LLDBDAPOptTable : public llvm::opt::GenericOptTable {
static void RegisterRequestCallbacks(DAP &dap) {
dap.RegisterRequest<AttachRequestHandler>();
dap.RegisterRequest<BreakpointLocationsRequestHandler>();
+ dap.RegisterRequest<CancelRequestHandler>();
dap.RegisterRequest<CompletionsRequestHandler>();
dap.RegisterRequest<ConfigurationDoneRequestHandler>();
dap.RegisterRequest<ContinueRequestHandler>();
@@ -594,8 +595,10 @@ int main(int argc, char *argv[]) {
redirection_test();
if (auto Err = dap.Loop()) {
+ DAP_LOG(log.get(), "({0}) DAP session error: {1}", client_name,
+ llvm::toStringWithoutConsuming(Err));
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
- "DAP session (" + client_name + ") error: ");
+ "DAP session error: ");
return EXIT_FAILURE;
}
return EXIT_SUCCESS;
>From facdc779869ee0496021801afdf594ed0f1726d7 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 19 Mar 2025 09:54:10 -0700
Subject: [PATCH 02/10] Only apply the Transport::Read timeout to the initial
read of the header, once we have the header we should read the full message.
---
lldb/tools/lldb-dap/Transport.cpp | 12 +++++++-----
lldb/tools/lldb-dap/Transport.h | 5 +++++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 5a359c190f87d..419a17cfdb6ce 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -17,6 +17,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/raw_ostream.h"
+#include <optional>
#include <string>
#include <utility>
@@ -30,14 +31,15 @@ using namespace lldb_dap::protocol;
/// encountered, an empty string is returned.
static Expected<std::string>
ReadFull(IOObject &descriptor, size_t length,
- const std::chrono::microseconds &timeout) {
+ std::optional<std::chrono::microseconds> timeout = std::nullopt) {
if (!descriptor.IsValid())
return createStringError("transport output is closed");
#ifndef _WIN32
// FIXME: SelectHelper does not work with NativeFile on Win32.
SelectHelper sh;
- sh.SetTimeout(timeout);
+ if (timeout)
+ sh.SetTimeout(*timeout);
sh.FDSetRead(descriptor.GetWaitableHandle());
Status status = sh.Select();
if (status.Fail())
@@ -55,7 +57,7 @@ ReadFull(IOObject &descriptor, size_t length,
static Expected<std::string>
ReadUntil(IOObject &descriptor, StringRef delimiter,
- const std::chrono::microseconds &timeout) {
+ std::optional<std::chrono::microseconds> timeout = std::nullopt) {
std::string buffer;
buffer.reserve(delimiter.size() + 1);
while (!llvm::StringRef(buffer).ends_with(delimiter)) {
@@ -104,7 +106,7 @@ Transport::Read(const std::chrono::microseconds &timeout) {
.str());
Expected<std::string> raw_length =
- ReadUntil(*input, kHeaderSeparator, timeout);
+ ReadUntil(*input, kHeaderSeparator);
if (!raw_length)
return raw_length.takeError();
if (raw_length->empty())
@@ -115,7 +117,7 @@ Transport::Read(const std::chrono::microseconds &timeout) {
return createStringError(
formatv("invalid content length {0}", *raw_length).str());
- Expected<std::string> raw_json = ReadFull(*input, length, timeout);
+ Expected<std::string> raw_json = ReadFull(*input, length);
if (!raw_json)
return raw_json.takeError();
// If we got less than the expected number of bytes then we hit EOF.
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index b3849903dd431..bd7ebd2251b96 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -42,6 +42,11 @@ class Transport {
/// Reads the next Debug Adater Protocol message from the input stream.
///
+ /// \param timeout[in]
+ /// A timeout to wait for reading the initial header. Once a message
+ /// header is recieved, this will block until the full message is
+ /// read.
+ ///
/// \returns Returns the next protocol message or nullopt if EOF is reached.
llvm::Expected<std::optional<protocol::Message>>
Read(const std::chrono::microseconds &timeout);
>From 6a474fef1bbd1e11d4119b129d9d397c1c5afee1 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 19 Mar 2025 16:45:10 -0700
Subject: [PATCH 03/10] Fixing missing includes and rebasing on main.
---
lldb/tools/lldb-dap/Handler/RequestHandler.h | 5 ++++-
lldb/tools/lldb-dap/Transport.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 113b1f6d39bf7..be7d12dcd6e10 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -428,7 +428,10 @@ class CancelRequestHandler
protocol::CancelResponseBody> {
public:
using RequestHandler::RequestHandler;
- static llvm::StringLiteral getCommand() { return "cancel"; }
+ static llvm::StringLiteral GetCommand() { return "cancel"; }
+ llvm::StringMap<bool> GetCapabilities() const override {
+ return {{"supportsCancelRequest", true}};
+ }
llvm::Expected<protocol::CancelResponseBody>
Run(const protocol::CancelArguments &args) const override;
};
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index bd7ebd2251b96..a94caefcfa73d 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -19,6 +19,7 @@
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include <chrono>
#include <optional>
namespace lldb_dap {
>From 1c2d19b901390c6080c1367341115150661a6a14 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 19 Mar 2025 17:14:47 -0700
Subject: [PATCH 04/10] Applying clang-format.
---
lldb/tools/lldb-dap/Transport.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 419a17cfdb6ce..3aafef85e8c47 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -105,8 +105,7 @@ Transport::Read(const std::chrono::microseconds &timeout) {
kHeaderContentLength, *message_header)
.str());
- Expected<std::string> raw_length =
- ReadUntil(*input, kHeaderSeparator);
+ Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
if (!raw_length)
return raw_length.takeError();
if (raw_length->empty())
>From d95773e8f62a23f3ad6b32803f753a6cb621406d Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Thu, 20 Mar 2025 16:15:37 -0700
Subject: [PATCH 05/10] Adding fine grained locks for the DAP processing queue
and tried to simplify the Loop reader.
---
lldb/tools/lldb-dap/DAP.cpp | 141 ++++++++++++++++--------------
lldb/tools/lldb-dap/DAP.h | 16 +++-
lldb/tools/lldb-dap/Transport.cpp | 49 +++++++----
lldb/tools/lldb-dap/Transport.h | 34 ++++++-
4 files changed, 150 insertions(+), 90 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 30e282cc7b95a..ce7a0079d131b 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -14,6 +14,7 @@
#include "LLDBUtils.h"
#include "OutputRedirector.h"
#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolRequests.h"
#include "Transport.h"
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBCommandInterpreter.h"
@@ -690,6 +691,29 @@ void DAP::SetTarget(const lldb::SBTarget target) {
bool DAP::HandleObject(const protocol::Message &M) {
if (const auto *req = std::get_if<protocol::Request>(&M)) {
+ {
+ std::lock_guard<std::mutex> lock(m_active_request_mutex);
+ m_active_request = req;
+ }
+
+ auto cleanup = llvm::make_scope_exit([&]() {
+ std::scoped_lock<std::mutex> active_request_lock(m_active_request_mutex);
+ m_active_request = nullptr;
+ });
+
+ {
+ std::lock_guard<std::mutex> lock(m_cancelled_requests_mutex);
+ if (m_cancelled_requests.find(req->seq) != m_cancelled_requests.end()) {
+ protocol::Response response;
+ response.request_seq = req->seq;
+ response.command = req->command;
+ response.success = false;
+ response.message = protocol::Response::Message::cancelled;
+ Send(response);
+ return true;
+ }
+ }
+
// Clear interrupt marker prior to handling the next request.
if (debugger.InterruptRequested())
debugger.CancelInterruptRequest();
@@ -798,6 +822,13 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
return ToError(error);
}
+void DAP::ClearCancelRequest(const protocol::CancelArguments &args) {
+ std::lock_guard<std::mutex> cancalled_requests_lock(
+ m_cancelled_requests_mutex);
+ if (args.requestId)
+ m_cancelled_requests.erase(*args.requestId);
+}
+
template <typename T>
static std::optional<T> getArgumentsIfRequest(const protocol::Message &pm,
llvm::StringLiteral command) {
@@ -815,58 +846,66 @@ static std::optional<T> getArgumentsIfRequest(const protocol::Message &pm,
}
llvm::Error DAP::Loop() {
- std::deque<protocol::Message> queue;
- std::condition_variable queue_cv;
- std::mutex queue_mutex;
std::future<llvm::Error> queue_reader = std::async([&]() -> llvm::Error {
llvm::set_thread_name(transport.GetClientName() + ".transport_handler");
auto cleanup = llvm::make_scope_exit([&]() {
// Ensure we're marked as disconnecting when the reader exits.
disconnecting = true;
- queue_cv.notify_all();
+ m_queue_cv.notify_all();
});
while (!disconnecting) {
- llvm::Expected<std::optional<protocol::Message>> next =
+ llvm::Expected<protocol::Message> next =
transport.Read(std::chrono::seconds(1));
bool timeout = false;
+ bool eof = false;
if (llvm::Error Err = llvm::handleErrors(
next.takeError(),
- [&](std::unique_ptr<llvm::StringError> Err) -> llvm::Error {
- if (Err->convertToErrorCode() == std::errc::timed_out) {
- timeout = true;
- return llvm::Error::success();
- }
- return llvm::Error(std::move(Err));
+ [&](const EndOfFileError &E) -> llvm::Error {
+ eof = true;
+ return llvm::Error::success();
+ },
+ [&](const TimeoutError &) -> llvm::Error {
+ timeout = true;
+ return llvm::Error::success();
}))
return Err;
+ if (eof)
+ break;
+
// If the read timed out, continue to check if we should disconnect.
if (timeout)
continue;
- // nullopt is returned on EOF.
- if (!*next)
- break;
-
- {
- std::lock_guard<std::mutex> lock(queue_mutex);
+ const std::optional<protocol::CancelArguments> cancel_args =
+ getArgumentsIfRequest<protocol::CancelArguments>(*next, "cancel");
+ if (cancel_args) {
+ {
+ std::lock_guard<std::mutex> cancalled_requests_lock(
+ m_cancelled_requests_mutex);
+ if (cancel_args->requestId)
+ m_cancelled_requests.insert(*cancel_args->requestId);
+ }
// If a cancel is requested for the active request, make a best
// effort attempt to interrupt.
- if (const auto cancel_args =
- getArgumentsIfRequest<protocol::CancelArguments>(**next,
- "cancel");
- cancel_args && active_seq == cancel_args->requestId) {
- DAP_LOG(log, "({0}) interrupting inflight request {1}",
- transport.GetClientName(), active_seq);
+ std::lock_guard<std::mutex> active_request_lock(m_active_request_mutex);
+ if (m_active_request &&
+ cancel_args->requestId == m_active_request->seq) {
+ DAP_LOG(log,
+ "({0}) interrupting inflight request (command={1} seq={2})",
+ transport.GetClientName(), m_active_request->command,
+ m_active_request->seq);
debugger.RequestInterrupt();
- debugger.GetCommandInterpreter().InterruptCommand();
}
+ }
- queue.push_back(std::move(**next));
+ {
+ std::lock_guard<std::mutex> queue_lock(m_queue_mutex);
+ m_queue.push_back(std::move(*next));
}
- queue_cv.notify_one();
+ m_queue_cv.notify_one();
}
return llvm::Error::success();
@@ -879,50 +918,18 @@ llvm::Error DAP::Loop() {
});
while (!disconnecting) {
- protocol::Message next;
- {
- std::unique_lock<std::mutex> lock(queue_mutex);
- queue_cv.wait(lock, [&] { return disconnecting || !queue.empty(); });
+ std::unique_lock<std::mutex> lock(m_queue_mutex);
+ m_queue_cv.wait(lock, [&] { return disconnecting || !m_queue.empty(); });
- if (queue.empty())
- break;
+ if (m_queue.empty())
+ break;
- next = queue.front();
- queue.pop_front();
-
- if (protocol::Request *req = std::get_if<protocol::Request>(&next)) {
- active_seq = req->seq;
-
- // Check if we should preempt this request from a queued cancel.
- bool cancelled = false;
- for (const auto &message : queue) {
- if (const auto args =
- getArgumentsIfRequest<protocol::CancelArguments>(message,
- "cancel");
- args && args->requestId == req->seq) {
- cancelled = true;
- break;
- }
- }
+ protocol::Message next = m_queue.front();
+ m_queue.pop_front();
- // Preempt the request and immeidately respond with cancelled.
- if (cancelled) {
- protocol::Response response;
- response.request_seq = req->seq;
- response.command = req->command;
- response.success = false;
- response.message = protocol::Response::Message::cancelled;
- Send(response);
- continue;
- }
- } else
- active_seq = 0;
- }
-
- if (!HandleObject(next)) {
+ if (!HandleObject(next))
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"unhandled packet");
- }
}
return queue_reader.get();
@@ -1250,8 +1257,8 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
}
}
} else {
- // This is not under the globals or locals scope, so there are no duplicated
- // names.
+ // This is not under the globals or locals scope, so there are no
+ // duplicated names.
// We have a named item within an actual variable so we need to find it
// withing the container variable by name.
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 770f82c017fcf..b04d7002f77a7 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -16,6 +16,7 @@
#include "OutputRedirector.h"
#include "ProgressEvent.h"
#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolRequests.h"
#include "SourceBreakpoint.h"
#include "Transport.h"
#include "lldb/API/SBBroadcaster.h"
@@ -38,9 +39,11 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/Threading.h"
+#include <cstdint>
#include <memory>
#include <mutex>
#include <optional>
+#include <set>
#include <thread>
#include <vector>
@@ -398,8 +401,19 @@ struct DAP {
InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
+ /// Clears the cancel request from the set of tracked cancel requests.
+ void ClearCancelRequest(const protocol::CancelArguments &);
+
private:
- std::atomic<int64_t> active_seq;
+ std::mutex m_queue_mutex;
+ std::deque<protocol::Message> m_queue;
+ std::condition_variable m_queue_cv;
+
+ std::mutex m_cancelled_requests_mutex;
+ std::set<int64_t> m_cancelled_requests;
+
+ std::mutex m_active_request_mutex;
+ const protocol::Request *m_active_request;
};
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 3aafef85e8c47..1e646a1e00dab 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -35,22 +35,35 @@ ReadFull(IOObject &descriptor, size_t length,
if (!descriptor.IsValid())
return createStringError("transport output is closed");
-#ifndef _WIN32
- // FIXME: SelectHelper does not work with NativeFile on Win32.
SelectHelper sh;
+ // FIXME: SelectHelper does not work with NativeFile on Win32.
+#if _WIN32
+ if (timeout && descriptor.GetFdType() == eFDTypeSocket)
+ sh.SetTimeout(*timeout);
+#else
if (timeout)
sh.SetTimeout(*timeout);
+#endif
sh.FDSetRead(descriptor.GetWaitableHandle());
Status status = sh.Select();
- if (status.Fail())
+ if (status.Fail()) {
+ // Convert timeouts into a specific error.
+ if (status.GetType() == lldb::eErrorTypePOSIX &&
+ status.GetError() == ETIMEDOUT)
+ return make_error<TimeoutError>();
return status.takeError();
-#endif
+ }
std::string data;
data.resize(length);
status = descriptor.Read(data.data(), length);
if (status.Fail())
return status.takeError();
+
+ // Read returns '' on EOF.
+ if (length == 0)
+ return make_error<EndOfFileError>();
+
// Return the actual number of bytes read.
return data.substr(0, length);
}
@@ -65,9 +78,6 @@ ReadUntil(IOObject &descriptor, StringRef delimiter,
ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1, timeout);
if (auto Err = next.takeError())
return std::move(Err);
- // Return "" if EOF is encountered.
- if (next->empty())
- return "";
buffer += *next;
}
return buffer.substr(0, buffer.size() - delimiter.size());
@@ -82,13 +92,15 @@ static constexpr StringLiteral kHeaderSeparator = "\r\n\r\n";
namespace lldb_dap {
+char EndOfFileError::ID;
+char TimeoutError::ID;
+
Transport::Transport(StringRef client_name, Log *log, IOObjectSP input,
IOObjectSP output)
: m_client_name(client_name), m_log(log), m_input(std::move(input)),
m_output(std::move(output)) {}
-Expected<std::optional<Message>>
-Transport::Read(const std::chrono::microseconds &timeout) {
+Expected<Message> Transport::Read(const std::chrono::microseconds &timeout) {
if (!m_input || !m_input->IsValid())
return createStringError("transport output is closed");
@@ -97,9 +109,6 @@ Transport::Read(const std::chrono::microseconds &timeout) {
ReadFull(*input, kHeaderContentLength.size(), timeout);
if (!message_header)
return message_header.takeError();
- // '' returned on EOF.
- if (message_header->empty())
- return std::nullopt;
if (*message_header != kHeaderContentLength)
return createStringError(formatv("expected '{0}' and got '{1}'",
kHeaderContentLength, *message_header)
@@ -107,9 +116,11 @@ Transport::Read(const std::chrono::microseconds &timeout) {
Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
if (!raw_length)
- return raw_length.takeError();
- if (raw_length->empty())
- return createStringError("unexpected EOF parsing DAP header");
+ return handleErrors(raw_length.takeError(),
+ [&](const EndOfFileError &E) -> llvm::Error {
+ return createStringError(
+ "unexpected EOF while reading header separator");
+ });
size_t length;
if (!to_integer(*raw_length, length))
@@ -118,10 +129,10 @@ Transport::Read(const std::chrono::microseconds &timeout) {
Expected<std::string> raw_json = ReadFull(*input, length);
if (!raw_json)
- return raw_json.takeError();
- // If we got less than the expected number of bytes then we hit EOF.
- if (raw_json->length() != length)
- return createStringError("unexpected EOF parse DAP message body");
+ return handleErrors(
+ raw_json.takeError(), [&](const EndOfFileError &E) -> llvm::Error {
+ return createStringError("unexpected EOF while reading JSON");
+ });
DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, *raw_json);
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index a94caefcfa73d..b9ae0c1a903f9 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -20,10 +20,38 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <chrono>
-#include <optional>
+#include <system_error>
namespace lldb_dap {
+class EndOfFileError : public llvm::ErrorInfo<EndOfFileError> {
+public:
+ static char ID;
+
+ EndOfFileError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "End of file reached.";
+ }
+ std::error_code convertToErrorCode() const override {
+ return std::make_error_code(std::errc::timed_out);
+ }
+};
+
+class TimeoutError : public llvm::ErrorInfo<TimeoutError> {
+public:
+ static char ID;
+
+ TimeoutError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "Operation timed out.";
+ }
+ std::error_code convertToErrorCode() const override {
+ return std::make_error_code(std::errc::timed_out);
+ }
+};
+
/// A transport class that performs the Debug Adapter Protocol communication
/// with the client.
class Transport {
@@ -48,8 +76,8 @@ class Transport {
/// header is recieved, this will block until the full message is
/// read.
///
- /// \returns Returns the next protocol message or nullopt if EOF is reached.
- llvm::Expected<std::optional<protocol::Message>>
+ /// \returns Returns the next protocol message.
+ llvm::Expected<protocol::Message>
Read(const std::chrono::microseconds &timeout);
/// Returns the name of this transport client, for example `stdin/stdout` or
>From 0cdd970307ba9c8389ca7d05b0784c4a281b37f9 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Thu, 20 Mar 2025 16:18:36 -0700
Subject: [PATCH 06/10] Ensure we clear the cancel requests that have been
processed.
---
lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
index e6746316e6e74..ad8299602f09e 100644
--- a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
@@ -49,6 +49,7 @@ llvm::Expected<CancelResponseBody>
CancelRequestHandler::Run(const CancelArguments &arguments) const {
// Cancel support is built into the DAP::Loop handler for detecting
// cancellations of pending or inflight requests.
+ dap.ClearCancelRequest(arguments);
return CancelResponseBody();
}
>From 8141476a30790e74fb63a7c5781e90fae456bccf Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Thu, 20 Mar 2025 16:40:36 -0700
Subject: [PATCH 07/10] Using a few helpers to simplify the cancel responses.
---
.../tools/lldb-dap/cancel/TestDAP_cancel.py | 2 +-
lldb/tools/lldb-dap/DAP.cpp | 82 ++++++++++---------
2 files changed, 43 insertions(+), 41 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
index f3b2f9fcb7a92..ca4cc0ee2f77a 100644
--- a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
+++ b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
@@ -52,7 +52,7 @@ def test_pending_request(self):
blocking_seq = self.async_blocking_request(duration=1.0)
# Use a longer timeout to ensure we catch if the request was interrupted
# properly.
- pending_seq = self.async_blocking_request(duration=self.timeoutval)
+ pending_seq = self.async_blocking_request(duration=self.timeoutval / 2)
cancel_seq = self.async_cancel(requestId=pending_seq)
blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index ce7a0079d131b..d958d74206707 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -60,6 +60,7 @@
#endif
using namespace lldb_dap;
+using namespace lldb_dap::protocol;
namespace {
#ifdef _WIN32
@@ -69,6 +70,15 @@ const char DEV_NULL[] = "/dev/null";
#endif
} // namespace
+static Response CancelledResponse(int64_t seq, std::string command) {
+ Response response;
+ response.request_seq = seq;
+ response.command = command;
+ response.success = false;
+ response.message = Response::Message::cancelled;
+ return response;
+}
+
namespace lldb_dap {
DAP::DAP(llvm::StringRef path, Log *log, const ReplMode default_repl_mode,
@@ -232,7 +242,7 @@ void DAP::StopEventHandlers() {
void DAP::SendJSON(const llvm::json::Value &json) {
// FIXME: Instead of parsing the output message from JSON, pass the `Message`
// as parameter to `SendJSON`.
- protocol::Message message;
+ Message message;
llvm::json::Path::Root root;
if (!fromJSON(json, message, root)) {
DAP_LOG_ERROR(log, root.getError(), "({1}) encoding failed: {0}",
@@ -242,16 +252,12 @@ void DAP::SendJSON(const llvm::json::Value &json) {
Send(message);
}
-void DAP::Send(const protocol::Message &message) {
- if (auto *resp = std::get_if<protocol::Response>(&message);
+void DAP::Send(const Message &message) {
+ if (auto *resp = std::get_if<Response>(&message);
resp && debugger.InterruptRequested()) {
// If the debugger was interrupted, convert this response into a 'cancelled'
// response.
- protocol::Response cancelled;
- cancelled.command = resp->command;
- cancelled.request_seq = resp->request_seq;
- cancelled.success = false;
- cancelled.message = protocol::Response::Message::cancelled;
+ Response cancelled = CancelledResponse(resp->request_seq, resp->command);
if (llvm::Error err = transport.Write(cancelled))
DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
transport.GetClientName());
@@ -689,8 +695,8 @@ void DAP::SetTarget(const lldb::SBTarget target) {
}
}
-bool DAP::HandleObject(const protocol::Message &M) {
- if (const auto *req = std::get_if<protocol::Request>(&M)) {
+bool DAP::HandleObject(const Message &M) {
+ if (const auto *req = std::get_if<Request>(&M)) {
{
std::lock_guard<std::mutex> lock(m_active_request_mutex);
m_active_request = req;
@@ -702,14 +708,12 @@ bool DAP::HandleObject(const protocol::Message &M) {
});
{
+ // If there is a pending cancelled request, preempt the request and mark
+ // it cancelled.
std::lock_guard<std::mutex> lock(m_cancelled_requests_mutex);
if (m_cancelled_requests.find(req->seq) != m_cancelled_requests.end()) {
- protocol::Response response;
- response.request_seq = req->seq;
- response.command = req->command;
- response.success = false;
- response.message = protocol::Response::Message::cancelled;
- Send(response);
+ Response cancelled = CancelledResponse(req->seq, req->command);
+ Send(cancelled);
return true;
}
}
@@ -729,7 +733,7 @@ bool DAP::HandleObject(const protocol::Message &M) {
return false; // Fail
}
- if (const auto *resp = std::get_if<protocol::Response>(&M)) {
+ if (const auto *resp = std::get_if<Response>(&M)) {
std::unique_ptr<ResponseHandler> response_handler;
{
std::lock_guard<std::mutex> locker(call_mutex);
@@ -750,21 +754,20 @@ bool DAP::HandleObject(const protocol::Message &M) {
} else {
llvm::StringRef message = "Unknown error, response failed";
if (resp->message) {
- message =
- std::visit(llvm::makeVisitor(
- [](const std::string &message) -> llvm::StringRef {
- return message;
- },
- [](const protocol::Response::Message &message)
- -> llvm::StringRef {
- switch (message) {
- case protocol::Response::Message::cancelled:
- return "cancelled";
- case protocol::Response::Message::notStopped:
- return "notStopped";
- }
- }),
- *resp->message);
+ message = std::visit(
+ llvm::makeVisitor(
+ [](const std::string &message) -> llvm::StringRef {
+ return message;
+ },
+ [](const Response::Message &message) -> llvm::StringRef {
+ switch (message) {
+ case Response::Message::cancelled:
+ return "cancelled";
+ case Response::Message::notStopped:
+ return "notStopped";
+ }
+ }),
+ *resp->message);
}
(*response_handler)(llvm::createStringError(
@@ -822,7 +825,7 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
return ToError(error);
}
-void DAP::ClearCancelRequest(const protocol::CancelArguments &args) {
+void DAP::ClearCancelRequest(const CancelArguments &args) {
std::lock_guard<std::mutex> cancalled_requests_lock(
m_cancelled_requests_mutex);
if (args.requestId)
@@ -830,9 +833,9 @@ void DAP::ClearCancelRequest(const protocol::CancelArguments &args) {
}
template <typename T>
-static std::optional<T> getArgumentsIfRequest(const protocol::Message &pm,
+static std::optional<T> getArgumentsIfRequest(const Message &pm,
llvm::StringLiteral command) {
- auto *const req = std::get_if<protocol::Request>(&pm);
+ auto *const req = std::get_if<Request>(&pm);
if (!req || req->command != command)
return std::nullopt;
@@ -855,8 +858,7 @@ llvm::Error DAP::Loop() {
});
while (!disconnecting) {
- llvm::Expected<protocol::Message> next =
- transport.Read(std::chrono::seconds(1));
+ llvm::Expected<Message> next = transport.Read(std::chrono::seconds(1));
bool timeout = false;
bool eof = false;
if (llvm::Error Err = llvm::handleErrors(
@@ -878,8 +880,8 @@ llvm::Error DAP::Loop() {
if (timeout)
continue;
- const std::optional<protocol::CancelArguments> cancel_args =
- getArgumentsIfRequest<protocol::CancelArguments>(*next, "cancel");
+ const std::optional<CancelArguments> cancel_args =
+ getArgumentsIfRequest<CancelArguments>(*next, "cancel");
if (cancel_args) {
{
std::lock_guard<std::mutex> cancalled_requests_lock(
@@ -924,7 +926,7 @@ llvm::Error DAP::Loop() {
if (m_queue.empty())
break;
- protocol::Message next = m_queue.front();
+ Message next = m_queue.front();
m_queue.pop_front();
if (!HandleObject(next))
>From c8a3e8421cf77c7b1076759bbf748f420e240379 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Thu, 20 Mar 2025 16:51:09 -0700
Subject: [PATCH 08/10] Trying to isolate the Win32 timeout.
---
lldb/tools/lldb-dap/DAP.cpp | 2 +-
.../lldb-dap/Handler/CancelRequestHandler.cpp | 2 +-
lldb/tools/lldb-dap/Transport.cpp | 31 ++++++++++---------
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index d958d74206707..7a89891b927ef 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -256,7 +256,7 @@ void DAP::Send(const Message &message) {
if (auto *resp = std::get_if<Response>(&message);
resp && debugger.InterruptRequested()) {
// If the debugger was interrupted, convert this response into a 'cancelled'
- // response.
+ // response because we might have a partial result.
Response cancelled = CancelledResponse(resp->request_seq, resp->command);
if (llvm::Error err = transport.Write(cancelled))
DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
diff --git a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
index ad8299602f09e..f09de13c3ff72 100644
--- a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
@@ -43,7 +43,7 @@ namespace lldb_dap {
/// The progress that got cancelled still needs to send a `progressEnd` event
/// back.
///
-/// A client should not assume that progress just got cancelled after sending
+/// A client cannot assume that progress just got cancelled after sending
/// the `cancel` request.
llvm::Expected<CancelResponseBody>
CancelRequestHandler::Run(const CancelArguments &arguments) const {
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 1e646a1e00dab..96b8a48fdf61e 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -35,28 +35,29 @@ ReadFull(IOObject &descriptor, size_t length,
if (!descriptor.IsValid())
return createStringError("transport output is closed");
- SelectHelper sh;
+ bool timeout_supported = true;
// FIXME: SelectHelper does not work with NativeFile on Win32.
#if _WIN32
- if (timeout && descriptor.GetFdType() == eFDTypeSocket)
- sh.SetTimeout(*timeout);
-#else
- if (timeout)
- sh.SetTimeout(*timeout);
+ timeout_supported = descriptor.GetFdType() == eFDTypeSocket;
#endif
- sh.FDSetRead(descriptor.GetWaitableHandle());
- Status status = sh.Select();
- if (status.Fail()) {
- // Convert timeouts into a specific error.
- if (status.GetType() == lldb::eErrorTypePOSIX &&
- status.GetError() == ETIMEDOUT)
- return make_error<TimeoutError>();
- return status.takeError();
+
+ if (timeout && timeout_supported) {
+ SelectHelper sh;
+ sh.SetTimeout(*timeout);
+ sh.FDSetRead(descriptor.GetWaitableHandle());
+ Status status = sh.Select();
+ if (status.Fail()) {
+ // Convert timeouts into a specific error.
+ if (status.GetType() == lldb::eErrorTypePOSIX &&
+ status.GetError() == ETIMEDOUT)
+ return make_error<TimeoutError>();
+ return status.takeError();
+ }
}
std::string data;
data.resize(length);
- status = descriptor.Read(data.data(), length);
+ Status status = descriptor.Read(data.data(), length);
if (status.Fail())
return status.takeError();
>From c6a9796897f09a8187e22969f10f07e6ed3ee7bf Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Thu, 20 Mar 2025 16:59:08 -0700
Subject: [PATCH 09/10] Fixing the EndOfFileError error code.
---
lldb/tools/lldb-dap/Transport.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index b9ae0c1a903f9..d2e7d52a5237c 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -34,7 +34,7 @@ class EndOfFileError : public llvm::ErrorInfo<EndOfFileError> {
OS << "End of file reached.";
}
std::error_code convertToErrorCode() const override {
- return std::make_error_code(std::errc::timed_out);
+ return llvm::inconvertibleErrorCode();
}
};
>From 8a1c97f54fefc656de9733b05f2a6cfbfed8bbad Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 21 Mar 2025 14:02:15 -0700
Subject: [PATCH 10/10] Adjusting includes and when we call
CancelInterruptRequest.
---
lldb/tools/lldb-dap/DAP.cpp | 8 ++++----
lldb/tools/lldb-dap/DAP.h | 2 ++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 7a89891b927ef..d1239adeebea4 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -700,6 +700,10 @@ bool DAP::HandleObject(const Message &M) {
{
std::lock_guard<std::mutex> lock(m_active_request_mutex);
m_active_request = req;
+
+ // Clear interrupt marker prior to handling the next request.
+ if (debugger.InterruptRequested())
+ debugger.CancelInterruptRequest();
}
auto cleanup = llvm::make_scope_exit([&]() {
@@ -718,10 +722,6 @@ bool DAP::HandleObject(const Message &M) {
}
}
- // Clear interrupt marker prior to handling the next request.
- if (debugger.InterruptRequested())
- debugger.CancelInterruptRequest();
-
auto handler_pos = request_handlers.find(req->command);
if (handler_pos != request_handlers.end()) {
(*handler_pos->second)(*req);
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index b04d7002f77a7..b43ad53e6be70 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -39,7 +39,9 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/Threading.h"
+#include <condition_variable>
#include <cstdint>
+#include <deque>
#include <memory>
#include <mutex>
#include <optional>
More information about the lldb-commits
mailing list