-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb] Move Transport class into lldb_private (NFC) #143806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Move lldb-dap's Transport class into lldb_private so the code can be shared between the "JSON with header" protocol used by DAP and the JSON RPC protocol used by MCP (see [1]). [1]: https://discourse.llvm.org/t/rfc-adding-mcp-support-to-lldb/86798
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesMove lldb-dap's Transport class into lldb_private so the code can be shared between the "JSON with header" protocol used by DAP and the JSON RPC protocol used by MCP (see 1). Patch is 22.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143806.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h
new file mode 100644
index 0000000000000..907351abf8f74
--- /dev/null
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -0,0 +1,131 @@
+//===-- JSONTransport.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
+//
+//===----------------------------------------------------------------------===//
+//
+// Transport layer for encoding and decoding JSON protocol messages.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_JSONTRANSPORT_H
+#define LLDB_HOST_JSONTRANSPORT_H
+
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
+#include <chrono>
+#include <system_error>
+
+namespace lldb_private {
+
+class TransportEOFError : public llvm::ErrorInfo<TransportEOFError> {
+public:
+ static char ID;
+
+ TransportEOFError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "transport end of file reached";
+ }
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+class TransportTimeoutError : public llvm::ErrorInfo<TransportTimeoutError> {
+public:
+ static char ID;
+
+ TransportTimeoutError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "transport operation timed out";
+ }
+ std::error_code convertToErrorCode() const override {
+ return std::make_error_code(std::errc::timed_out);
+ }
+};
+
+class TransportClosedError : public llvm::ErrorInfo<TransportClosedError> {
+public:
+ static char ID;
+
+ TransportClosedError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "transport is closed";
+ }
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+/// A transport class that uses JSON for communication.
+class JSONTransport {
+public:
+ JSONTransport(llvm::StringRef client_name, 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);
+ }
+
+ /// Reads the next message from the input stream.
+ template <typename T>
+ llvm::Expected<T> Read(const std::chrono::microseconds &timeout) {
+ llvm::Expected<std::string> message = ReadImpl(timeout);
+ if (!message)
+ return message.takeError();
+ return llvm::json::parse<T>(/*JSON=*/*message,
+ /*RootName=*/"transport_message");
+ }
+
+ /// Returns the name of this transport client, for example `stdin/stdout` or
+ /// `client_1`.
+ llvm::StringRef GetClientName() { return m_client_name; }
+
+protected:
+ virtual void Log(llvm::StringRef message);
+ virtual llvm::Error WriteImpl(const std::string &message) = 0;
+ virtual llvm::Expected<std::string>
+ ReadImpl(const std::chrono::microseconds &timeout) = 0;
+
+ llvm::StringRef m_client_name;
+ lldb::IOObjectSP m_input;
+ lldb::IOObjectSP m_output;
+};
+
+/// A transport class that uses JSON with a header for communication.
+class JSONWithHeaderTransport : public JSONTransport {
+public:
+ JSONWithHeaderTransport(llvm::StringRef client_name, lldb::IOObjectSP input,
+ lldb::IOObjectSP output)
+ : JSONTransport(client_name, input, output) {}
+ virtual ~JSONWithHeaderTransport() = default;
+
+ virtual llvm::Error WriteImpl(const std::string &message) override;
+ virtual llvm::Expected<std::string>
+ ReadImpl(const std::chrono::microseconds &timeout) override;
+
+ static constexpr llvm::StringLiteral kHeaderContentLength =
+ "Content-Length: ";
+ static constexpr llvm::StringLiteral kHeaderSeparator = "\r\n\r\n";
+};
+
+} // namespace lldb_private
+
+#endif
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index 5b713133afeaf..b15d72e61b6e5 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -27,8 +27,9 @@ add_host_subdirectory(common
common/HostNativeThreadBase.cpp
common/HostProcess.cpp
common/HostThread.cpp
- common/LockFileBase.cpp
+ common/JSONTransport.cpp
common/LZMA.cpp
+ common/LockFileBase.cpp
common/MainLoopBase.cpp
common/MemoryMonitor.cpp
common/MonitoringProcessLauncher.cpp
diff --git a/lldb/source/Host/common/JSONTransport.cpp b/lldb/source/Host/common/JSONTransport.cpp
new file mode 100644
index 0000000000000..32d1889f69c02
--- /dev/null
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -0,0 +1,149 @@
+//===-- JSONTransport.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 "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<TransportClosedError>();
+
+ 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(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(StringRef client_name, IOObjectSP input,
+ IOObjectSP output)
+ : m_client_name(client_name), 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>
+JSONWithHeaderTransport::ReadImpl(const std::chrono::microseconds &timeout) {
+ if (!m_input || !m_input->IsValid())
+ return createStringError("transport output is closed");
+
+ IOObject *input = m_input.get();
+ Expected<std::string> message_header =
+ ReadFull(*input, kHeaderContentLength.size(), timeout);
+ if (!message_header)
+ return message_header.takeError();
+ if (*message_header != kHeaderContentLength)
+ return createStringError(formatv("expected '{0}' and got '{1}'",
+ kHeaderContentLength, *message_header)
+ .str());
+
+ Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
+ if (!raw_length)
+ return handleErrors(raw_length.takeError(),
+ [&](const TransportEOFError &E) -> llvm::Error {
+ return createStringError(
+ "unexpected EOF while reading header separator");
+ });
+
+ size_t length;
+ if (!to_integer(*raw_length, length))
+ return createStringError(
+ formatv("invalid content length {0}", *raw_length).str());
+
+ Expected<std::string> raw_json = ReadFull(*input, length);
+ if (!raw_json)
+ return handleErrors(
+ raw_json.takeError(), [&](const TransportEOFError &E) -> llvm::Error {
+ return createStringError("unexpected EOF while reading JSON");
+ });
+
+ Log(llvm::formatv("--> ({0}) {1}", m_client_name, *raw_json).str());
+
+ return raw_json;
+}
+
+Error JSONWithHeaderTransport::WriteImpl(const std::string &message) {
+ if (!m_output || !m_output->IsValid())
+ return llvm::make_error<TransportClosedError>();
+
+ Log(llvm::formatv("<-- ({0}) {1}", m_client_name, message).str());
+
+ std::string Output;
+ raw_string_ostream OS(Output);
+ OS << kHeaderContentLength << message.length() << kHeaderSeparator << message;
+ size_t num_bytes = Output.size();
+ return m_output->Write(Output.data(), num_bytes).takeError();
+}
+
+char TransportEOFError::ID;
+char TransportTimeoutError::ID;
+char TransportClosedError::ID;
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index b034c967594ba..9fe8227cd2d6f 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -70,6 +70,7 @@
using namespace lldb_dap;
using namespace lldb_dap::protocol;
+using namespace lldb_private;
namespace {
#ifdef _WIN32
@@ -893,14 +894,14 @@ llvm::Error DAP::Loop() {
while (!disconnecting) {
llvm::Expected<Message> next =
- transport.Read(std::chrono::seconds(1));
- if (next.errorIsA<EndOfFileError>()) {
+ transport.Read<protocol::Message>(std::chrono::seconds(1));
+ if (next.errorIsA<TransportEOFError>()) {
consumeError(next.takeError());
break;
}
// If the read timed out, continue to check if we should disconnect.
- if (next.errorIsA<TimeoutError>()) {
+ if (next.errorIsA<TransportTimeoutError>()) {
consumeError(next.takeError());
continue;
}
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 4e322e9ff1358..90fc895badc25 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -8,152 +8,16 @@
#include "Transport.h"
#include "DAPLog.h"
-#include "Protocol/ProtocolBase.h"
-#include "lldb/Utility/IOObject.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;
using namespace lldb_dap;
-using namespace lldb_dap::protocol;
-/// 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 createStringError("transport output is closed");
+Transport::Transport(llvm::StringRef client_name, lldb_dap::Log *log,
+ lldb::IOObjectSP input, lldb::IOObjectSP output)
+ : JSONWithHeaderTransport(client_name, input, output), m_log(log) {}
- 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(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<TimeoutError>();
- 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<EndOfFileError>();
-
- // 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());
-}
-
-/// DAP message format
-/// ```
-/// Content-Length: (?<length>\d+)\r\n\r\n(?<content>.{\k<length>})
-/// ```
-static constexpr StringLiteral kHeaderContentLength = "Content-Length: ";
-static constexpr StringLiteral kHeaderSeparator = "\r\n\r\n";
-
-namespace lldb_dap {
-
-char EndOfFileError::ID;
-char TimeoutError::ID;
-
-Transport::Transport(StringRef client_name, Log *log, IOObjectSP input,
- IOObjectSP output)
- : m_client_name(client_name), m_log(log), m_input(std::move(input)),
- m_output(std::move(output)) {}
-
-Expected<Message> Transport::Read(const std::chrono::microseconds &timeout) {
- if (!m_input || !m_input->IsValid())
- return createStringError("transport output is closed");
-
- IOObject *input = m_input.get();
- Expected<std::string> message_header =
- ReadFull(*input, kHeaderContentLength.size(), timeout);
- if (!message_header)
- return message_header.takeError();
- if (*message_header != kHeaderContentLength)
- return createStringError(formatv("expected '{0}' and got '{1}'",
- kHeaderContentLength, *message_header)
- .str());
-
- Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
- if (!raw_length)
- return handleErrors(raw_length.takeError(),
- [&](const EndOfFileError &E) -> llvm::Error {
- return createStringError(
- "unexpected EOF while reading header separator");
- });
-
- size_t length;
- if (!to_integer(*raw_length, length))
- return createStringError(
- formatv("invalid content length {0}", *raw_length).str());
-
- Expected<std::string> raw_json = ReadFull(*input, length);
- if (!raw_json)
- return handleErrors(
- raw_json.takeError(), [&](const EndOfFileError &E) -> llvm::Error {
- return createStringError("unexpected EOF while reading JSON");
- });
-
- DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, *raw_json);
-
- return json::parse<Message>(/*JSON=*/*raw_json,
- /*RootName=*/"protocol_message");
-}
-
-Error Transport::Write(const Message &message) {
- if (!m_output || !m_output->IsValid())
- return createStringError("transport output is closed");
-
- std::string json = formatv("{0}", toJSON(message)).str();
-
- DAP_LOG(m_log, "<-- ({0}) {1}", m_client_name, json);
-
- std::string Output;
- raw_string_ostream OS(Output);
- OS << kHeaderContentLength << json.length() << kHeaderSeparator << json;
- size_t num_bytes = Output.size();
- return m_output->Write(Output.data(), num_bytes).takeError();
-}
-
-} // end namespace lldb_dap
+void Transport::Log(llvm::StringRef message) { DAP_LOG(m_log, "{0}", message); }
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 4e347eaa51314..0b083dd90dcd0 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -15,80 +15,24 @@
#define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
#include "DAPForward.h"
-#include "Protocol/ProtocolBase.h"
+#include "lldb/Host/JSONTransport.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Error.h"
-#include <chrono>
-#include <system_error>
namespace lldb_dap {
-class EndOfFileError : public llvm::ErrorInfo<EndOfFileError> {
-public:
- static char ID;
-
- EndOfFileError() = default;
-
- void log(llvm::raw_ostream &OS) const override {
- OS << "end of file reached";
- }
- std::error_code convertToErrorCode() const override {
- return llvm::inconvertibleErrorCode();
- }
-};
-
-class TimeoutError : public llvm::ErrorInfo<TimeoutError> {
-public:
- static char ID;
-
- TimeoutError() = default;
-
- void log(llvm::raw_ostream &OS) const override {
- OS << "operation timed out";
- }
- std::error_code convertToErrorCode() const override {
- return std::make_error_code(std::errc::timed_out);
- }
-};
-
/// A transport class that performs the Debug Adapter Protocol communication
/// with the client.
-class Transport {
+class Transport : public lldb_private::JSONWithHeaderTransport {
public:
- Transport(llvm::StringRef client_name, Log *log, lldb::IOObjectSP input,
- lldb::IOObjectSP output);
- ~Transport() = default;
-
- /// Transport is not copyable.
- /// @{
- Transport(const Transport &rhs) = delete;
- void operator=(const Transport &rhs) = delete;
- /// @}
-
- /// Writes a Debug Adater Protocol message to the output stream.
- llvm::Error Write(const protocol::Message &M);
-
- /// Reads the next Debug Adater Protocol message from the input stream.
- ///
- /// \param timeout[in]
- /// A timeout to wait for reading the initial header. Once a message
- /// header is recieved, this will block until the full message is
- /// read.
- ///
- /// \returns Returns the next protocol message.
- llvm::Expected<protocol::Message>
- Read(const std::chrono::microseconds &timeout);
+ Transport(llvm::StringRef client_name, lldb_dap::Log *log,
+ lldb::IOObjectSP input, lldb::IOObjectSP output);
+ virtual ~Transport() = default;
- /// Returns the name of this transport client, for example `stdin/stdout` or
- /// `client_1`.
- llvm::StringRef GetClientName() { return m_client_name; }
+ virtual void Log(llvm::StringRef message) override;
private:
- llvm::StringRef m_client_name;
- Log *m_log;
- lldb::IOObjectSP m_input;
- lldb::IOObjectSP m_output;
+ lldb_dap::Log *m_log;
};
} // namespace lldb_dap
diff --git a/lldb/unittests/DAP/DAPTest.cpp b/lldb/unittests/DAP/DAPTest.cpp
index 5fb6bf7e564ab..40ffaf87c9c45 100644
--- a/lldb/unittests/DAP/DAPTest.cpp
+++ b/lldb/unittests/DAP/DAPTest.cpp
@@ -32,7 +32,8 @@ TEST_F(DAPTest, SendProtocolMessages) {
/*transport=*/*to_dap,
};
dap.Send(Event{/*event=*/"my-event", /*body=*/std::nullopt});
- ASSERT_THAT_EXPECTED(from_dap->Read(std::chrono::milliseconds(1)),
- HasValue(testing::VariantWith<Event>(testing:...
[truncated]
|
@ashgti Let me know if you think it's too early to land this change. Given the popularity of JSON RPC, I think this would be generally useful, but let me know if you'd like me to hold off until the RFC has converged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
- Move client name out of the base class - Use HTTPDelimitedJSONTransport - Don't set the root name
Move lldb-dap's Transport class into lldb_private so the code can be shared between the "JSON with header" protocol used by DAP and the JSON RPC protocol used by MCP (see [1]). [1]: https://discourse.llvm.org/t/rfc-adding-mcp-support-to-lldb/86798
Move lldb-dap's Transport class into lldb_private so the code can be shared between the "JSON with header" protocol used by DAP and the JSON RPC protocol used by MCP (see 1).