Skip to content

Commit 8b71292

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 fbb8929 commit 8b71292

File tree

15 files changed

+379
-20
lines changed

15 files changed

+379
-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
@@ -40,6 +40,7 @@ add_lldb_tool(lldb-dap
4040
Handler/ResponseHandler.cpp
4141
Handler/AttachRequestHandler.cpp
4242
Handler/BreakpointLocationsHandler.cpp
43+
Handler/CancelRequestHandler.cpp
4344
Handler/CompileUnitsRequestHandler.cpp
4445
Handler/CompletionsHandler.cpp
4546
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>
@@ -241,6 +243,21 @@ void DAP::SendJSON(const llvm::json::Value &json) {
241243
}
242244

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

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

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

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

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

802-
return llvm::Error::success();
929+
return queue_reader.get();
803930
}
804931

805932
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
@@ -395,6 +395,9 @@ struct DAP {
395395
InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id);
396396

397397
InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
398+
399+
private:
400+
std::atomic<int64_t> active_seq;
398401
};
399402

400403
} // 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

lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,8 @@ void InitializeRequestHandler::operator()(
463463
body.try_emplace("supportsDataBreakpoints", true);
464464
// The debug adapter supports the `readMemory` request.
465465
body.try_emplace("supportsReadMemoryRequest", true);
466+
// The debug adapter supports the `cancel` request.
467+
body.try_emplace("supportsCancelRequest", true);
466468

467469
// Put in non-DAP specification lldb specific information.
468470
llvm::json::Object lldb_json;

0 commit comments

Comments
 (0)