https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/143806
>From 7fad13d3dba1974ece6f4e7da6fe4f48d7f3b4b0 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Wed, 11 Jun 2025 16:17:21 -0700 Subject: [PATCH 1/2] [lldb] Move Transport class into lldb_private (NFC) 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 --- lldb/include/lldb/Host/JSONTransport.h | 131 +++++++++++++++++++ lldb/source/Host/CMakeLists.txt | 3 +- lldb/source/Host/common/JSONTransport.cpp | 149 ++++++++++++++++++++++ lldb/tools/lldb-dap/DAP.cpp | 7 +- lldb/tools/lldb-dap/Transport.cpp | 144 +-------------------- lldb/tools/lldb-dap/Transport.h | 70 +--------- lldb/unittests/DAP/DAPTest.cpp | 7 +- lldb/unittests/DAP/TestBase.cpp | 3 +- lldb/unittests/DAP/TransportTest.cpp | 16 ++- 9 files changed, 313 insertions(+), 217 deletions(-) create mode 100644 lldb/include/lldb/Host/JSONTransport.h create mode 100644 lldb/source/Host/common/JSONTransport.cpp 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::FieldsAre( - /*event=*/"my-event", /*body=*/std::nullopt)))); + ASSERT_THAT_EXPECTED( + from_dap->Read<protocol::Message>(std::chrono::milliseconds(1)), + HasValue(testing::VariantWith<Event>(testing::FieldsAre( + /*event=*/"my-event", /*body=*/std::nullopt)))); } diff --git a/lldb/unittests/DAP/TestBase.cpp b/lldb/unittests/DAP/TestBase.cpp index 388d1b901507e..4063b34250312 100644 --- a/lldb/unittests/DAP/TestBase.cpp +++ b/lldb/unittests/DAP/TestBase.cpp @@ -122,7 +122,8 @@ std::vector<Message> DAPTestBase::DrainOutput() { std::vector<Message> msgs; output.CloseWriteFileDescriptor(); while (true) { - Expected<Message> next = from_dap->Read(std::chrono::milliseconds(1)); + Expected<Message> next = + from_dap->Read<protocol::Message>(std::chrono::milliseconds(1)); if (!next) { consumeError(next.takeError()); break; diff --git a/lldb/unittests/DAP/TransportTest.cpp b/lldb/unittests/DAP/TransportTest.cpp index e6dab42e30941..aaf257993af23 100644 --- a/lldb/unittests/DAP/TransportTest.cpp +++ b/lldb/unittests/DAP/TransportTest.cpp @@ -26,6 +26,8 @@ using namespace lldb_dap::protocol; using lldb_private::File; using lldb_private::NativeFile; using lldb_private::Pipe; +using lldb_private::TransportEOFError; +using lldb_private::TransportTimeoutError; class TransportTest : public PipeBase { protected: @@ -50,7 +52,7 @@ TEST_F(TransportTest, MalformedRequests) { input.Write(malformed_header.data(), malformed_header.size()), Succeeded()); ASSERT_THAT_EXPECTED( - transport->Read(std::chrono::milliseconds(1)), + transport->Read<protocol::Message>(std::chrono::milliseconds(1)), FailedWithMessage( "expected 'Content-Length: ' and got 'COnTent-LenGth: '")); } @@ -63,20 +65,22 @@ TEST_F(TransportTest, Read) { ASSERT_THAT_EXPECTED(input.Write(message.data(), message.size()), Succeeded()); ASSERT_THAT_EXPECTED( - transport->Read(std::chrono::milliseconds(1)), + transport->Read<protocol::Message>(std::chrono::milliseconds(1)), HasValue(testing::VariantWith<Request>(testing::FieldsAre( /*seq=*/1, /*command=*/"abc", /*arguments=*/std::nullopt)))); } TEST_F(TransportTest, ReadWithTimeout) { - ASSERT_THAT_EXPECTED(transport->Read(std::chrono::milliseconds(1)), - Failed<TimeoutError>()); + ASSERT_THAT_EXPECTED( + transport->Read<protocol::Message>(std::chrono::milliseconds(1)), + Failed<TransportTimeoutError>()); } TEST_F(TransportTest, ReadWithEOF) { input.CloseWriteFileDescriptor(); - ASSERT_THAT_EXPECTED(transport->Read(std::chrono::milliseconds(1)), - Failed<EndOfFileError>()); + ASSERT_THAT_EXPECTED( + transport->Read<protocol::Message>(std::chrono::milliseconds(1)), + Failed<TransportEOFError>()); } TEST_F(TransportTest, Write) { >From ef1dbae85322c4595da10992bf90165726645286 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Wed, 11 Jun 2025 18:08:20 -0700 Subject: [PATCH 2/2] Address John's feedback - Move client name out of the base class - Use HTTPDelimitedJSONTransport - Don't set the root name --- lldb/include/lldb/Host/JSONTransport.h | 25 +++++++++-------------- lldb/source/Host/common/JSONTransport.cpp | 14 ++++++------- lldb/tools/lldb-dap/Transport.cpp | 7 +++++-- lldb/tools/lldb-dap/Transport.h | 7 ++++++- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h index 907351abf8f74..4db5e417ea852 100644 --- a/lldb/include/lldb/Host/JSONTransport.h +++ b/lldb/include/lldb/Host/JSONTransport.h @@ -68,8 +68,7 @@ class TransportClosedError : public llvm::ErrorInfo<TransportClosedError> { /// A transport class that uses JSON for communication. class JSONTransport { public: - JSONTransport(llvm::StringRef client_name, lldb::IOObjectSP input, - lldb::IOObjectSP output); + JSONTransport(lldb::IOObjectSP input, lldb::IOObjectSP output); virtual ~JSONTransport() = default; /// Transport is not copyable. @@ -90,37 +89,33 @@ class JSONTransport { llvm::Expected<std::string> message = ReadImpl(timeout); if (!message) return message.takeError(); - return llvm::json::parse<T>(/*JSON=*/*message, - /*RootName=*/"transport_message"); + return llvm::json::parse<T>(/*JSON=*/*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 { +/// A transport class for JSON with a HTTP header. +class HTTPDelimitedJSONTransport : public JSONTransport { public: - JSONWithHeaderTransport(llvm::StringRef client_name, lldb::IOObjectSP input, - lldb::IOObjectSP output) - : JSONTransport(client_name, input, output) {} - virtual ~JSONWithHeaderTransport() = default; + HTTPDelimitedJSONTransport(lldb::IOObjectSP input, lldb::IOObjectSP output) + : JSONTransport(input, output) {} + virtual ~HTTPDelimitedJSONTransport() = default; +protected: virtual llvm::Error WriteImpl(const std::string &message) override; virtual llvm::Expected<std::string> ReadImpl(const std::chrono::microseconds &timeout) override; + // FIXME: Support any header. static constexpr llvm::StringLiteral kHeaderContentLength = "Content-Length: "; static constexpr llvm::StringLiteral kHeaderSeparator = "\r\n\r\n"; diff --git a/lldb/source/Host/common/JSONTransport.cpp b/lldb/source/Host/common/JSONTransport.cpp index 32d1889f69c02..103c76d25daf7 100644 --- a/lldb/source/Host/common/JSONTransport.cpp +++ b/lldb/source/Host/common/JSONTransport.cpp @@ -82,17 +82,15 @@ ReadUntil(IOObject &descriptor, StringRef delimiter, 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)) {} +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> -JSONWithHeaderTransport::ReadImpl(const std::chrono::microseconds &timeout) { +HTTPDelimitedJSONTransport::ReadImpl(const std::chrono::microseconds &timeout) { if (!m_input || !m_input->IsValid()) return createStringError("transport output is closed"); @@ -126,16 +124,16 @@ JSONWithHeaderTransport::ReadImpl(const std::chrono::microseconds &timeout) { return createStringError("unexpected EOF while reading JSON"); }); - Log(llvm::formatv("--> ({0}) {1}", m_client_name, *raw_json).str()); + Log(llvm::formatv("--> {0}", *raw_json).str()); return raw_json; } -Error JSONWithHeaderTransport::WriteImpl(const std::string &message) { +Error HTTPDelimitedJSONTransport::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()); + Log(llvm::formatv("<-- {0}", message).str()); std::string Output; raw_string_ostream OS(Output); diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp index 90fc895badc25..d602920da34e3 100644 --- a/lldb/tools/lldb-dap/Transport.cpp +++ b/lldb/tools/lldb-dap/Transport.cpp @@ -18,6 +18,9 @@ using namespace lldb_dap; Transport::Transport(llvm::StringRef client_name, lldb_dap::Log *log, lldb::IOObjectSP input, lldb::IOObjectSP output) - : JSONWithHeaderTransport(client_name, input, output), m_log(log) {} + : HTTPDelimitedJSONTransport(input, output), m_client_name(client_name), + m_log(log) {} -void Transport::Log(llvm::StringRef message) { DAP_LOG(m_log, "{0}", message); } +void Transport::Log(llvm::StringRef message) { + DAP_LOG(m_log, "({0}) {1}", m_client_name, message); +} diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h index 0b083dd90dcd0..51f62e718a0d0 100644 --- a/lldb/tools/lldb-dap/Transport.h +++ b/lldb/tools/lldb-dap/Transport.h @@ -23,7 +23,7 @@ namespace lldb_dap { /// A transport class that performs the Debug Adapter Protocol communication /// with the client. -class Transport : public lldb_private::JSONWithHeaderTransport { +class Transport : public lldb_private::HTTPDelimitedJSONTransport { public: Transport(llvm::StringRef client_name, lldb_dap::Log *log, lldb::IOObjectSP input, lldb::IOObjectSP output); @@ -31,7 +31,12 @@ class Transport : public lldb_private::JSONWithHeaderTransport { virtual void Log(llvm::StringRef message) override; + /// Returns the name of this transport client, for example `stdin/stdout` or + /// `client_1`. + llvm::StringRef GetClientName() { return m_client_name; } + private: + llvm::StringRef m_client_name; lldb_dap::Log *m_log; }; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
