[Lldb-commits] [lldb] [lldb] Refactoring JSONTransport into an abstract RPC Message Handler and transport layer. (PR #153121)

[John Harrison via lldb-commits]

================
@@ -938,88 +933,74 @@ void DAP::ClearCancelRequest(const CancelArguments &args) {
 }
 
 template <typename T>
-static std::optional<T> getArgumentsIfRequest(const Message &pm,
+static std::optional<T> getArgumentsIfRequest(const Request &req,
                                               llvm::StringLiteral command) {
-  auto *const req = std::get_if<Request>(&pm);
-  if (!req || req->command != command)
+  if (req.command != command)
     return std::nullopt;
 
   T args;
   llvm::json::Path::Root root;
-  if (!fromJSON(req->arguments, args, root))
+  if (!fromJSON(req.arguments, args, root))
     return std::nullopt;
 
   return args;
 }
 
-Status DAP::TransportHandler() {
-  llvm::set_thread_name(transport.GetClientName() + ".transport_handler");
+void DAP::OnEvent(const protocol::Event &event) {
+  // no-op, no supported events from the client to the server as of DAP v1.68.
+}
 
-  auto cleanup = llvm::make_scope_exit([&]() {
-    // Ensure we're marked as disconnecting when the reader exits.
+void DAP::OnRequest(const protocol::Request &request) {
+  if (request.command == "disconnect")
     disconnecting = true;
-    m_queue_cv.notify_all();
-  });
 
-  Status status;
-  auto handle = transport.RegisterReadObject<protocol::Message>(
-      m_loop,
-      [&](MainLoopBase &loop, llvm::Expected<protocol::Message> message) {
-        if (message.errorIsA<TransportEOFError>()) {
-          llvm::consumeError(message.takeError());
-          loop.RequestTermination();
-          return;
-        }
+  const std::optional<CancelArguments> cancel_args =
+      getArgumentsIfRequest<CancelArguments>(request, "cancel");
+  if (cancel_args) {
+    {
+      std::lock_guard<std::mutex> guard(m_cancelled_requests_mutex);
+      if (cancel_args->requestId)
+        m_cancelled_requests.insert(*cancel_args->requestId);
+    }
 
-        if (llvm::Error err = message.takeError()) {
-          status = Status::FromError(std::move(err));
-          loop.RequestTermination();
-          return;
-        }
+    // If a cancel is requested for the active request, make a best
+    // effort attempt to interrupt.
+    std::lock_guard<std::mutex> guard(m_active_request_mutex);
+    if (m_active_request && cancel_args->requestId == m_active_request->seq) {
+      DAP_LOG(log, "({0}) interrupting inflight request (command={1} seq={2})",
+              m_client_name, m_active_request->command, m_active_request->seq);
+      debugger.RequestInterrupt();
+    }
+  }
 
-        if (const protocol::Request *req =
-                std::get_if<protocol::Request>(&*message);
-            req && req->arguments == "disconnect")
-          disconnecting = true;
-
-        const std::optional<CancelArguments> cancel_args =
-            getArgumentsIfRequest<CancelArguments>(*message, "cancel");
-        if (cancel_args) {
-          {
-            std::lock_guard<std::mutex> guard(m_cancelled_requests_mutex);
-            if (cancel_args->requestId)
-              m_cancelled_requests.insert(*cancel_args->requestId);
-          }
+  std::lock_guard<std::mutex> guard(m_queue_mutex);
+  DAP_LOG(log, "({0}) queued (command={1} seq={2})", m_client_name,
+          request.command, request.seq);
+  m_queue.push_back(request);
+  m_queue_cv.notify_one();
+}
 
-          // If a cancel is requested for the active request, make a best
-          // effort attempt to interrupt.
-          std::lock_guard<std::mutex> guard(m_active_request_mutex);
-          if (m_active_request &&
-              cancel_args->requestId == m_active_request->seq) {
-            DAP_LOG(log,
-                    "({0}) interrupting inflight request (command={1} seq={2})",
-                    transport.GetClientName(), m_active_request->command,
-                    m_active_request->seq);
-            debugger.RequestInterrupt();
-          }
-        }
+void DAP::OnResponse(const protocol::Response &response) {
+  std::lock_guard<std::mutex> guard(m_queue_mutex);
+  DAP_LOG(log, "({0}) queued (command={1} seq={2})", m_client_name,
+          response.command, response.request_seq);
+  m_queue.push_back(response);
+  m_queue_cv.notify_one();
+}
 
-        std::lock_guard<std::mutex> guard(m_queue_mutex);
-        m_queue.push_back(std::move(*message));
-        m_queue_cv.notify_one();
-      });
-  if (auto err = handle.takeError())
-    return Status::FromError(std::move(err));
-  if (llvm::Error err = m_loop.Run().takeError())
-    return Status::FromError(std::move(err));
-  return status;
+void DAP::TransportHandler(llvm::Error *error) {
+  llvm::ErrorAsOutParameter ErrAsOutParam(*error);
+  auto cleanup = llvm::make_scope_exit([&]() {
+    // Ensure we're marked as disconnecting when the reader exits.
+    disconnecting = true;
----------------
ashgti wrote:

Added a lock guard around `disconnecting` and made it private.

https://github.com/llvm/llvm-project/pull/153121