[Lldb-commits] [lldb] [lldb-dap] Refactoring IOStream into Transport handler. (PR #130026)

John Harrison via lldb-commits lldb-commits at lists.llvm.org
Mon Mar 10 14:47:12 PDT 2025


https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/130026

>From 88f1cf3469c9f24a768f5147c22cfe414aea014e Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Sat, 8 Mar 2025 17:55:34 -0800
Subject: [PATCH 1/4] [lldb-dap] Adding logging helpers.

Improving logging by defining new helpers for more uniform log handling. This should help us clearly identify log messages and helps abstract the underlying log type within the macro in case we want to update the log handler in the future.
---
 lldb/tools/lldb-dap/DAP.cpp                   | 42 ++++----------
 lldb/tools/lldb-dap/DAP.h                     |  4 +-
 lldb/tools/lldb-dap/DAPLog.h                  | 58 +++++++++++++++++++
 .../Handler/InitializeRequestHandler.cpp      |  2 +-
 lldb/tools/lldb-dap/lldb-dap.cpp              | 33 ++++-------
 5 files changed, 83 insertions(+), 56 deletions(-)
 create mode 100644 lldb/tools/lldb-dap/DAPLog.h

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 1f7b25e7c5bcc..88bd628699bd3 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
+#include "DAPLog.h"
 #include "Handler/ResponseHandler.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
@@ -25,6 +26,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -61,10 +63,10 @@ const char DEV_NULL[] = "/dev/null";
 
 namespace lldb_dap {
 
-DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log,
+DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
          lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
          std::vector<std::string> pre_init_commands)
-    : name(std::move(name)), debug_adapter_path(path), log(log),
+    : client_name(client_name), debug_adapter_path(path), log(log),
       input(std::move(input)), output(std::move(output)),
       broadcaster("lldb-dap"), exception_breakpoints(),
       pre_init_commands(std::move(pre_init_commands)),
@@ -239,14 +241,7 @@ void DAP::SendJSON(const llvm::json::Value &json) {
   std::lock_guard<std::mutex> locker(mutex);
   SendJSON(json_str);
 
-  if (log) {
-    auto now = std::chrono::duration<double>(
-        std::chrono::system_clock::now().time_since_epoch());
-    *log << llvm::formatv("{0:f9} {1} <-- ", now.count(), name).str()
-         << std::endl
-         << "Content-Length: " << json_str.size() << "\r\n\r\n"
-         << llvm::formatv("{0:2}", json).str() << std::endl;
-  }
+  DAP_LOG(log, "({0}) <-- {1}", client_name, json_str);
 }
 
 // Read a JSON packet from the "in" stream.
@@ -270,13 +265,7 @@ std::string DAP::ReadJSON() {
   if (!input.read_full(log, length, json_str))
     return json_str;
 
-  if (log) {
-    auto now = std::chrono::duration<double>(
-        std::chrono::system_clock::now().time_since_epoch());
-    *log << llvm::formatv("{0:f9} {1} --> ", now.count(), name).str()
-         << std::endl
-         << "Content-Length: " << length << "\r\n\r\n";
-  }
+  DAP_LOG(log, "{0} --> {1}", client_name, json_str);
   return json_str;
 }
 
@@ -711,22 +700,15 @@ PacketStatus DAP::GetNextObject(llvm::json::Object &object) {
 
   llvm::StringRef json_sref(json);
   llvm::Expected<llvm::json::Value> json_value = llvm::json::parse(json_sref);
-  if (auto error = json_value.takeError()) {
-    std::string error_str = llvm::toString(std::move(error));
-    if (log)
-      *log << "error: failed to parse JSON: " << error_str << std::endl
-           << json << std::endl;
+  if (!json_value) {
+    DAP_LOG_ERROR(log, json_value.takeError(),
+                  "({1}) failed to parse JSON: {0}", client_name);
     return PacketStatus::JSONMalformed;
   }
 
-  if (log) {
-    *log << llvm::formatv("{0:2}", *json_value).str() << std::endl;
-  }
-
   llvm::json::Object *object_ptr = json_value->getAsObject();
   if (!object_ptr) {
-    if (log)
-      *log << "error: json packet isn't a object" << std::endl;
+    DAP_LOG(log, "error: json packet isn't a object");
     return PacketStatus::JSONNotObject;
   }
   object = *object_ptr;
@@ -744,9 +726,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
       return true; // Success
     }
 
-    if (log)
-      *log << "error: unhandled command \"" << command.data() << "\""
-           << std::endl;
+    DAP_LOG(log, "({0}) error: unhandled command '{1}'", client_name, command);
     return false; // Fail
   }
 
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 8b2e498a28c95..3ff1992b61f5b 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -145,7 +145,7 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
 };
 
 struct DAP {
-  std::string name;
+  llvm::StringRef client_name;
   llvm::StringRef debug_adapter_path;
   std::ofstream *log;
   InputStream input;
@@ -210,7 +210,7 @@ struct DAP {
   // will contain that expression.
   std::string last_nonempty_var_expression;
 
-  DAP(std::string name, llvm::StringRef path, std::ofstream *log,
+  DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
       lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
       std::vector<std::string> pre_init_commands);
   ~DAP();
diff --git a/lldb/tools/lldb-dap/DAPLog.h b/lldb/tools/lldb-dap/DAPLog.h
new file mode 100644
index 0000000000000..647d787dc8196
--- /dev/null
+++ b/lldb/tools/lldb-dap/DAPLog.h
@@ -0,0 +1,58 @@
+//===-- DAPLog.h ----------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_DAPLOG_H
+#define LLDB_TOOLS_LLDB_DAP_DAPLOG_H
+
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <chrono>
+#include <fstream>
+#include <string>
+
+// Write a message to log, if logging is enabled.
+#define DAP_LOG(log, ...)                                                      \
+  do {                                                                         \
+    ::std::ofstream *log_private = (log);                                      \
+    if (log_private) {                                                         \
+      ::std::chrono::duration<double> now{                                     \
+          ::std::chrono::system_clock::now().time_since_epoch()};              \
+      *log_private << ::llvm::formatv("{0:f9} ", now.count()).str()            \
+                   << ::llvm::formatv(__VA_ARGS__).str() << std::endl;         \
+    }                                                                          \
+  } while (0)
+
+// Write message to log, if error is set. In the log message refer to the error
+// with {0}. Error is cleared regardless of whether logging is enabled.
+#define DAP_LOG_ERROR(log, error, ...)                                         \
+  do {                                                                         \
+    ::std::ofstream *log_private = (log);                                      \
+    ::llvm::Error error_private = (error);                                     \
+    if (log_private && error_private) {                                        \
+      ::std::chrono::duration<double> now{                                     \
+          std::chrono::system_clock::now().time_since_epoch()};                \
+      *log_private << ::llvm::formatv("{0:f9} ", now.count()).str()            \
+                   << ::lldb_dap::FormatError(::std::move(error_private),      \
+                                              __VA_ARGS__)                     \
+                   << std::endl;                                               \
+    } else                                                                     \
+      ::llvm::consumeError(::std::move(error_private));                        \
+  } while (0)
+
+namespace lldb_dap {
+
+template <typename... Args>
+inline auto FormatError(llvm::Error error, const char *format, Args &&...args) {
+  return llvm::formatv(format, llvm::toString(std::move(error)),
+                       std::forward<Args>(args)...)
+      .str();
+}
+} // namespace lldb_dap
+
+#endif
\ No newline at end of file
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index 5bb73a7ec0d85..7b7d8d5cedaa6 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -111,7 +111,7 @@ void ProgressEventThreadFunction(DAP &dap) {
 // them prevent multiple threads from writing simultaneously so no locking
 // is required.
 static void EventThreadFunction(DAP &dap) {
-  llvm::set_thread_name(dap.name + ".event_handler");
+  llvm::set_thread_name(dap.client_name + ".event_handler");
   lldb::SBEvent event;
   lldb::SBListener listener = dap.debugger.GetListener();
   dap.broadcaster.AddListener(listener, eBroadcastBitStopEventThread);
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index a5d9978e30248..ab40b75e5425d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
+#include "DAPLog.h"
 #include "EventHelper.h"
 #include "Handler/RequestHandler.h"
 #include "RunInTerminal.h"
@@ -294,8 +295,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
   }
 
   std::string address = llvm::join(listener->GetListeningConnectionURI(), ", ");
-  if (log)
-    *log << "started with connection listeners " << address << "\n";
+  DAP_LOG(log, "started with connection listeners {0}", address);
 
   llvm::outs() << "Listening for: " << address << "\n";
   // Ensure listening address are flushed for calles to retrieve the resolve
@@ -315,13 +315,8 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
                                           &dap_sessions_mutex, &dap_sessions,
                                           &clientCount](
                                              std::unique_ptr<Socket> sock) {
-    std::string name = llvm::formatv("client_{0}", clientCount++).str();
-    if (log) {
-      auto now = std::chrono::duration<double>(
-          std::chrono::system_clock::now().time_since_epoch());
-      *log << llvm::formatv("{0:f9}", now.count()).str()
-           << " client connected: " << name << "\n";
-    }
+    std::string client_name = llvm::formatv("client_{0}", clientCount++).str();
+    DAP_LOG(log, "({0}) client connected", client_name);
 
     lldb::IOObjectSP io(std::move(sock));
 
@@ -329,8 +324,8 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
     // client.
     std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex,
                         &dap_sessions]() {
-      llvm::set_thread_name(name + ".runloop");
-      DAP dap = DAP(name, program_path, log, io, io, default_repl_mode,
+      llvm::set_thread_name(client_name + ".runloop");
+      DAP dap = DAP(client_name, program_path, log, io, io, default_repl_mode,
                     pre_init_commands);
 
       if (auto Err = dap.ConfigureIO()) {
@@ -351,13 +346,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
                                     "DAP session error: ");
       }
 
-      if (log) {
-        auto now = std::chrono::duration<double>(
-            std::chrono::system_clock::now().time_since_epoch());
-        *log << llvm::formatv("{0:f9}", now.count()).str()
-             << " client closed: " << name << "\n";
-      }
-
+      DAP_LOG(log, "({0}) client disconnected", client_name);
       std::unique_lock<std::mutex> lock(dap_sessions_mutex);
       dap_sessions.erase(io.get());
       std::notify_all_at_thread_exit(dap_sessions_condition, std::move(lock));
@@ -374,9 +363,9 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
     return status.takeError();
   }
 
-  if (log)
-    *log << "lldb-dap server shutdown requested, disconnecting remaining "
-            "clients...\n";
+  DAP_LOG(
+      log,
+      "lldb-dap server shutdown requested, disconnecting remaining clients...");
 
   bool client_failed = false;
   {
@@ -385,7 +374,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
       auto error = dap->Disconnect();
       if (error.Fail()) {
         client_failed = true;
-        llvm::errs() << "DAP client " << dap->name
+        llvm::errs() << "DAP client " << dap->client_name
                      << " disconnected failed: " << error.GetCString() << "\n";
       }
       // Close the socket to ensure the DAP::Loop read finishes.

>From 7419a6d5c8db5c26e79c430542239522d5445bb4 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Thu, 6 Mar 2025 10:18:36 +0100
Subject: [PATCH 2/4] [lldb-dap] Refactoring IOStream into Transport handler.

Instead of having two discrete InputStream and OutputStream helpers,
this merges the two into a unifed 'Transport' handler.

This handler is responsible for reading the DAP message headers, parsing
the resulting JSON and converting the messages into
`lldb_dap::protocol::Message`s for both input and output.
---
 .../test/tools/lldb-dap/dap_server.py         |   4 +-
 lldb/tools/lldb-dap/CMakeLists.txt            |   4 +-
 lldb/tools/lldb-dap/DAP.cpp                   | 110 ++++---------
 lldb/tools/lldb-dap/DAP.h                     |  18 +--
 lldb/tools/lldb-dap/IOStream.cpp              |  73 ---------
 lldb/tools/lldb-dap/IOStream.h                |  42 -----
 lldb/tools/lldb-dap/Transport.cpp             | 146 ++++++++++++++++++
 lldb/tools/lldb-dap/Transport.h               |  54 +++++++
 8 files changed, 240 insertions(+), 211 deletions(-)
 delete mode 100644 lldb/tools/lldb-dap/IOStream.cpp
 delete mode 100644 lldb/tools/lldb-dap/IOStream.h
 create mode 100644 lldb/tools/lldb-dap/Transport.cpp
 create mode 100644 lldb/tools/lldb-dap/Transport.h

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 9471594b66012..0fea3419d9725 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -337,7 +337,7 @@ def send_recv(self, command):
                     self.send_packet(
                         {
                             "type": "response",
-                            "seq": -1,
+                            "seq": 0,
                             "request_seq": response_or_request["seq"],
                             "success": True,
                             "command": "runInTerminal",
@@ -349,7 +349,7 @@ def send_recv(self, command):
                     self.send_packet(
                         {
                             "type": "response",
-                            "seq": -1,
+                            "seq": 0,
                             "request_seq": response_or_request["seq"],
                             "success": True,
                             "command": "startDebugging",
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 9a2d604f4d573..8a76cb58dbcab 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -28,14 +28,14 @@ add_lldb_tool(lldb-dap
   FifoFiles.cpp
   FunctionBreakpoint.cpp
   InstructionBreakpoint.cpp
-  IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
   OutputRedirector.cpp
   ProgressEvent.cpp
+  Protocol.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
-  Protocol.cpp
+  Transport.cpp
   Watchpoint.cpp
 
   Handler/ResponseHandler.cpp
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 88bd628699bd3..6d19eb2917a1b 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -12,6 +12,7 @@
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "OutputRedirector.h"
+#include "Transport.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
@@ -67,7 +68,7 @@ DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
          lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
          std::vector<std::string> pre_init_commands)
     : client_name(client_name), debug_adapter_path(path), log(log),
-      input(std::move(input)), output(std::move(output)),
+      transport(client_name, std::move(input), std::move(output)),
       broadcaster("lldb-dap"), exception_breakpoints(),
       pre_init_commands(std::move(pre_init_commands)),
       focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false),
@@ -221,52 +222,27 @@ void DAP::StopEventHandlers() {
   }
 }
 
-// Send the JSON in "json_str" to the "out" stream. Correctly send the
-// "Content-Length:" field followed by the length, followed by the raw
-// JSON bytes.
-void DAP::SendJSON(const std::string &json_str) {
-  output.write_full("Content-Length: ");
-  output.write_full(llvm::utostr(json_str.size()));
-  output.write_full("\r\n\r\n");
-  output.write_full(json_str);
-}
-
 // Serialize the JSON value into a string and send the JSON packet to
 // the "out" stream.
 void DAP::SendJSON(const llvm::json::Value &json) {
-  std::string json_str;
-  llvm::raw_string_ostream strm(json_str);
-  strm << json;
-  static std::mutex mutex;
-  std::lock_guard<std::mutex> locker(mutex);
-  SendJSON(json_str);
-
-  DAP_LOG(log, "({0}) <-- {1}", client_name, json_str);
-}
-
-// Read a JSON packet from the "in" stream.
-std::string DAP::ReadJSON() {
-  std::string length_str;
-  std::string json_str;
-  int length;
-
-  if (!input.read_expected(log, "Content-Length: "))
-    return json_str;
-
-  if (!input.read_line(log, length_str))
-    return json_str;
-
-  if (!llvm::to_integer(length_str, length))
-    return json_str;
-
-  if (!input.read_expected(log, "\r\n"))
-    return json_str;
-
-  if (!input.read_full(log, length, json_str))
-    return json_str;
-
-  DAP_LOG(log, "{0} --> {1}", client_name, json_str);
-  return json_str;
+  // FIXME: Instead of parsing the output message from JSON, pass the `Message`
+  // as parameter to `SendJSON`.
+  protocol::Message M;
+  llvm::json::Path::Root root;
+  if (!protocol::fromJSON(json, M, root)) {
+    if (log) {
+      std::string error;
+      llvm::raw_string_ostream OS(error);
+      root.printErrorContext(json, OS);
+      *log << "encoding failure: " << error << "\n";
+    }
+    return;
+  }
+  auto status = transport.Write(log, M);
+  if (status.Fail() && log)
+    *log << llvm::formatv("failed to send {0}: {1}\n", llvm::json::Value(M),
+                          status.AsCString())
+                .str();
 }
 
 // "OutputEvent": {
@@ -693,29 +669,10 @@ void DAP::SetTarget(const lldb::SBTarget target) {
   }
 }
 
-PacketStatus DAP::GetNextObject(llvm::json::Object &object) {
-  std::string json = ReadJSON();
-  if (json.empty())
-    return PacketStatus::EndOfFile;
-
-  llvm::StringRef json_sref(json);
-  llvm::Expected<llvm::json::Value> json_value = llvm::json::parse(json_sref);
-  if (!json_value) {
-    DAP_LOG_ERROR(log, json_value.takeError(),
-                  "({1}) failed to parse JSON: {0}", client_name);
-    return PacketStatus::JSONMalformed;
-  }
-
-  llvm::json::Object *object_ptr = json_value->getAsObject();
-  if (!object_ptr) {
-    DAP_LOG(log, "error: json packet isn't a object");
-    return PacketStatus::JSONNotObject;
-  }
-  object = *object_ptr;
-  return PacketStatus::Success;
-}
-
-bool DAP::HandleObject(const llvm::json::Object &object) {
+bool DAP::HandleObject(const protocol::Message &M) {
+  // FIXME: Directly handle `Message` instead of serializing to JSON.
+  llvm::json::Value v = toJSON(M);
+  llvm::json::Object object = *v.getAsObject();
   const auto packet_type = GetString(object, "type");
   if (packet_type == "request") {
     const auto command = GetString(object, "command");
@@ -818,19 +775,16 @@ llvm::Error DAP::Loop() {
     StopEventHandlers();
   });
   while (!disconnecting) {
-    llvm::json::Object object;
-    lldb_dap::PacketStatus status = GetNextObject(object);
-
-    if (status == lldb_dap::PacketStatus::EndOfFile) {
-      break;
-    }
-
-    if (status != lldb_dap::PacketStatus::Success) {
-      return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                     "failed to send packet");
+    auto next = transport.Read(log);
+    if (auto Err = next.takeError()) {
+      // On EOF, simply break out of the loop.
+      std::error_code ec = llvm::errorToErrorCode(std::move(Err));
+      if (ec == Transport::kEOF)
+        break;
+      return llvm::errorCodeToError(ec);
     }
 
-    if (!HandleObject(object)) {
+    if (!HandleObject(*next)) {
       return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                      "unhandled packet");
     }
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 3ff1992b61f5b..3254d0c1422cc 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -14,11 +14,12 @@
 #include "FunctionBreakpoint.h"
 #include "Handler/RequestHandler.h"
 #include "Handler/ResponseHandler.h"
-#include "IOStream.h"
 #include "InstructionBreakpoint.h"
 #include "OutputRedirector.h"
 #include "ProgressEvent.h"
+#include "Protocol.h"
 #include "SourceBreakpoint.h"
+#include "Transport.h"
 #include "lldb/API/SBBroadcaster.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBDebugger.h"
@@ -39,7 +40,6 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Threading.h"
-#include <map>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -148,8 +148,7 @@ struct DAP {
   llvm::StringRef client_name;
   llvm::StringRef debug_adapter_path;
   std::ofstream *log;
-  InputStream input;
-  OutputStream output;
+  Transport transport;
   lldb::SBFile in;
   OutputRedirector out;
   OutputRedirector err;
@@ -233,8 +232,6 @@ struct DAP {
   // the "out" stream.
   void SendJSON(const llvm::json::Value &json);
 
-  std::string ReadJSON();
-
   void SendOutput(OutputType o, const llvm::StringRef output);
 
   void SendProgressEvent(uint64_t progress_id, const char *message,
@@ -307,8 +304,7 @@ struct DAP {
   /// listeing for its breakpoint events.
   void SetTarget(const lldb::SBTarget target);
 
-  PacketStatus GetNextObject(llvm::json::Object &object);
-  bool HandleObject(const llvm::json::Object &object);
+  bool HandleObject(const protocol::Message &M);
 
   /// Disconnect the DAP session.
   lldb::SBError Disconnect();
@@ -382,12 +378,6 @@ struct DAP {
   InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id);
 
   InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
-
-private:
-  // Send the JSON in "json_str" to the "out" stream. Correctly send the
-  // "Content-Length:" field followed by the length, followed by the raw
-  // JSON bytes.
-  void SendJSON(const std::string &json_str);
 };
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
deleted file mode 100644
index ee22a297ec248..0000000000000
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ /dev/null
@@ -1,73 +0,0 @@
-//===-- IOStream.cpp --------------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "IOStream.h"
-#include "lldb/Utility/IOObject.h"
-#include "lldb/Utility/Status.h"
-#include <fstream>
-#include <string>
-
-using namespace lldb_dap;
-
-bool OutputStream::write_full(llvm::StringRef str) {
-  if (!descriptor)
-    return false;
-
-  size_t num_bytes = str.size();
-  auto status = descriptor->Write(str.data(), num_bytes);
-  return status.Success();
-}
-
-bool InputStream::read_full(std::ofstream *log, size_t length,
-                            std::string &text) {
-  if (!descriptor)
-    return false;
-
-  std::string data;
-  data.resize(length);
-
-  auto status = descriptor->Read(data.data(), length);
-  if (status.Fail())
-    return false;
-
-  text += data.substr(0, length);
-  return true;
-}
-
-bool InputStream::read_line(std::ofstream *log, std::string &line) {
-  line.clear();
-  while (true) {
-    std::string next;
-    if (!read_full(log, 1, next))
-      return false;
-
-    // If EOF is encoutnered, '' is returned, break out of this loop.
-    if (next.empty())
-      return false;
-
-    line += next;
-
-    if (llvm::StringRef(line).ends_with("\r\n"))
-      break;
-  }
-  line.erase(line.size() - 2);
-  return true;
-}
-
-bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
-  std::string result;
-  if (!read_full(log, expected.size(), result))
-    return false;
-  if (expected != result) {
-    if (log)
-      *log << "Warning: Expected '" << expected.str() << "', got '" << result
-           << "\n";
-    return false;
-  }
-  return true;
-}
diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h
deleted file mode 100644
index e9fb8e11c92da..0000000000000
--- a/lldb/tools/lldb-dap/IOStream.h
+++ /dev/null
@@ -1,42 +0,0 @@
-//===-- IOStream.h ----------------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_TOOLS_LLDB_DAP_IOSTREAM_H
-#define LLDB_TOOLS_LLDB_DAP_IOSTREAM_H
-
-#include "lldb/lldb-forward.h"
-#include "llvm/ADT/StringRef.h"
-#include <fstream>
-#include <string>
-
-namespace lldb_dap {
-
-struct InputStream {
-  lldb::IOObjectSP descriptor;
-
-  explicit InputStream(lldb::IOObjectSP descriptor)
-      : descriptor(std::move(descriptor)) {}
-
-  bool read_full(std::ofstream *log, size_t length, std::string &text);
-
-  bool read_line(std::ofstream *log, std::string &line);
-
-  bool read_expected(std::ofstream *log, llvm::StringRef expected);
-};
-
-struct OutputStream {
-  lldb::IOObjectSP descriptor;
-
-  explicit OutputStream(lldb::IOObjectSP descriptor)
-      : descriptor(std::move(descriptor)) {}
-
-  bool write_full(llvm::StringRef str);
-};
-} // namespace lldb_dap
-
-#endif
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
new file mode 100644
index 0000000000000..f49bf373c4aff
--- /dev/null
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -0,0 +1,146 @@
+//===-- Transport.cpp -----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Transport.h"
+#include "Protocol.h"
+#include "c++/v1/__system_error/error_code.h"
+#include "lldb/Utility/IOObject.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+#include <string>
+#include <system_error>
+#include <utility>
+
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
+
+static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) {
+  if (!descriptor || !descriptor->IsValid())
+    return createStringError("transport input is closed");
+
+  std::string data;
+  data.resize(length);
+
+  auto status = descriptor->Read(data.data(), length);
+  if (status.Fail())
+    return status.takeError();
+
+  // If we got back zero then we have reached EOF.
+  if (length == 0)
+    return createStringError(Transport::kEOF, "end-of-file");
+
+  return data.substr(0, length);
+}
+
+static Expected<std::string> ReadUntil(IOObjectSP &descriptor,
+                                       StringRef delimiter) {
+  std::string buffer;
+  buffer.reserve(delimiter.size() + 1);
+  while (!llvm::StringRef(buffer).ends_with(delimiter)) {
+    auto next = ReadFull(descriptor, 1);
+    if (auto Err = next.takeError())
+      return std::move(Err);
+    buffer += *next;
+  }
+  return buffer.substr(0, buffer.size() - delimiter.size());
+}
+
+static Error ReadExpected(IOObjectSP &descriptor, StringRef want) {
+  auto got = ReadFull(descriptor, want.size());
+  if (auto Err = got.takeError())
+    return Err;
+  if (*got != want) {
+    return createStringError("want %s, got %s", want.str().c_str(),
+                             got->c_str());
+  }
+  return Error::success();
+}
+
+namespace lldb_dap {
+
+const std::error_code Transport::kEOF =
+    std::error_code(0x1001, std::generic_category());
+
+Transport::Transport(StringRef client_name, IOObjectSP input, IOObjectSP output)
+    : m_client_name(client_name), m_input(std::move(input)),
+      m_output(std::move(output)) {}
+
+Expected<protocol::Message> Transport::Read(std::ofstream *log) {
+  // If we don't find the expected header we have reached EOF.
+  if (auto Err = ReadExpected(m_input, "Content-Length: "))
+    return std::move(Err);
+
+  auto rawLength = ReadUntil(m_input, "\r\n\r\n");
+  if (auto Err = rawLength.takeError())
+    return std::move(Err);
+
+  size_t length;
+  if (!to_integer(*rawLength, length))
+    return createStringError("invalid content length %s", rawLength->c_str());
+
+  auto rawJSON = ReadFull(m_input, length);
+  if (auto Err = rawJSON.takeError())
+    return std::move(Err);
+  if (rawJSON->length() != length)
+    return createStringError(
+        "malformed request, expected %ld bytes, got %ld bytes", length,
+        rawJSON->length());
+
+  if (log) {
+    auto now = std::chrono::duration<double>(
+        std::chrono::system_clock::now().time_since_epoch());
+    *log << formatv("{0:f9} <-- ({1}) {2}\n", now.count(), m_client_name,
+                    *rawJSON)
+                .str();
+  }
+
+  auto JSON = json::parse(*rawJSON);
+  if (auto Err = JSON.takeError()) {
+    return createStringError("malformed JSON %s\n%s", rawJSON->c_str(),
+                             llvm::toString(std::move(Err)).c_str());
+  }
+
+  protocol::Message M;
+  llvm::json::Path::Root Root;
+  if (!fromJSON(*JSON, M, Root)) {
+    std::string error;
+    raw_string_ostream OS(error);
+    Root.printErrorContext(*JSON, OS);
+    return createStringError("malformed request: %s", error.c_str());
+  }
+  return std::move(M);
+}
+
+lldb_private::Status Transport::Write(std::ofstream *log,
+                                      const protocol::Message &M) {
+  if (!m_output || !m_output->IsValid())
+    return Status("transport output is closed");
+
+  std::string JSON = formatv("{0}", toJSON(M)).str();
+
+  if (log) {
+    auto now = std::chrono::duration<double>(
+        std::chrono::system_clock::now().time_since_epoch());
+    *log << formatv("{0:f9} --> ({1}) {2}\n", now.count(), m_client_name, JSON)
+                .str();
+  }
+
+  std::string Output;
+  raw_string_ostream OS(Output);
+  OS << "Content-Length: " << JSON.length() << "\r\n\r\n" << JSON;
+  size_t num_bytes = Output.size();
+  return m_output->Write(Output.data(), num_bytes);
+}
+
+} // end namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
new file mode 100644
index 0000000000000..0150b762ceb10
--- /dev/null
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -0,0 +1,54 @@
+//===-- Transport.h -------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Debug Adapter Protocol transport layer for encoding and decoding protocol
+// messages.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
+#define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
+
+#include "Protocol.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include <fstream>
+
+namespace lldb_dap {
+
+/// A transport class that performs the Debug Adapter Protocol communication
+/// with the client.
+class Transport {
+public:
+  Transport(llvm::StringRef client_name, lldb::IOObjectSP input,
+            lldb::IOObjectSP output);
+  ~Transport() = default;
+
+  Transport(const Transport &rhs) = delete;
+  void operator=(const Transport &rhs) = delete;
+
+  /// Error code returned this if EOF is encountered.
+  static const std::error_code kEOF;
+
+  /// Writes a Debug Adater Protocol message to the output stream.
+  lldb_private::Status Write(std::ofstream *log, const protocol::Message &M);
+
+  /// Reads the next Debug Adater Protocol message from the input stream.
+  llvm::Expected<protocol::Message> Read(std::ofstream *log);
+
+private:
+  llvm::StringRef m_client_name;
+  lldb::IOObjectSP m_input;
+  lldb::IOObjectSP m_output;
+};
+
+} // namespace lldb_dap
+
+#endif

>From d1ff9c886b37697211ee61167530ce77766bd771 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Sat, 8 Mar 2025 17:56:54 -0800
Subject: [PATCH 3/4] Adjusting `Transport::Read` to return an
 `optional<string>` and adding logging directly to the `Transport` class.

---
 lldb/tools/lldb-dap/DAP.cpp       |  45 ++++------
 lldb/tools/lldb-dap/DAP.h         |  37 ++++++--
 lldb/tools/lldb-dap/Transport.cpp | 141 ++++++++++++++----------------
 lldb/tools/lldb-dap/Transport.h   |  17 ++--
 lldb/tools/lldb-dap/lldb-dap.cpp  |  12 ++-
 5 files changed, 132 insertions(+), 120 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 6d19eb2917a1b..a3a0b0ad2f452 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -64,12 +64,12 @@ const char DEV_NULL[] = "/dev/null";
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
-         lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
-         std::vector<std::string> pre_init_commands)
-    : client_name(client_name), debug_adapter_path(path), log(log),
-      transport(client_name, std::move(input), std::move(output)),
-      broadcaster("lldb-dap"), exception_breakpoints(),
+DAP::DAP(llvm::StringRef path, std::ofstream *log,
+         const ReplMode default_repl_mode,
+         const std::vector<std::string> &pre_init_commands,
+         llvm::StringRef client_name, Transport &transport)
+    : debug_adapter_path(path), log(log), client_name(client_name),
+      transport(transport), broadcaster("lldb-dap"), exception_breakpoints(),
       pre_init_commands(std::move(pre_init_commands)),
       focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false),
       enable_auto_variable_summaries(false),
@@ -79,7 +79,7 @@ DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
       configuration_done_sent(false), waiting_for_run_in_terminal(false),
       progress_event_reporter(
           [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
-      reverse_request_seq(0), repl_mode(repl_mode) {}
+      reverse_request_seq(0), repl_mode(default_repl_mode) {}
 
 DAP::~DAP() = default;
 
@@ -227,22 +227,17 @@ void DAP::StopEventHandlers() {
 void DAP::SendJSON(const llvm::json::Value &json) {
   // FIXME: Instead of parsing the output message from JSON, pass the `Message`
   // as parameter to `SendJSON`.
-  protocol::Message M;
+  protocol::Message message;
   llvm::json::Path::Root root;
-  if (!protocol::fromJSON(json, M, root)) {
-    if (log) {
-      std::string error;
-      llvm::raw_string_ostream OS(error);
-      root.printErrorContext(json, OS);
-      *log << "encoding failure: " << error << "\n";
-    }
+  if (!fromJSON(json, message, root)) {
+    DAP_LOG_ERROR(log, root.getError(), "({1}) encoding failed: {0}",
+                  client_name);
     return;
   }
-  auto status = transport.Write(log, M);
-  if (status.Fail() && log)
-    *log << llvm::formatv("failed to send {0}: {1}\n", llvm::json::Value(M),
-                          status.AsCString())
-                .str();
+  auto err = transport.Write(message);
+  if (err) {
+    DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}", client_name);
+  }
 }
 
 // "OutputEvent": {
@@ -775,13 +770,9 @@ llvm::Error DAP::Loop() {
     StopEventHandlers();
   });
   while (!disconnecting) {
-    auto next = transport.Read(log);
-    if (auto Err = next.takeError()) {
-      // On EOF, simply break out of the loop.
-      std::error_code ec = llvm::errorToErrorCode(std::move(Err));
-      if (ec == Transport::kEOF)
-        break;
-      return llvm::errorCodeToError(ec);
+    std::optional<protocol::Message> next = transport.Read();
+    if (!next) {
+      break;
     }
 
     if (!HandleObject(*next)) {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 3254d0c1422cc..cc357158283dc 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -125,30 +125,30 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
                  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
                  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+  explicit SendEventRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
                  lldb::SBCommandReturnObject &result) override;
 };
 
 struct DAP {
-  llvm::StringRef client_name;
   llvm::StringRef debug_adapter_path;
   std::ofstream *log;
-  Transport transport;
+  llvm::StringRef client_name;
+  Transport &transport;
   lldb::SBFile in;
   OutputRedirector out;
   OutputRedirector err;
@@ -209,12 +209,33 @@ struct DAP {
   // will contain that expression.
   std::string last_nonempty_var_expression;
 
-  DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
-      lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
-      std::vector<std::string> pre_init_commands);
+  /// Creates a new DAP sessions.
+  ///
+  /// \param[in] path
+  ///     Path to the lldb-dap binary.
+  /// \param[in] log
+  ///     Log file stream, if configured.
+  /// \param[in] default_repl_mode
+  ///     Default repl mode behavior, as configured by the binary.
+  /// \param[in] pre_init_commands
+  ///     LLDB commands to execute as soon as the debugger instance is allocaed.
+  /// \param[in] client_name
+  ///     Debug session client name, for example 'stdin/stdout' or 'client_1'.
+  /// \param[in] transport
+  ///     Transport for this debug session.
+  DAP(llvm::StringRef path, std::ofstream *log,
+      const ReplMode default_repl_mode,
+      const std::vector<std::string> &pre_init_commands,
+      llvm::StringRef client_name, Transport &transport);
+
   ~DAP();
+
+  /// DAP is not copyable.
+  /// @{
   DAP(const DAP &rhs) = delete;
   void operator=(const DAP &rhs) = delete;
+  /// @}
+
   ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
   ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
 
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index f49bf373c4aff..fdfecf250fb16 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -7,16 +7,16 @@
 //===----------------------------------------------------------------------===//
 
 #include "Transport.h"
+#include "DAPLog.h"
 #include "Protocol.h"
-#include "c++/v1/__system_error/error_code.h"
 #include "lldb/Utility/IOObject.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
-#include <system_error>
 #include <utility>
 
 using namespace llvm;
@@ -28,18 +28,11 @@ using namespace lldb_dap::protocol;
 static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) {
   if (!descriptor || !descriptor->IsValid())
     return createStringError("transport input is closed");
-
   std::string data;
   data.resize(length);
-
   auto status = descriptor->Read(data.data(), length);
   if (status.Fail())
     return status.takeError();
-
-  // If we got back zero then we have reached EOF.
-  if (length == 0)
-    return createStringError(Transport::kEOF, "end-of-file");
-
   return data.substr(0, length);
 }
 
@@ -48,99 +41,101 @@ static Expected<std::string> ReadUntil(IOObjectSP &descriptor,
   std::string buffer;
   buffer.reserve(delimiter.size() + 1);
   while (!llvm::StringRef(buffer).ends_with(delimiter)) {
-    auto next = ReadFull(descriptor, 1);
+    Expected<std::string> next =
+        ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1);
     if (auto Err = next.takeError())
       return std::move(Err);
+    // '' is returned on EOF.
+    if (next->empty())
+      return buffer;
     buffer += *next;
   }
   return buffer.substr(0, buffer.size() - delimiter.size());
 }
 
-static Error ReadExpected(IOObjectSP &descriptor, StringRef want) {
-  auto got = ReadFull(descriptor, want.size());
-  if (auto Err = got.takeError())
-    return Err;
-  if (*got != want) {
-    return createStringError("want %s, got %s", want.str().c_str(),
-                             got->c_str());
-  }
-  return Error::success();
-}
+/// DAP message format
+/// ```
+/// Content-Length: (?<length>\d+)\r\n\r\n(?<content>.{\k<length>})
+/// ```
+static const StringLiteral kHeaderContentLength = "Content-Length: ";
+static const StringLiteral kHeaderSeparator = "\r\n\r\n";
 
 namespace lldb_dap {
 
-const std::error_code Transport::kEOF =
-    std::error_code(0x1001, std::generic_category());
-
-Transport::Transport(StringRef client_name, IOObjectSP input, IOObjectSP output)
-    : m_client_name(client_name), m_input(std::move(input)),
+Transport::Transport(StringRef client_name, std::ofstream *log,
+                     IOObjectSP input, IOObjectSP output)
+    : m_client_name(client_name), m_log(log), m_input(std::move(input)),
       m_output(std::move(output)) {}
 
-Expected<protocol::Message> Transport::Read(std::ofstream *log) {
-  // If we don't find the expected header we have reached EOF.
-  if (auto Err = ReadExpected(m_input, "Content-Length: "))
-    return std::move(Err);
+std::optional<Message> Transport::Read() {
+  Expected<std::string> message_header =
+      ReadFull(m_input, kHeaderContentLength.size());
+  if (!message_header) {
+    DAP_LOG_ERROR(m_log, message_header.takeError(), "({1}) read failed: {0}",
+                  m_client_name);
+    return std::nullopt;
+  }
 
-  auto rawLength = ReadUntil(m_input, "\r\n\r\n");
-  if (auto Err = rawLength.takeError())
-    return std::move(Err);
+  // '' returned on EOF.
+  if (message_header->empty())
+    return std::nullopt;
+  if (*message_header != kHeaderContentLength) {
+    DAP_LOG(m_log, "({0}) read failed: expected '{1}' and got '{2}'",
+            m_client_name, kHeaderContentLength, *message_header);
+    return std::nullopt;
+  }
+
+  Expected<std::string> raw_length = ReadUntil(m_input, kHeaderSeparator);
+  if (!raw_length) {
+    DAP_LOG_ERROR(m_log, raw_length.takeError(), "({1}) read failed: {0}",
+                  m_client_name);
+    return std::nullopt;
+  }
 
   size_t length;
-  if (!to_integer(*rawLength, length))
-    return createStringError("invalid content length %s", rawLength->c_str());
-
-  auto rawJSON = ReadFull(m_input, length);
-  if (auto Err = rawJSON.takeError())
-    return std::move(Err);
-  if (rawJSON->length() != length)
-    return createStringError(
-        "malformed request, expected %ld bytes, got %ld bytes", length,
-        rawJSON->length());
-
-  if (log) {
-    auto now = std::chrono::duration<double>(
-        std::chrono::system_clock::now().time_since_epoch());
-    *log << formatv("{0:f9} <-- ({1}) {2}\n", now.count(), m_client_name,
-                    *rawJSON)
-                .str();
+  if (!to_integer(*raw_length, length)) {
+    DAP_LOG(m_log, "({0}) read failed: invalid content length {1}",
+            m_client_name, *raw_length);
+    return std::nullopt;
   }
 
-  auto JSON = json::parse(*rawJSON);
-  if (auto Err = JSON.takeError()) {
-    return createStringError("malformed JSON %s\n%s", rawJSON->c_str(),
-                             llvm::toString(std::move(Err)).c_str());
+  Expected<std::string> raw_json = ReadFull(m_input, length);
+  if (!raw_json) {
+    DAP_LOG_ERROR(m_log, raw_json.takeError(), "({1}) read failed: {0}",
+                  m_client_name);
+    return std::nullopt;
+  }
+  if (raw_json->length() != length) {
+    DAP_LOG(m_log, "({0}) read failed: expected {1} bytes and got {2} bytes",
+            m_client_name, length, raw_json->length());
+    return std::nullopt;
   }
 
-  protocol::Message M;
-  llvm::json::Path::Root Root;
-  if (!fromJSON(*JSON, M, Root)) {
-    std::string error;
-    raw_string_ostream OS(error);
-    Root.printErrorContext(*JSON, OS);
-    return createStringError("malformed request: %s", error.c_str());
+  DAP_LOG(m_log, "<-- ({0}) {1}", m_client_name, *raw_json);
+
+  llvm::Expected<Message> message = json::parse<Message>(*raw_json);
+  if (!message) {
+    DAP_LOG_ERROR(m_log, message.takeError(), "({1}) read failed: {0}",
+                  m_client_name);
+    return std::nullopt;
   }
-  return std::move(M);
+
+  return std::move(*message);
 }
 
-lldb_private::Status Transport::Write(std::ofstream *log,
-                                      const protocol::Message &M) {
+Error Transport::Write(const Message &message) {
   if (!m_output || !m_output->IsValid())
-    return Status("transport output is closed");
+    return createStringError("transport output is closed");
 
-  std::string JSON = formatv("{0}", toJSON(M)).str();
+  std::string json = formatv("{0}", toJSON(message)).str();
 
-  if (log) {
-    auto now = std::chrono::duration<double>(
-        std::chrono::system_clock::now().time_since_epoch());
-    *log << formatv("{0:f9} --> ({1}) {2}\n", now.count(), m_client_name, JSON)
-                .str();
-  }
+  DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, json);
 
   std::string Output;
   raw_string_ostream OS(Output);
-  OS << "Content-Length: " << JSON.length() << "\r\n\r\n" << JSON;
+  OS << kHeaderContentLength << json.length() << kHeaderSeparator << json;
   size_t num_bytes = Output.size();
-  return m_output->Write(Output.data(), num_bytes);
+  return m_output->Write(Output.data(), num_bytes).takeError();
 }
 
 } // end namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 0150b762ceb10..a77f30f3aaf49 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -15,11 +15,11 @@
 #define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
 
 #include "Protocol.h"
-#include "lldb/Utility/Status.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include <fstream>
+#include <optional>
 
 namespace lldb_dap {
 
@@ -27,24 +27,25 @@ namespace lldb_dap {
 /// with the client.
 class Transport {
 public:
-  Transport(llvm::StringRef client_name, lldb::IOObjectSP input,
-            lldb::IOObjectSP output);
+  Transport(llvm::StringRef client_name, std::ofstream *log,
+            lldb::IOObjectSP input, lldb::IOObjectSP output);
   ~Transport() = default;
 
+  /// Transport is not copyable.
+  /// @{
   Transport(const Transport &rhs) = delete;
   void operator=(const Transport &rhs) = delete;
-
-  /// Error code returned this if EOF is encountered.
-  static const std::error_code kEOF;
+  /// @}
 
   /// Writes a Debug Adater Protocol message to the output stream.
-  lldb_private::Status Write(std::ofstream *log, const protocol::Message &M);
+  llvm::Error Write(const protocol::Message &M);
 
   /// Reads the next Debug Adater Protocol message from the input stream.
-  llvm::Expected<protocol::Message> Read(std::ofstream *log);
+  std::optional<protocol::Message> Read();
 
 private:
   llvm::StringRef m_client_name;
+  std::ofstream *m_log;
   lldb::IOObjectSP m_input;
   lldb::IOObjectSP m_output;
 };
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ab40b75e5425d..660bc9c046b6f 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -11,6 +11,7 @@
 #include "EventHelper.h"
 #include "Handler/RequestHandler.h"
 #include "RunInTerminal.h"
+#include "Transport.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/Host/Config.h"
@@ -325,8 +326,9 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
     std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex,
                         &dap_sessions]() {
       llvm::set_thread_name(client_name + ".runloop");
-      DAP dap = DAP(client_name, program_path, log, io, io, default_repl_mode,
-                    pre_init_commands);
+      Transport transport{client_name, log, io, io};
+      DAP dap = DAP(program_path, log, default_repl_mode, pre_init_commands,
+                    client_name, transport);
 
       if (auto Err = dap.ConfigureIO()) {
         llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
@@ -565,8 +567,10 @@ int main(int argc, char *argv[]) {
   lldb::IOObjectSP output = std::make_shared<NativeFile>(
       stdout_fd, File::eOpenOptionWriteOnly, false);
 
-  DAP dap = DAP("stdin/stdout", program_path, log.get(), std::move(input),
-                std::move(output), default_repl_mode, pre_init_commands);
+  std::string client_name = "stdin/stdout";
+  Transport transport{client_name, log.get(), input, output};
+  DAP dap = DAP(program_path, log.get(), default_repl_mode, pre_init_commands,
+                client_name, transport);
 
   // stdout/stderr redirection to the IDE's console
   if (auto Err = dap.ConfigureIO(stdout, stderr)) {

>From 0fd2160f790026f3a03c7322b71b04332bde7d47 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 10 Mar 2025 14:46:38 -0700
Subject: [PATCH 4/4] Adjusting the reader helpers to take
 `lldb_private::IOObject *` instead of the shared_ptr.

---
 lldb/tools/lldb-dap/Transport.cpp | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index fdfecf250fb16..c5c6651db1c35 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -25,9 +25,7 @@ using namespace lldb_private;
 using namespace lldb_dap;
 using namespace lldb_dap::protocol;
 
-static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) {
-  if (!descriptor || !descriptor->IsValid())
-    return createStringError("transport input is closed");
+static Expected<std::string> ReadFull(IOObject *descriptor, size_t length) {
   std::string data;
   data.resize(length);
   auto status = descriptor->Read(data.data(), length);
@@ -36,7 +34,7 @@ static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) {
   return data.substr(0, length);
 }
 
-static Expected<std::string> ReadUntil(IOObjectSP &descriptor,
+static Expected<std::string> ReadUntil(IOObject *descriptor,
                                        StringRef delimiter) {
   std::string buffer;
   buffer.reserve(delimiter.size() + 1);
@@ -68,8 +66,13 @@ Transport::Transport(StringRef client_name, std::ofstream *log,
       m_output(std::move(output)) {}
 
 std::optional<Message> Transport::Read() {
+  if (!m_input || !m_input->IsValid()) {
+    DAP_LOG(m_log, "({0}) input is closed", m_client_name);
+    return std::nullopt;
+  }
+  IOObject *input = m_input.get();
   Expected<std::string> message_header =
-      ReadFull(m_input, kHeaderContentLength.size());
+      ReadFull(input, kHeaderContentLength.size());
   if (!message_header) {
     DAP_LOG_ERROR(m_log, message_header.takeError(), "({1}) read failed: {0}",
                   m_client_name);
@@ -85,7 +88,7 @@ std::optional<Message> Transport::Read() {
     return std::nullopt;
   }
 
-  Expected<std::string> raw_length = ReadUntil(m_input, kHeaderSeparator);
+  Expected<std::string> raw_length = ReadUntil(input, kHeaderSeparator);
   if (!raw_length) {
     DAP_LOG_ERROR(m_log, raw_length.takeError(), "({1}) read failed: {0}",
                   m_client_name);
@@ -99,7 +102,7 @@ std::optional<Message> Transport::Read() {
     return std::nullopt;
   }
 
-  Expected<std::string> raw_json = ReadFull(m_input, length);
+  Expected<std::string> raw_json = ReadFull(input, length);
   if (!raw_json) {
     DAP_LOG_ERROR(m_log, raw_json.takeError(), "({1}) read failed: {0}",
                   m_client_name);



More information about the lldb-commits mailing list