Skip to content

Commit 8700ee9

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 b9fb980 commit 8700ee9

File tree

14 files changed

+371
-11
lines changed

14 files changed

+371
-11
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: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
"""
2+
Test lldb-dap cancel request
3+
"""
4+
5+
from lldbsuite.test.decorators import *
6+
from lldbsuite.test.lldbtest import *
7+
import lldbdap_testcase
8+
9+
10+
class TestDAP_launch(lldbdap_testcase.DAPTestCaseBase):
11+
def send_async_req(self, command: str, arguments={}) -> int:
12+
seq = self.dap_server.sequence
13+
self.dap_server.send_packet(
14+
{
15+
"type": "request",
16+
"command": command,
17+
"arguments": arguments,
18+
}
19+
)
20+
return seq
21+
22+
def async_blocking_request(self, duration: float) -> int:
23+
"""
24+
Sends an evaluate request that will sleep for the specified duration to
25+
block the request handling thread.
26+
"""
27+
return self.send_async_req(
28+
command="evaluate",
29+
arguments={
30+
"expression": "`script import time; time.sleep({})".format(duration),
31+
"context": "repl",
32+
},
33+
)
34+
35+
def async_cancel(self, requestId: int) -> int:
36+
return self.send_async_req(command="cancel", arguments={"requestId": requestId})
37+
38+
def test_pending_request(self):
39+
"""
40+
Tests cancelling a pending request.
41+
"""
42+
program = self.getBuildArtifact("a.out")
43+
self.build_and_launch(program, stopOnEntry=True)
44+
self.continue_to_next_stop()
45+
46+
# Use a relatively short timeout since this is only to ensure the
47+
# following request is queued.
48+
blocking_seq = self.async_blocking_request(duration=1.0)
49+
# Use a longer timeout to ensure we catch if the request was interrupted
50+
# properly.
51+
pending_seq = self.async_blocking_request(duration=self.timeoutval)
52+
cancel_seq = self.async_cancel(requestId=pending_seq)
53+
54+
blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
55+
self.assertEqual(blocking_resp["request_seq"], blocking_seq)
56+
self.assertEqual(blocking_resp["command"], "evaluate")
57+
self.assertEqual(blocking_resp["success"], True)
58+
59+
pending_resp = self.dap_server.recv_packet(filter_type=["response"])
60+
self.assertEqual(pending_resp["request_seq"], pending_seq)
61+
self.assertEqual(pending_resp["command"], "evaluate")
62+
self.assertEqual(pending_resp["success"], False)
63+
self.assertEqual(pending_resp["message"], "cancelled")
64+
65+
cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
66+
self.assertEqual(cancel_resp["request_seq"], cancel_seq)
67+
self.assertEqual(cancel_resp["command"], "cancel")
68+
self.assertEqual(cancel_resp["success"], True)
69+
self.continue_to_exit()
70+
71+
def test_inflight_request(self):
72+
"""
73+
Tests cancelling an inflight request.
74+
"""
75+
program = self.getBuildArtifact("a.out")
76+
self.build_and_launch(program, stopOnEntry=True)
77+
self.continue_to_next_stop()
78+
79+
blocking_seq = self.async_blocking_request(duration=self.timeoutval)
80+
cancel_seq = self.async_cancel(requestId=blocking_seq)
81+
82+
blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
83+
self.assertEqual(blocking_resp["request_seq"], blocking_seq)
84+
self.assertEqual(blocking_resp["command"], "evaluate")
85+
self.assertEqual(blocking_resp["success"], False)
86+
self.assertEqual(blocking_resp["message"], "cancelled")
87+
88+
cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
89+
self.assertEqual(cancel_resp["request_seq"], cancel_seq)
90+
self.assertEqual(cancel_resp["command"], "cancel")
91+
self.assertEqual(cancel_resp["success"], True)
92+
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/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: 118 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include <algorithm>
3838
#include <cassert>
3939
#include <chrono>
40+
#include <condition_variable>
4041
#include <cstdarg>
4142
#include <cstdio>
4243
#include <fstream>
@@ -256,7 +257,27 @@ void DAP::SendJSON(const llvm::json::Value &json) {
256257
}
257258
return;
258259
}
259-
auto status = transport.Write(log, M);
260+
Send(M);
261+
}
262+
263+
void DAP::Send(const protocol::Message &M) {
264+
lldb_private::Status status;
265+
// If the debugger was interrupted while handling a response, mark the
266+
// response as cancelled since it may contain partial information.
267+
if (const auto *resp = std::get_if<protocol::Response>(&M);
268+
debugger.InterruptRequested()) {
269+
status = transport.Write(
270+
log, protocol::Response{
271+
/*request_seq=*/resp->request_seq,
272+
/*command=*/resp->command,
273+
/*success=*/false,
274+
/*message=*/protocol::Response::Message::cancelled,
275+
/*rawBody=*/std::nullopt,
276+
});
277+
} else {
278+
status = transport.Write(log, M);
279+
}
280+
260281
if (status.Fail() && log)
261282
*log << llvm::formatv("failed to send {0}: {1}\n", llvm::json::Value(M),
262283
status.AsCString())
@@ -798,22 +819,112 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
798819
return error;
799820
}
800821

