Skip to content

Commit c183231

Browse files
committed
[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()`.
1 parent 5475834 commit c183231

File tree

14 files changed

+377
-20
lines changed

14 files changed

+377
-20
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
C_SOURCES := main.c
2+
3+
include Makefile.rules
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
"""
2+
Test lldb-dap cancel request
3+
"""
4+
5+
import time
6+
7+
from lldbsuite.test.decorators import *
8+
from lldbsuite.test.lldbtest import *
9+
import lldbdap_testcase
10+
11+
12+
class TestDAP_launch(lldbdap_testcase.DAPTestCaseBase):
13+
def send_async_req(self, command: str, arguments={}) -> int:
14+
seq = self.dap_server.sequence
15+
self.dap_server.send_packet(
16+
{
17+
"type": "request",
18+
"command": command,
19+
"arguments": arguments,
20+
}
21+
)
22+
return seq
23+
24+
def async_blocking_request(self, duration: float) -> int:
25+
"""
26+
Sends an evaluate request that will sleep for the specified duration to
27+
block the request handling thread.
28+
"""
29+
return self.send_async_req(
30+
command="evaluate",
31+
arguments={
32+
"expression": '`script import time; print("starting sleep", file=lldb.debugger.GetOutputFileHandle()); time.sleep({})'.format(
33+
duration
34+
),
35+
"context": "repl",
36+
},
37+
)
38+
39+
def async_cancel(self, requestId: int) -> int:
40+
return self.send_async_req(command="cancel", arguments={"requestId": requestId})
41+
42+
def test_pending_request(self):
43+
"""
44+
Tests cancelling a pending request.
45+
"""
46+
program = self.getBuildArtifact("a.out")
47+
self.build_and_launch(program, stopOnEntry=True)
48+
self.continue_to_next_stop()
49+
50+
# Use a relatively short timeout since this is only to ensure the
51+
# following request is queued.
52+
blocking_seq = self.async_blocking_request(duration=1.0)
53+
# Use a longer timeout to ensure we catch if the request was interrupted
54+
# properly.
55+
pending_seq = self.async_blocking_request(duration=self.timeoutval)
56+
cancel_seq = self.async_cancel(requestId=pending_seq)
57+
58+
blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
59+
self.assertEqual(blocking_resp["request_seq"], blocking_seq)
60+
self.assertEqual(blocking_resp["command"], "evaluate")
61+
self.assertEqual(blocking_resp["success"], True)
62+
63+
pending_resp = self.dap_server.recv_packet(filter_type=["response"])
64+
self.assertEqual(pending_resp["request_seq"], pending_seq)
65+
self.assertEqual(pending_resp["command"], "evaluate")
66+
self.assertEqual(pending_resp["success"], False)
67+
self.assertEqual(pending_resp["message"], "cancelled")
68+
69+
cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
70+
self.assertEqual(cancel_resp["request_seq"], cancel_seq)
71+
self.assertEqual(cancel_resp["command"], "cancel")
72+
self.assertEqual(cancel_resp["success"], True)
73+
self.continue_to_exit()
74+
75+
def test_inflight_request(self):
76+
"""
77+
Tests cancelling an inflight request.
78+
"""
79+
program = self.getBuildArtifact("a.out")
80+
self.build_and_launch(program, stopOnEntry=True)
81+
self.continue_to_next_stop()
82+
83+
blocking_seq = self.async_blocking_request(duration=self.timeoutval / 2)
84+
# Wait for the sleep to start to cancel the inflight request.
85+
self.collect_stdout(
86+
timeout_secs=self.timeoutval,
87+
pattern="starting sleep",
88+
)
89+
cancel_seq = self.async_cancel(requestId=blocking_seq)
90+
91+
blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
92+
self.assertEqual(blocking_resp["request_seq"], blocking_seq)
93+
self.assertEqual(blocking_resp["command"], "evaluate")
94+
self.assertEqual(blocking_resp["success"], False)
95+
self.assertEqual(blocking_resp["message"], "cancelled")
96+
97+
cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
98+
self.assertEqual(cancel_resp["request_seq"], cancel_seq)
99+
self.assertEqual(cancel_resp["command"], "cancel")
100+
self.assertEqual(cancel_resp["success"], True)
101+
self.continue_to_exit()
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#include <stdio.h>
2+
3+
int main(int argc, char const *argv[]) {
4+
printf("Hello world!\n");
5+
return 0;
6+
}

lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ def test_default(self):
2727
lines = output.splitlines()
2828
self.assertIn(program, lines[0], "make sure program path is in first argument")
2929

