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

[John Harrison via lldb-commits]

================
@@ -54,112 +50,220 @@ class TransportUnhandledContentsError
   std::string m_unhandled_contents;
 };
 
-class TransportInvalidError : public llvm::ErrorInfo<TransportInvalidError> {
+/// A transport is responsible for maintaining the connection to a client
+/// application, and reading/writing structured messages to it.
+///
+/// Transports have limited thread safety requirements:
+///  - Messages will not be sent concurrently.
+///  - Messages MAY be sent while Run() is reading, or its callback is active.
+template <typename Req, typename Resp, typename Evt> class Transport {
 public:
-  static char ID;
-
-  TransportInvalidError() = default;
+  using Message = std::variant<Req, Resp, Evt>;
+
+  virtual ~Transport() = default;
+
+  // Called by transport to send outgoing messages.
+  virtual void Event(const Evt &) = 0;
+  virtual void Request(const Req &) = 0;
+  virtual void Response(const Resp &) = 0;
+
+  /// Implemented to handle incoming messages. (See Run() below).
+  class MessageHandler {
+  public:
+    virtual ~MessageHandler() = default;
+    virtual void OnEvent(const Evt &) = 0;
+    virtual void OnRequest(const Req &) = 0;
+    virtual void OnResponse(const Resp &) = 0;
+  };
+
+  /// Called by server or client to receive messages from the connection.
+  /// The transport should in turn invoke the handler to process messages.
+  /// The MainLoop is used to handle reading from the incoming connection and
+  /// will run until the loop is terminated.
+  virtual llvm::Error Run(MainLoop &, MessageHandler &) = 0;
 
-  void log(llvm::raw_ostream &OS) const override;
-  std::error_code convertToErrorCode() const override;
+protected:
+  template <typename... Ts> inline auto Logv(const char *Fmt, Ts &&...Vals) {
+    Log(llvm::formatv(Fmt, std::forward<Ts>(Vals)...).str());
+  }
+  virtual void Log(llvm::StringRef message) = 0;
 };
 
-/// A transport class that uses JSON for communication.
-class JSONTransport {
+/// A JSONTransport will encode and decode messages using JSON.
+template <typename Req, typename Resp, typename Evt>
+class JSONTransport : public Transport<Req, Resp, Evt> {
 public:
-  using ReadHandleUP = MainLoopBase::ReadHandleUP;
-  template <typename T>
-  using Callback = std::function<void(MainLoopBase &, const llvm::Expected<T>)>;
-
-  JSONTransport(lldb::IOObjectSP input, lldb::IOObjectSP output);
-  virtual ~JSONTransport() = default;
-
-  /// Transport is not copyable.
-  /// @{
-  JSONTransport(const JSONTransport &rhs) = delete;
-  void operator=(const JSONTransport &rhs) = delete;
-  /// @}
-
-  /// Writes a message to the output stream.
-  template <typename T> llvm::Error Write(const T &t) {
-    const std::string message = llvm::formatv("{0}", toJSON(t)).str();
-    return WriteImpl(message);
+  using Transport<Req, Resp, Evt>::Transport;
+
+  JSONTransport(lldb::IOObjectSP in, lldb::IOObjectSP out)
+      : m_in(in), m_out(out) {}
+
+  void Event(const Evt &evt) override { Write(evt); }
+  void Request(const Req &req) override { Write(req); }
+  void Response(const Resp &resp) override { Write(resp); }
+
+  /// Run registers the transport with the given MainLoop and handles any
+  /// incoming messages using the given MessageHandler.
+  llvm::Error
+  Run(MainLoop &loop,
+      typename Transport<Req, Resp, Evt>::MessageHandler &handler) override {
+    llvm::Error error = llvm::Error::success();
+    Status status;
+    auto read_handle = loop.RegisterReadObject(
+        m_in,
+        std::bind(&JSONTransport::OnRead, this, &error, std::placeholders::_1,
+                  std::ref(handler)),
+        status);
+    if (status.Fail()) {
+      // This error is only set if the read object handler is invoked, mark it
+      // as consumed if registration of the handler failed.
+      llvm::consumeError(std::move(error));
+      return status.takeError();
+    }
+
+    status = loop.Run();
+    if (status.Fail())
+      return status.takeError();
+    return error;
   }
 
-  /// Registers the transport with the MainLoop.
-  template <typename T>
-  llvm::Expected<ReadHandleUP> RegisterReadObject(MainLoopBase &loop,
-                                                  Callback<T> read_cb) {
-    Status error;
-    ReadHandleUP handle = loop.RegisterReadObject(
-        m_input,
-        [read_cb, this](MainLoopBase &loop) {
-          char buf[kReadBufferSize];
-          size_t num_bytes = sizeof(buf);
-          if (llvm::Error error = m_input->Read(buf, num_bytes).takeError()) {
-            read_cb(loop, std::move(error));
-            return;
-          }
-          if (num_bytes)
-            m_buffer.append(std::string(buf, num_bytes));
-
-          // If the buffer has contents, try parsing any pending messages.
-          if (!m_buffer.empty()) {
-            llvm::Expected<std::vector<std::string>> messages = Parse();
-            if (llvm::Error error = messages.takeError()) {
-              read_cb(loop, std::move(error));
-              return;
-            }
-
-            for (const auto &message : *messages)
-              if constexpr (std::is_same<T, std::string>::value)
-                read_cb(loop, message);
-              else
-                read_cb(loop, llvm::json::parse<T>(message));
-          }
-
-          // On EOF, notify the callback after the remaining messages were
-          // handled.
-          if (num_bytes == 0) {
-            if (m_buffer.empty())
-              read_cb(loop, llvm::make_error<TransportEOFError>());
-            else
-              read_cb(loop, llvm::make_error<TransportUnhandledContentsError>(
-                                std::string(m_buffer)));
-          }
-        },
-        error);
-    if (error.Fail())
-      return error.takeError();
-    return handle;
-  }
+  /// Public for testing purposes, otherwise this should be an implementation
+  /// detail.
+  static constexpr size_t kReadBufferSize = 1024;
 
 protected:
-  template <typename... Ts> inline auto Logv(const char *Fmt, Ts &&...Vals) {
-    Log(llvm::formatv(Fmt, std::forward<Ts>(Vals)...).str());
+  virtual llvm::Expected<std::vector<std::string>> Parse() = 0;
+  virtual std::string Encode(const llvm::json::Value &message) = 0;
+  void Write(const llvm::json::Value &message) {
+    this->Logv("<-- {0}", message);
+    std::string output = Encode(message);
+    size_t bytes_written = output.size();
+    Status status = m_out->Write(output.data(), bytes_written);
+    if (status.Fail()) {
+      this->Logv("writing failed: s{0}", status.AsCString());
+    }
   }
-  virtual void Log(llvm::StringRef message);
 
-  virtual llvm::Error WriteImpl(const std::string &message) = 0;
-  virtual llvm::Expected<std::vector<std::string>> Parse() = 0;
+  llvm::SmallString<kReadBufferSize> m_buffer;
 
-  static constexpr size_t kReadBufferSize = 1024;
+private:
+  void OnRead(llvm::Error *err, MainLoopBase &loop,
+              typename Transport<Req, Resp, Evt>::MessageHandler &handler) {
+    llvm::ErrorAsOutParameter ErrAsOutParam(err);
+    char buf[kReadBufferSize];
+    size_t num_bytes = sizeof(buf);
+    if (Status status = m_in->Read(buf, num_bytes); status.Fail()) {
+      *err = status.takeError();
+      loop.RequestTermination();
+      return;
+    }
+
+    if (num_bytes)
+      m_buffer.append(llvm::StringRef(buf, num_bytes));
+
+    // If the buffer has contents, try parsing any pending messages.
+    if (!m_buffer.empty()) {
+      llvm::Expected<std::vector<std::string>> raw_messages = Parse();
+      if (llvm::Error error = raw_messages.takeError()) {
+        *err = std::move(error);
+        loop.RequestTermination();
+        return;
+      }
+
+      for (const auto &raw_message : *raw_messages) {
----------------
ashgti wrote:

Done.

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