Author: John Harrison Date: 2025-04-14T08:29:02-07:00 New Revision: 2d30a60e9ff8b22f7e07ca5360fe1582f96be1ac
URL: https://github.com/llvm/llvm-project/commit/2d30a60e9ff8b22f7e07ca5360fe1582f96be1ac DIFF: https://github.com/llvm/llvm-project/commit/2d30a60e9ff8b22f7e07ca5360fe1582f96be1ac.diff LOG: [lldb-dap] Adding support for cancelling a request. (#130169) 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()`. Added: lldb/test/API/tools/lldb-dap/cancel/Makefile lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py lldb/test/API/tools/lldb-dap/cancel/main.c lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp Modified: lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py lldb/tools/lldb-dap/CMakeLists.txt lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/Handler/RequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.h lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp lldb/tools/lldb-dap/Protocol/ProtocolRequests.h lldb/tools/lldb-dap/Transport.cpp lldb/tools/lldb-dap/Transport.h lldb/tools/lldb-dap/lldb-dap.cpp Removed: ################################################################################ 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..ca4cc0ee2f77a --- /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 / 2) + 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 64c99019a1c9b..1599ee5de7075 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 e9773db7586e6..a9dc19006293b 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -37,6 +37,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 1f49d70ab3ac2..b752e9cfaeb85 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 "Protocol/ProtocolTypes.h" #include "Transport.h" #include "lldb/API/SBBreakpoint.h" @@ -40,13 +41,17 @@ #include <algorithm> #include <cassert> #include <chrono> +#include <condition_variable> #include <cstdarg> #include <cstdio> #include <fstream> +#include <future> #include <memory> #include <mutex> +#include <optional> #include <string> #include <utility> +#include <variant> #if defined(_WIN32) #define NOMINMAX @@ -58,6 +63,7 @@ #endif using namespace lldb_dap; +using namespace lldb_dap::protocol; namespace { #ifdef _WIN32 @@ -230,7 +236,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}", @@ -240,7 +246,27 @@ void DAP::SendJSON(const llvm::json::Value &json) { Send(message); } -void DAP::Send(const protocol::Message &message) { +void DAP::Send(const Message &message) { + // FIXME: After all the requests have migrated from LegacyRequestHandler > + // RequestHandler<> this should be handled in RequestHandler<>::operator(). + if (auto *resp = std::get_if<Response>(&message); + resp && debugger.InterruptRequested()) { + // Clear the interrupt request. + debugger.CancelInterruptRequest(); + + // If the debugger was interrupted, convert this response into a 'cancelled' + // response because we might have a partial result. + Response cancelled{/*request_seq=*/resp->request_seq, + /*command=*/resp->command, + /*success=*/false, + /*message=*/eResponseMessageCancelled, + /*body=*/std::nullopt}; + 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()); @@ -675,11 +701,25 @@ 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> guard(m_active_request_mutex); + m_active_request = req; + + // Clear the interrupt request prior to invoking a handler. + if (debugger.InterruptRequested()) + debugger.CancelInterruptRequest(); + } + + auto cleanup = llvm::make_scope_exit([&]() { + std::scoped_lock<std::mutex> active_request_lock(m_active_request_mutex); + m_active_request = nullptr; + }); + auto handler_pos = request_handlers.find(req->command); if (handler_pos != request_handlers.end()) { - (*handler_pos->second)(*req); + handler_pos->second->Run(*req); return true; // Success } @@ -688,10 +728,10 @@ 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); + std::lock_guard<std::mutex> guard(call_mutex); auto inflight = inflight_reverse_requests.find(resp->request_seq); if (inflight != inflight_reverse_requests.end()) { response_handler = std::move(inflight->second); @@ -782,28 +822,121 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) { return ToError(error); } +bool DAP::IsCancelled(const protocol::Request &req) { + std::lock_guard<std::mutex> guard(m_cancelled_requests_mutex); + return m_cancelled_requests.contains(req.seq); +} + +void DAP::ClearCancelRequest(const CancelArguments &args) { + std::lock_guard<std::mutex> guard(m_cancelled_requests_mutex); + if (args.requestId) + m_cancelled_requests.erase(*args.requestId); +} + +template <typename T> +static std::optional<T> getArgumentsIfRequest(const Message &pm, + llvm::StringLiteral command) { + auto *const req = std::get_if<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::future<llvm::Error> queue_reader = + std::async(std::launch::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; + m_queue_cv.notify_all(); + }); + + while (!disconnecting) { + llvm::Expected<Message> next = + transport.Read(std::chrono::seconds(1)); + if (next.errorIsA<EndOfFileError>()) { + consumeError(next.takeError()); + break; + } + + // If the read timed out, continue to check if we should disconnect. + if (next.errorIsA<TimeoutError>()) { + consumeError(next.takeError()); + continue; + } + + if (llvm::Error err = next.takeError()) + return err; + + if (const protocol::Request *req = + std::get_if<protocol::Request>(&*next); + req && req->command == "disconnect") { + disconnecting = true; + } + + const std::optional<CancelArguments> cancel_args = + getArgumentsIfRequest<CancelArguments>(*next, "cancel"); + if (cancel_args) { + { + std::lock_guard<std::mutex> guard(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. + std::lock_guard<std::mutex> guard(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(); + } + } + + { + std::lock_guard<std::mutex> guard(m_queue_mutex); + m_queue.push_back(std::move(*next)); + } + m_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(); + std::unique_lock<std::mutex> lock(m_queue_mutex); + m_queue_cv.wait(lock, [&] { return disconnecting || !m_queue.empty(); }); - // nullopt on EOF - if (!*next) + if (m_queue.empty()) break; - if (!HandleObject(**next)) { + Message next = m_queue.front(); + m_queue.pop_front(); + + 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) { @@ -1128,8 +1261,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 fc43d988f3a09..8d32a18fb711e 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -35,11 +35,15 @@ #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #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> @@ -397,7 +401,24 @@ struct DAP { InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread); + /// Checks if the request is cancelled. + bool IsCancelled(const protocol::Request &); + + /// Clears the cancel request from the set of tracked cancel requests. + void ClearCancelRequest(const protocol::CancelArguments &); + lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); } + +private: + std::mutex m_queue_mutex; + std::deque<protocol::Message> m_queue; + std::condition_variable m_queue_cv; + + std::mutex m_cancelled_requests_mutex; + llvm::SmallSet<int64_t, 4> 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/Handler/CancelRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp new file mode 100644 index 0000000000000..f09de13c3ff72 --- /dev/null +++ b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp @@ -0,0 +1,56 @@ +//===-- 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 cannot 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. + dap.ClearCancelRequest(arguments); + return CancelResponseBody(); +} + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index 576f0dda64cf4..3520dc2c71a55 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -11,6 +11,7 @@ #include "Handler/ResponseHandler.h" #include "JSONUtils.h" #include "LLDBUtils.h" +#include "Protocol/ProtocolBase.h" #include "RunInTerminal.h" #include "llvm/Support/Error.h" @@ -18,6 +19,8 @@ #include <unistd.h> #endif +using namespace lldb_dap::protocol; + namespace lldb_dap { static std::vector<const char *> @@ -165,6 +168,28 @@ static llvm::Error RunInTerminal(DAP &dap, error.GetCString()); } +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, + /*command=*/request.command, + /*success=*/false, + /*message=*/eResponseMessageCancelled, + /*body=*/std::nullopt}; + dap.Send(cancelled); + return; + } + + // FIXME: After all the requests have migrated from LegacyRequestHandler > + // RequestHandler<> we should be able to move this into + // RequestHandler<>::operator(). + operator()(request); + + // FIXME: After all the requests have migrated from LegacyRequestHandler > + // RequestHandler<> we should be able to check `debugger.InterruptRequest` and + // mark the response as cancelled. +} + lldb::SBError BaseRequestHandler::LaunchProcess(const llvm::json::Object &request) const { lldb::SBError error; diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 396815b04c84a..50795f8252de3 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -47,6 +47,8 @@ class BaseRequestHandler { virtual ~BaseRequestHandler() = default; + void Run(const protocol::Request &); + virtual void operator()(const protocol::Request &request) const = 0; using FeatureSet = llvm::SmallDenseSet<AdapterFeature, 1>; @@ -161,6 +163,15 @@ class RequestHandler : public BaseRequestHandler { response.body = std::move(*body); } + // Mark the request as 'cancelled' if the debugger was interrupted while + // evaluating this handler. + if (dap.debugger.InterruptRequested()) { + dap.debugger.CancelInterruptRequest(); + response.success = false; + response.message = protocol::eResponseMessageCancelled; + response.body = std::nullopt; + } + dap.Send(response); }; @@ -464,6 +475,19 @@ 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"; } + FeatureSet GetSupportedFeatures() const override { + return {protocol::eAdapterFeatureCancelRequest}; + } + 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 9a613128739c2..7163399899f7e 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -17,6 +17,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 64c5116315239..22d400fd494a5 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -30,6 +30,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..96b8a48fdf61e 100644 --- a/lldb/tools/lldb-dap/Transport.cpp +++ b/lldb/tools/lldb-dap/Transport.cpp @@ -10,12 +10,14 @@ #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" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/raw_ostream.h" +#include <optional> #include <string> #include <utility> @@ -27,28 +29,56 @@ 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, + std::optional<std::chrono::microseconds> timeout = std::nullopt) { + if (!descriptor.IsValid()) + return createStringError("transport output is closed"); + + bool timeout_supported = true; + // FIXME: SelectHelper does not work with NativeFile on Win32. +#if _WIN32 + timeout_supported = descriptor.GetFdType() == eFDTypeSocket; +#endif + + 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); - auto status = descriptor.Read(data.data(), length); + Status 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); } -static Expected<std::string> ReadUntil(IOObject &descriptor, - StringRef delimiter) { +static Expected<std::string> +ReadUntil(IOObject &descriptor, StringRef delimiter, + std::optional<std::chrono::microseconds> timeout = std::nullopt) { 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. - if (next->empty()) - return ""; buffer += *next; } return buffer.substr(0, buffer.size() - delimiter.size()); @@ -63,23 +93,23 @@ 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() { +Expected<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. - if (message_header->empty()) - return std::nullopt; if (*message_header != kHeaderContentLength) return createStringError(formatv("expected '{0}' and got '{1}'", kHeaderContentLength, *message_header) @@ -87,9 +117,11 @@ Expected<std::optional<Message>> Transport::Read() { 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)) @@ -98,10 +130,10 @@ Expected<std::optional<Message>> Transport::Read() { 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 78b1a3fb23832..4e347eaa51314 100644 --- a/lldb/tools/lldb-dap/Transport.h +++ b/lldb/tools/lldb-dap/Transport.h @@ -19,10 +19,39 @@ #include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" -#include <optional> +#include <chrono> +#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 llvm::inconvertibleErrorCode(); + } +}; + +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 { @@ -42,8 +71,14 @@ 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(); + /// \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. + llvm::Expected<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 16a90dd20707e..6e17b13cc9e33 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>(); @@ -603,8 +604,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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
