[Lldb-commits] [lldb] [lldb-dap] Adding support for cancelling a request. (PR #130169)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 18 14:09:47 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: John Harrison (ashgti)
<details>
<summary>Changes</summary>
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()`.
---
Patch is 24.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130169.diff
15 Files Affected:
- (added) lldb/test/API/tools/lldb-dap/cancel/Makefile (+3)
- (added) lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py (+101)
- (added) lldb/test/API/tools/lldb-dap/cancel/main.c (+6)
- (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1)
- (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+136-9)
- (modified) lldb/tools/lldb-dap/DAP.h (+3)
- (added) lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp (+55)
- (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+2)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+10)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+7)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+20)
- (modified) lldb/tools/lldb-dap/Transport.cpp (+28-9)
- (modified) lldb/tools/lldb-dap/Transport.h (+2-1)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+4-1)
``````````diff
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 adad75a79fa7a..1e87b28c6d1bd 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -40,6 +40,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 a1e2187288768..ebdaa4949eb77 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>
@@ -241,6 +243,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());
@@ -674,6 +691,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);
@@ -778,28 +799,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 4c57f9fef3d89..17c5ec2bfee9f 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -395,6 +395,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/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index 3262b70042a0e..4373c59798b75 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -463,6 +463,8 @@ void InitializeRequestHandler::operator()(
body.try_emplace("supportsDataBreakpoints", true);
// The debug adapter supports the `readMemory` request.
body.try_emplace("supportsReadMemoryRequest", true);
+ // The debug adapter supports the `cancel` request.
+ body.try_emplace("supportsCancelRequest", true);
// Put in non-DAP specification lldb specific information.
llvm::json::Object lldb_json;
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index c9bcf15933c33..32c3199f4904b 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -381,6 +381,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 4500e7cf909ba..2241ea586c19a 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 = descript...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/130169
More information about the lldb-commits
mailing list