30+
@skipIfWindows
3031
def test_termination(self):
3132
"""
3233
Tests the correct termination of lldb-dap upon a 'disconnect'

lldb/tools/lldb-dap/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ add_lldb_tool(lldb-dap
4141
Handler/ResponseHandler.cpp
4242
Handler/AttachRequestHandler.cpp
4343
Handler/BreakpointLocationsHandler.cpp
44+
Handler/CancelRequestHandler.cpp
4445
Handler/CompileUnitsRequestHandler.cpp
4546
Handler/CompletionsHandler.cpp
4647
Handler/ConfigurationDoneRequestHandler.cpp

lldb/tools/lldb-dap/DAP.cpp

Lines changed: 136 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@
3939
#include <algorithm>
4040
#include <cassert>
4141
#include <chrono>
42+
#include <condition_variable>
4243
#include <cstdarg>
4344
#include <cstdio>
4445
#include <fstream>
46+
#include <future>
4547
#include <memory>
4648
#include <mutex>
4749
#include <string>
@@ -240,6 +242,21 @@ void DAP::SendJSON(const llvm::json::Value &json) {
240242
}
241243

242244
void DAP::Send(const protocol::Message &message) {
245+
if (auto *resp = std::get_if<protocol::Response>(&message);
246+
resp && debugger.InterruptRequested()) {
247+
// If the debugger was interrupted, convert this response into a 'cancelled'
248+
// response.
249+
protocol::Response cancelled;
250+
cancelled.command = resp->command;
251+
cancelled.request_seq = resp->request_seq;
252+
cancelled.success = false;
253+
cancelled.message = protocol::Response::Message::cancelled;
254+
if (llvm::Error err = transport.Write(cancelled))
255+
DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
256+
transport.GetClientName());
257+
return;
258+
}
259+
243260
if (llvm::Error err = transport.Write(message))
244261
DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
245262
transport.GetClientName());
@@ -673,6 +690,10 @@ void DAP::SetTarget(const lldb::SBTarget target) {
673690

674691
bool DAP::HandleObject(const protocol::Message &M) {
675692
if (const auto *req = std::get_if<protocol::Request>(&M)) {
693+
// Clear interrupt marker prior to handling the next request.
694+
if (debugger.InterruptRequested())
695+
debugger.CancelInterruptRequest();
696+
676697
auto handler_pos = request_handlers.find(req->command);
677698
if (handler_pos != request_handlers.end()) {
678699
(*handler_pos->second)(*req);
@@ -777,28 +798,134 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
777798
return ToError(error);
778799
}
779800

801+
template <typename T>
802+
static std::optional<T> getArgumentsIfRequest(const protocol::Message &pm,
803+
llvm::StringLiteral command) {
804+
auto *const req = std::get_if<protocol::Request>(&pm);
805+
if (!req || req->command != command)
806+
return std::nullopt;
807+
808+
T args;
809+
llvm::json::Path::Root root;
810+
if (!fromJSON(req->arguments, args, root)) {
811+
return std::nullopt;
812+
}
813+
814+
return std::move(args);
815+
}
816+
780817
llvm::Error DAP::Loop() {
781-
auto cleanup = llvm::make_scope_exit([this]() {
818+
std::deque<protocol::Message> queue;
819+
std::condition_variable queue_cv;
820+
std::mutex queue_mutex;
821+
std::future<llvm::Error> queue_reader = std::async([&]() -> llvm::Error {
822+
llvm::set_thread_name(transport.GetClientName() + ".transport_handler");
823+
auto cleanup = llvm::make_scope_exit([&]() {
824+
// Ensure we're marked as disconnecting when the reader exits.
825+
disconnecting = true;
826+
queue_cv.notify_all();
827+
});
828+
829+
while (!disconnecting) {
830+
llvm::Expected<std::optional<protocol::Message>> next =
831+
transport.Read(std::chrono::seconds(1));
832+
bool timeout = false;
833+
if (llvm::Error Err = llvm::handleErrors(
834+
next.takeError(),
835+
[&](std::unique_ptr<llvm::StringError> Err) -> llvm::Error {
836+
if (Err->convertToErrorCode() == std::errc::timed_out) {
837+
timeout = true;
838+
return llvm::Error::success();
839+
}
840+
return llvm::Error(std::move(Err));
841+
}))
842+
return Err;
843+
844+
// If the read timed out, continue to check if we should disconnect.
845+
if (timeout)
846+
continue;
847+
848+
// nullopt is returned on EOF.
849+
if (!*next)
850+
break;
851+
852+
{
853+
std::lock_guard<std::mutex> lock(queue_mutex);
854+
855+
// If a cancel is requested for the active request, make a best
856+
// effort attempt to interrupt.
857+
if (const auto cancel_args =
858+
getArgumentsIfRequest<protocol::CancelArguments>(**next,
859+
"cancel");
860+
cancel_args && active_seq == cancel_args->requestId) {
861+
DAP_LOG(log, "({0}) interrupting inflight request {1}",
862+
transport.GetClientName(), active_seq);
863+
debugger.RequestInterrupt();
864+
debugger.GetCommandInterpreter().InterruptCommand();
865+
}
866+
867+
queue.push_back(std::move(**next));
868+
}
869+
queue_cv.notify_one();
870+
}
871+
872+
return llvm::Error::success();
873+
});
874+
875+
auto cleanup = llvm::make_scope_exit([&]() {
782876
out.Stop();
783877
err.Stop();
784878
StopEventHandlers();
785879
});
880+
786881
while (!disconnecting) {
787-
llvm::Expected<std::optional<protocol::Message>> next = transport.Read();
788-
if (!next)
789-
return next.takeError();
882+
protocol::Message next;
883+
{
884+
std::unique_lock<std::mutex> lock(queue_mutex);
885+
queue_cv.wait(lock, [&] { return disconnecting || !queue.empty(); });
790886

791-
// nullopt on EOF
792-
if (!*next)
793-
break;
887+
if (queue.empty())
888+
break;
889+
890+
next = queue.front();
891+
queue.pop_front();
892+
893+
if (protocol::Request *req = std::get_if<protocol::Request>(&next)) {
894+
active_seq = req->seq;
895+
896+
// Check if we should preempt this request from a queued cancel.
897+
bool cancelled = false;
898+
for (const auto &message : queue) {
899+
if (const auto args =
900+
getArgumentsIfRequest<protocol::CancelArguments>(message,
901+
"cancel");
902+
args && args->requestId == req->seq) {
903+
cancelled = true;
904+
break;
905+
}
906+
}
794907

795-
if (!HandleObject(**next)) {
908+
// Preempt the request and immeidately respond with cancelled.
909+
if (cancelled) {
910+
protocol::Response response;
911+
response.request_seq = req->seq;
912+
response.command = req->command;
913+
response.success = false;
914+
response.message = protocol::Response::Message::cancelled;
915+
Send(response);
916+
continue;
917+
}
918+
} else
919+
active_seq = 0;
920+
}
921+
922+
if (!HandleObject(next)) {
796923
return llvm::createStringError(llvm::inconvertibleErrorCode(),
797924
"unhandled packet");
798925
}
799926
}
800927

801-
return llvm::Error::success();
928+
return queue_reader.get();
802929
}
803930

804931
lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {

lldb/tools/lldb-dap/DAP.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,9 @@ struct DAP {
397397
InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id);
398398

399399
InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
400+
401+
private:
402+
std::atomic<int64_t> active_seq;
400403
};
401404

402405
} // namespace lldb_dap
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//===-- CancelRequestHandler.cpp ------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "Handler/RequestHandler.h"
10+
#include "Protocol/ProtocolRequests.h"
11+
#include "llvm/Support/Error.h"
12+
13+
using namespace lldb_dap;
14+
using namespace lldb_dap::protocol;
15+
16+
namespace lldb_dap {
17+
18+
/// The `cancel` request is used by the client in two situations:
19+
///
20+
/// - to indicate that it is no longer interested in the result produced by a
21+
/// specific request issued earlier
22+
/// - to cancel a progress sequence.
23+
///
24+
/// Clients should only call this request if the corresponding capability
25+
/// `supportsCancelRequest` is true.
26+
///
27+
/// This request has a hint characteristic: a debug adapter can only be
28+
/// expected to make a 'best effort' in honoring this request but there are no
29+
/// guarantees.
30+
///
31+
/// The `cancel` request may return an error if it could not cancel
32+
/// an operation but a client should refrain from presenting this error to end
33+
/// users.
34+
///
35+
/// The request that got cancelled still needs to send a response back.
36+
/// This can either be a normal result (`success` attribute true) or an error
37+
/// response (`success` attribute false and the `message` set to `cancelled`).
38+
///
39+
/// Returning partial results from a cancelled request is possible but please
40+
/// note that a client has no generic way for detecting that a response is
41+
/// partial or not.
42+
///
43+
/// The progress that got cancelled still needs to send a `progressEnd` event
44+
/// back.
45+
///
46+
/// A client should not assume that progress just got cancelled after sending
47+
/// the `cancel` request.
48+
llvm::Expected<CancelResponseBody>
49+
CancelRequestHandler::Run(const CancelArguments &arguments) const {
50+
// Cancel support is built into the DAP::Loop handler for detecting
51+
// cancellations of pending or inflight requests.
52+
return CancelResponseBody();
53+
}
54+
55+
} // namespace lldb_dap

0 commit comments

Comments
 (0)