Skip to content

[lldb][lldb-dap] Basic implementation of a deferred request. #140260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[lldb][lldb-dap] Basic implementation of a deferred request. #140260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1796613
[lldb][lldb-dap] Basic implementation of a deferred request.
da-viper May 16, 2025
751aad3
[lldb][lldb-dap] add nodiscard `DeferRequest` function.
da-viper May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions lldb/tools/lldb-dap/DAP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ void DAP::SetTarget(const lldb::SBTarget target) {
}
}

bool DAP::HandleObject(const Message &M) {
bool DAP::HandleObject(const Message &M, bool &defer) {
TelemetryDispatcher dispatcher(&debugger);
dispatcher.Set("client_name", transport.GetClientName().str());
if (const auto *req = std::get_if<Request>(&M)) {
Expand All @@ -748,7 +748,7 @@ bool DAP::HandleObject(const Message &M) {
dispatcher.Set("client_data",
llvm::Twine("request_command:", req->command).str());
if (handler_pos != request_handlers.end()) {
handler_pos->second->Run(*req);
defer = handler_pos->second->Run(*req);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check the deferred call here like?

if (handler_pos->second->DeferRequest())
  m_pending_queue.push_back(M);
else
  handler_pos->second->Run(*req);

return true; // Success
}

Expand Down Expand Up @@ -918,17 +918,11 @@ llvm::Error DAP::Loop() {

// The launch sequence is special and we need to carefully handle
// packets in the right order. Until we've handled configurationDone,
bool add_to_pending_queue = false;

if (const protocol::Request *req =
std::get_if<protocol::Request>(&*next)) {
llvm::StringRef command = req->command;
if (command == "disconnect")
disconnecting = true;
if (!configuration_done)
add_to_pending_queue =
command != "initialize" && command != "configurationDone" &&
command != "disconnect" && !command.ends_with("Breakpoints");
}

const std::optional<CancelArguments> cancel_args =
Expand Down Expand Up @@ -956,8 +950,7 @@ llvm::Error DAP::Loop() {

{
std::lock_guard<std::mutex> guard(m_queue_mutex);
auto &queue = add_to_pending_queue ? m_pending_queue : m_queue;
queue.push_back(std::move(*next));
m_queue.push_back(std::move(*next));
}
m_queue_cv.notify_one();
}
Expand All @@ -984,9 +977,13 @@ llvm::Error DAP::Loop() {
// Unlock while we're processing the event.
lock.unlock();

if (!HandleObject(next))
bool defer_message = false;
if (!HandleObject(next, defer_message))
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"unhandled packet");
if (defer_message) {
m_pending_queue.push_back(next);
}
}

return ToError(queue_reader.get());
Expand Down
2 changes: 1 addition & 1 deletion lldb/tools/lldb-dap/DAP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ struct DAP {
/// listeing for its breakpoint events.
void SetTarget(const lldb::SBTarget target);

bool HandleObject(const protocol::Message &M);
bool HandleObject(const protocol::Message &M, bool &defer);

/// Disconnect the DAP session.
llvm::Error Disconnect();
Expand Down
4 changes: 4 additions & 0 deletions lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,8 @@ void AttachRequestHandler::PostRun() const {
dap.target.GetProcess().Continue();
}

bool AttachRequestHandler::DeferRequest() const {
return !dap.configuration_done;
}

} // namespace lldb_dap
4 changes: 4 additions & 0 deletions lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,8 @@ void LaunchRequestHandler::PostRun() const {
dap.target.GetProcess().Continue();
}

bool LaunchRequestHandler::DeferRequest() const {
return !dap.configuration_done;
}

} // namespace lldb_dap
8 changes: 6 additions & 2 deletions lldb/tools/lldb-dap/Handler/RequestHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
error.GetCString());
}

void BaseRequestHandler::Run(const Request &request) {
bool BaseRequestHandler::Run(const Request &request) {
// If this request was cancelled, send a cancelled response.
if (dap.IsCancelled(request)) {
Response cancelled{/*request_seq=*/request.seq,
Expand All @@ -135,12 +135,15 @@ void BaseRequestHandler::Run(const Request &request) {
/*message=*/eResponseMessageCancelled,
/*body=*/std::nullopt};
dap.Send(cancelled);
return;
return false;
}

lldb::SBMutex lock = dap.GetAPIMutex();
std::lock_guard<lldb::SBMutex> guard(lock);

if (DeferRequest()) {
return true;
}
// FIXME: After all the requests have migrated from LegacyRequestHandler >
// RequestHandler<> we should be able to move this into
// RequestHandler<>::operator().
Expand All @@ -149,6 +152,7 @@ void BaseRequestHandler::Run(const Request &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.
return false;
}

llvm::Error BaseRequestHandler::LaunchProcess(
Expand Down
11 changes: 10 additions & 1 deletion lldb/tools/lldb-dap/Handler/RequestHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ class BaseRequestHandler {

virtual ~BaseRequestHandler() = default;

void Run(const protocol::Request &);
/// Return `true` if the request should be deferred.
[[nodiscard]]
bool Run(const protocol::Request &);

[[nodiscard]]
virtual bool DeferRequest() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specifically about deferring until configurationDone is called. We don't really have support for an async request handler yet. I thought about implementing an AsyncRequestHandler that would allow the request handler to unblock the handler queue before sending a reply, but I haven't needed it just yet so I haven't sent that out for a PR.

Thats a long winded way of saying, should we make this more specific? Maybe DeferUntilConfigurationDone? I'm open to other names, just want to be clear.

return false;
}

virtual void operator()(const protocol::Request &request) const = 0;

Expand Down Expand Up @@ -203,6 +210,7 @@ class AttachRequestHandler
static llvm::StringLiteral GetCommand() { return "attach"; }
llvm::Error Run(const protocol::AttachRequestArguments &args) const override;
void PostRun() const override;
bool DeferRequest() const override;
};

class BreakpointLocationsRequestHandler
Expand Down Expand Up @@ -302,6 +310,7 @@ class LaunchRequestHandler
llvm::Error
Run(const protocol::LaunchRequestArguments &arguments) const override;
void PostRun() const override;
bool DeferRequest() const override;
};

class RestartRequestHandler : public LegacyRequestHandler {
Expand Down
Loading