[lldb-dap] Adding support for cancelling a request.

ashgti · ashgti · commit 6bb322fd3ed0 · 2025-03-18T12:45:13.000-07:00
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()`.
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -88,13 +88,13 @@ def packet_type_is(packet, packet_type):
 
 
 def dump_dap_log(log_file):
-    print("========= DEBUG ADAPTER PROTOCOL LOGS =========")
+    print("========= DEBUG ADAPTER PROTOCOL LOGS =========", file=sys.stderr)
     if log_file is None:
-        print("no log file available")
+        print("no log file available", file=sys.stderr)
     else:
         with open(log_file, "r") as file:
-            print(file.read())
-    print("========= END =========")
+            print(file.read(), file=sys.stderr)
+    print("========= END =========", file=sys.stderr)
 
 
 def read_packet_thread(vs_comm, log_file):
@@ -107,6 +107,10 @@ def read_packet_thread(vs_comm, log_file):
             # termination of lldb-dap and stop waiting for new packets.
             done = not vs_comm.handle_recv_packet(packet)
     finally:
+        # Wait for the process to fully exit before dumping the log file to
+        # ensure we have the entire log contents.
+        if vs_comm.process is not None:
+            vs_comm.process.wait()
         dump_dap_log(log_file)
 
 
diff --git a/lldb/test/API/tools/lldb-dap/cancel/Makefile 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
@@ -0,0 +1,92 @@
+"""
+Test lldb-dap cancel request
+"""
+
+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; 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)
+        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
@@ -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
@@ -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
@@ -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
@@ -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>
@@ -778,28 +780,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)
+          debugger.RequestInterrupt();
+
+        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;
 
-    if (!HandleObject(**next)) {
+      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;
+          }
+        }
+
+        // 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");
     }
+
+    // Clear interrupt marker prior to handling the next request.
+    if (debugger.InterruptRequested())
+      debugger.CancelInterruptRequest();
   }
 
-  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
@@ -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
@@ -0,0 +1,56 @@
+//===-- SourceRequestHandler.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"
+#include <variant>
+
+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
@@ -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
@@ -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
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+C_SOURCES := main.c`
	`2`	`+`
	`3`	`+include Makefile.rules`