822+
template <typename T>
823+
static std::optional<T> getArgumentsIfRequest(const protocol::Message &pm,
824+
llvm::StringLiteral command) {
825+
auto *const req = std::get_if<protocol::Request>(&pm);
826+
if (!req || req->command != command)
827+
return std::nullopt;
828+
829+
T args;
830+
llvm::json::Path::Root root;
831+
if (!fromJSON(req->rawArguments, args, root)) {
832+
return std::nullopt;
833+
}
834+
835+
return std::move(args);
836+
}
837+
801838
llvm::Error DAP::Loop() {
802-
auto cleanup = llvm::make_scope_exit([this]() { StopEventHandlers(); });
839+
std::deque<protocol::Message> queue;
840+
std::condition_variable queue_cv;
841+
std::mutex queue_mutex;
842+
std::thread queue_reader([&]() {
843+
auto cleanup = llvm::make_scope_exit([&]() {
844+
// Ensure we're marked as disconnecting when the reader exits.
845+
disconnecting = true;
846+
queue_cv.notify_all();
847+
});
848+
while (!disconnecting) {
849+
protocol::Message next;
850+
auto status = transport.Read(log, next);
851+
if (status.Fail()) {
852+
if (status.GetError() != Transport::kEOF && log)
853+
*log << "DAP transport failure: " << status.AsCString() << std::endl;
854+
break;
855+
}
856+
857+
{
858+
std::lock_guard<std::mutex> lock(queue_mutex);
859+
860+
// If a cancel is requested for the active request, make a best
861+
// effort attempt to interrupt.
862+
if (const auto args = getArgumentsIfRequest<protocol::CancelArguments>(
863+
next, "cancel");
864+
args && active_seq == args->requestId)
865+
debugger.RequestInterrupt();
866+
867+
queue.push_back(std::move(next));
868+
}
869+
queue_cv.notify_one();
870+
}
871+
});
872+
873+
auto cleanup = llvm::make_scope_exit([&]() {
874+
StopEventHandlers();
875+
queue_reader.join();
876+
});
877+
803878
while (!disconnecting) {
804879
protocol::Message next;
805-
auto status = transport.Read(log, next);
806-
if (status.Fail()) {
807-
// On EOF, simply break out of the loop.
808-
if (status.GetError() == Transport::kEOF)
880+
{
881+
std::unique_lock<std::mutex> lock(queue_mutex);
882+
queue_cv.wait(lock, [&] { return disconnecting || !queue.empty(); });
883+
884+
if (queue.empty())
809885
break;
810-
return status.takeError();
886+
887+
next = queue.front();
888+
queue.pop_front();
889+
890+
if (protocol::Request *req = std::get_if<protocol::Request>(&next)) {
891+
active_seq = req->seq;
892+
893+
// Check if we should preempt this request from a queued cancel.
894+
bool cancelled = false;
895+
for (const auto &message : queue) {
896+
if (const auto args =
897+
getArgumentsIfRequest<protocol::CancelArguments>(message,
898+
"cancel");
899+
args && args->requestId == req->seq) {
900+
cancelled = true;
901+
break;
902+
}
903+
}
904+
905+
// Preempt the request and immeidately respond with cancelled.
906+
if (cancelled) {
907+
Send(protocol::Response{
908+
/*request_seq=*/req->seq,
909+
/*command=*/req->command,
910+
/*success=*/false,
911+
/*message=*/protocol::Response::Message::cancelled,
912+
/*rawBody=*/std::nullopt,
913+
});
914+
continue;
915+
}
916+
} else
917+
active_seq = 0;
811918
}
812919

813920
if (!HandleObject(next)) {
814921
return llvm::createStringError(llvm::inconvertibleErrorCode(),
815922
"unhandled packet");
816923
}
924+
925+
// Clear interrupt marker prior to handling the next request.
926+
if (debugger.InterruptRequested())
927+
debugger.CancelInterruptRequest();
817928
}
818929

819930
return llvm::Error::success();

lldb/tools/lldb-dap/DAP.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,11 @@ struct DAP {
391391
InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id);
392392

393393
InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
394+
395+
private:
396+
void Send(const protocol::Message &M);
397+
398+
std::atomic<int64_t> active_seq;
394399
};
395400

396401
template <typename Body>
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//===-- SourceRequestHandler.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 "Protocol.h"
10+
#include "RequestHandler.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+
// "CancelRequest": {
19+
// "allOf": [ { "$ref": "#/definitions/Request" }, {
20+
// "type": "object",
21+
// "description": "The `cancel` request is used by the client in two
22+
// situations:\n- to indicate that it is no longer interested in the result
23+
// produced by a specific request issued earlier\n- to cancel a progress
24+
// sequence.\nClients should only call this request if the corresponding
25+
// capability `supportsCancelRequest` is true.\nThis request has a hint
26+
// characteristic: a debug adapter can only be expected to make a 'best
27+
// effort' in honoring this request but there are no guarantees.\nThe
28+
// `cancel` request may return an error if it could not cancel an operation
29+
// but a client should refrain from presenting this error to end users.\nThe
30+
// request that got cancelled still needs to send a response back. This can
31+
// either be a normal result (`success` attribute true) or an error response
32+
// (`success` attribute false and the `message` set to
33+
// `cancelled`).\nReturning partial results from a cancelled request is
34+
// possible but please note that a client has no generic way for detecting
35+
// that a response is partial or not.\nThe progress that got cancelled still
36+
// needs to send a `progressEnd` event back.\n A client should not assume
37+
// that progress just got cancelled after sending the `cancel` request.",
38+
// "properties": {
39+
// "command": {
40+
// "type": "string",
41+
// "enum": [ "cancel" ]
42+
// },
43+
// "arguments": {
44+
// "$ref": "#/definitions/CancelArguments"
45+
// }
46+
// },
47+
// "required": [ "command" ]
48+
// }]
49+
// },
50+
llvm::Expected<CancelResponseBody>
51+
CancelRequestHandler::Run(const CancelArguments &arguments) const {
52+
/* no-op, simple ack of the request. */
53+
return nullptr;
54+
}
55+
56+
} // namespace lldb_dap

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ void InitializeRequestHandler::operator()(
422422
// The debug adapter support for instruction breakpoint.
423423
body.try_emplace("supportsInstructionBreakpoints", true);
424424

425+
425426
llvm::json::Array completion_characters;
426427
completion_characters.emplace_back(".");
427428
completion_characters.emplace_back(" ");
@@ -464,6 +465,8 @@ void InitializeRequestHandler::operator()(
464465
body.try_emplace("supportsDataBreakpoints", true);
465466
// The debug adapter supports the `readMemory` request.
466467
body.try_emplace("supportsReadMemoryRequest", true);
468+
// The debug adapter supports the `cancel` request.
469+
body.try_emplace("supportsCancelRequest", true);
467470

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

lldb/tools/lldb-dap/Handler/RequestHandler.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,16 @@ class ReadMemoryRequestHandler : public BaseRequestHandler {
354354
void operator()(const llvm::json::Object &request) const override;
355355
};
356356

357+
class CancelRequestHandler
358+
: public RequestHandler<protocol::CancelArguments,
359+
protocol::CancelResponseBody> {
360+
public:
361+
using RequestHandler::RequestHandler;
362+
static llvm::StringLiteral getCommand() { return "cancel"; }
363+
llvm::Expected<protocol::CancelResponseBody>
364+
Run(const protocol::CancelArguments &args) const override;
365+
};
366+
357367
/// A request used in testing to get the details on all breakpoints that are
358368
/// currently set in the target. This helps us to test "setBreakpoints" and
359369
/// "setFunctionBreakpoints" requests to verify we have the correct set of

lldb/tools/lldb-dap/Protocol.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,13 @@ llvm::json::Value toJSON(const ExitedEventBody &EEB) {
319319
return llvm::json::Object{{"exitCode", EEB.exitCode}};
320320
}
321321

322+
bool fromJSON(const llvm::json::Value &Params, CancelArguments &CA,
323+
llvm::json::Path P) {
324+
llvm::json::ObjectMapper O(Params, P);
325+
return O && O.mapOptional("requestId", CA.requestId) &&
326+
O.mapOptional("progressId", CA.progressId);
327+
}
328+
322329
bool fromJSON(const llvm::json::Value &Params, SourceArguments &SA,
323330
llvm::json::Path P) {
324331
llvm::json::ObjectMapper O(Params, P);

0 commit comments

Comments
 (0)