llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) <details> <summary>Changes</summary> Fixes #<!-- -->140154 Basic implementation of defering requests. It no longer depends on a white list of request, So if there is any future breakpoint types added to the DAP specification. it will not break existing binary. It can further be extended to defer depending of the request arguments. --- Full diff: https://github.com/llvm/llvm-project/pull/140260.diff 6 Files Affected: - (modified) lldb/tools/lldb-dap/DAP.cpp (+8-11) - (modified) lldb/tools/lldb-dap/DAP.h (+1-1) - (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+4) - (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+4) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+6-2) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+7-1) ``````````diff diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 56a0c38b00037..a1f738eef5fcc 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -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)) { @@ -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); return true; // Success } @@ -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 = @@ -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(); } @@ -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()); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index c1a1130b1e59f..a77a1561c5db2 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -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(); diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 0293ffbd0c922..96fb7682f240a 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -160,4 +160,8 @@ void AttachRequestHandler::PostRun() const { dap.target.GetProcess().Continue(); } +bool AttachRequestHandler::DeferRequest() const { + return !dap.configuration_done; +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 22d1a090187d8..e4163827ac69c 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -88,4 +88,8 @@ void LaunchRequestHandler::PostRun() const { dap.target.GetProcess().Continue(); } +bool LaunchRequestHandler::DeferRequest() const { + return !dap.configuration_done; +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index 93bc80a38e29d..074a76c928240 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -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, @@ -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(). @@ -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( diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index eaebaf6619bbd..bac193e6ce59e 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -46,7 +46,11 @@ class BaseRequestHandler { virtual ~BaseRequestHandler() = default; - void Run(const protocol::Request &); + /// Return `true` if the request should be deferred. + [[nodiscard]] + bool Run(const protocol::Request &); + + virtual bool DeferRequest() const { return false; }; virtual void operator()(const protocol::Request &request) const = 0; @@ -203,6 +207,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 @@ -302,6 +307,7 @@ class LaunchRequestHandler llvm::Error Run(const protocol::LaunchRequestArguments &arguments) const override; void PostRun() const override; + bool DeferRequest() const override; }; class RestartRequestHandler : public LegacyRequestHandler { `````````` </details> https://github.com/llvm/llvm-project/pull/140260 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
