[Lldb-commits] [lldb] [lldb] Update JSONTransport to use MainLoop for reading. (PR #148300)

[Pavel Labath via lldb-commits]

================
@@ -7,163 +7,122 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/JSONTransport.h"
-#include "lldb/Utility/IOObject.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
-#include "lldb/Utility/SelectHelper.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 <optional>
 #include <string>
 #include <utility>
 
 using namespace llvm;
 using namespace lldb;
 using namespace lldb_private;
 
-/// ReadFull attempts to read the specified number of bytes. If EOF is
-/// encountered, an empty string is returned.
-static Expected<std::string>
-ReadFull(IOObject &descriptor, size_t length,
-         std::optional<std::chrono::microseconds> timeout = std::nullopt) {
-  if (!descriptor.IsValid())
-    return llvm::make_error<TransportInvalidError>();
-
-  bool timeout_supported = true;
-  // FIXME: SelectHelper does not work with NativeFile on Win32.
-#if _WIN32
-  timeout_supported = descriptor.GetFdType() == IOObject::eFDTypeSocket;
-#endif
-
-  if (timeout && timeout_supported) {
-    SelectHelper sh;
-    sh.SetTimeout(*timeout);
-    sh.FDSetRead(
-        reinterpret_cast<lldb::socket_t>(descriptor.GetWaitableHandle()));
-    Status status = sh.Select();
-    if (status.Fail()) {
-      // Convert timeouts into a specific error.
-      if (status.GetType() == lldb::eErrorTypePOSIX &&
-          status.GetError() == ETIMEDOUT)
-        return make_error<TransportTimeoutError>();
-      return status.takeError();
-    }
-  }
-
-  std::string data;
-  data.resize(length);
-  Status status = descriptor.Read(data.data(), length);
-  if (status.Fail())
-    return status.takeError();
-
-  // Read returns '' on EOF.
-  if (length == 0)
-    return make_error<TransportEOFError>();
-
-  // Return the actual number of bytes read.
-  return data.substr(0, length);
-}
-
-static Expected<std::string>
-ReadUntil(IOObject &descriptor, StringRef delimiter,
-          std::optional<std::chrono::microseconds> timeout = std::nullopt) {
-  std::string buffer;
-  buffer.reserve(delimiter.size() + 1);
-  while (!llvm::StringRef(buffer).ends_with(delimiter)) {
-    Expected<std::string> next =
-        ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1, timeout);
-    if (auto Err = next.takeError())
-      return std::move(Err);
-    buffer += *next;
-  }
-  return buffer.substr(0, buffer.size() - delimiter.size());
-}
-
 JSONTransport::JSONTransport(IOObjectSP input, IOObjectSP output)
     : m_input(std::move(input)), m_output(std::move(output)) {}
 
 void JSONTransport::Log(llvm::StringRef message) {
   LLDB_LOG(GetLog(LLDBLog::Host), "{0}", message);
 }
 
-Expected<std::string>
-HTTPDelimitedJSONTransport::ReadImpl(const std::chrono::microseconds &timeout) {
-  if (!m_input || !m_input->IsValid())
-    return llvm::make_error<TransportInvalidError>();
+// Parses messages based on
+// https://microsoft.github.io/debug-adapter-protocol/overview#base-protocol
+Expected<std::vector<std::string>> HTTPDelimitedJSONTransport::Parse() {
+  if (m_buffer.empty())
+    return std::vector<std::string>{};
+
+  std::vector<std::string> messages;
+  StringRef buffer = m_buffer;
+  size_t content_length = 0, end_of_last_message = 0, cursor = 0;
+  do {
+    auto idx = buffer.find(kHeaderSeparator, cursor);
+    // Separator not found, we need more data.
+    if (idx == StringRef::npos)
+      break;
+
+    auto header = buffer.slice(cursor, idx);
+    cursor = idx + kHeaderSeparator.size();
+
+    // An empty line separates the headers from the message body.
+    if (header.empty()) {
+      // Check if we have enough data or wait for the next chunk to arrive.
+      if (content_length + cursor > buffer.size())
+        break;
+
+      std::string body = buffer.substr(cursor, content_length).str();
+      end_of_last_message = cursor + content_length;
+      cursor += content_length;
+      Logv("--> {0}", body);
+      messages.emplace_back(std::move(body));
+      content_length = 0;
+      continue;
+    }
+
+    // HTTP Headers are formatted like `<field-name> ':' [<field-value>]`.
+    if (!header.contains(kHeaderFieldSeparator))
+      return createStringError("malformed content header");
+
+    auto [name, value] = header.split(kHeaderFieldSeparator);
+
+    // Handle known headers, at the moment only "Content-Length" is specified,
+    // other headers are ignored.
+    if (name.lower() == kHeaderContentLength.lower()) {
+      value = value.trim();
+      if (value.trim().consumeInteger(10, content_length))
----------------
labath wrote:

```suggestion
      if (!llvm::to_integer(value, content_length))
```

optional, but I find it cleaner. Just making sure you're aware this exists.

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