Skip to content

[lldb] Implement JSON RPC (newline delimited) Transport #143946

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 12, 2025
Merged

[lldb] Implement JSON RPC (newline delimited) Transport #143946

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ebb398
[lldb] Implement JSON RPC (newline delimited) Transport
JDevlieghere Jun 12, 2025
5e62a8a
Rename error, add more tests
JDevlieghere Jun 12, 2025
e48ac82
Guard timeout tests on Windows
JDevlieghere Jun 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lldb/include/lldb/Host/JSONTransport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,21 @@ class HTTPDelimitedJSONTransport : public JSONTransport {
static constexpr llvm::StringLiteral kHeaderSeparator = "\r\n\r\n";
};

/// A transport class for JSON RPC.
class JSONRPCTransport : public JSONTransport {
public:
JSONRPCTransport(lldb::IOObjectSP input, lldb::IOObjectSP output)
: JSONTransport(input, output) {}
virtual ~JSONRPCTransport() = default;

protected:
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 kMessageSeparator = "\n";
};

} // namespace lldb_private

#endif
31 changes: 30 additions & 1 deletion lldb/source/Host/common/JSONTransport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void JSONTransport::Log(llvm::StringRef message) {
Expected<std::string>
HTTPDelimitedJSONTransport::ReadImpl(const std::chrono::microseconds &timeout) {
if (!m_input || !m_input->IsValid())
return createStringError("transport output is closed");
return llvm::make_error<TransportClosedError>();

IOObject *input = m_input.get();
Expected<std::string> message_header =
Expand Down Expand Up @@ -142,6 +142,35 @@ Error HTTPDelimitedJSONTransport::WriteImpl(const std::string &message) {
return m_output->Write(Output.data(), num_bytes).takeError();
}

Expected<std::string>
JSONRPCTransport::ReadImpl(const std::chrono::microseconds &timeout) {
if (!m_input || !m_input->IsValid())
return make_error<TransportClosedError>();

IOObject *input = m_input.get();
Expected<std::string> raw_json =
ReadUntil(*input, kMessageSeparator, timeout);
if (!raw_json)
return raw_json.takeError();

Log(llvm::formatv("--> {0}", *raw_json).str());

return *raw_json;
}

Error JSONRPCTransport::WriteImpl(const std::string &message) {
if (!m_output || !m_output->IsValid())
return llvm::make_error<TransportClosedError>();

Log(llvm::formatv("<-- {0}", message).str());

std::string Output;
llvm::raw_string_ostream OS(Output);
OS << message << kMessageSeparator;
size_t num_bytes = Output.size();
return m_output->Write(Output.data(), num_bytes).takeError();
}

char TransportEOFError::ID;
char TransportTimeoutError::ID;
char TransportClosedError::ID;
1 change: 0 additions & 1 deletion lldb/unittests/DAP/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ add_lldb_unittest(DAPTests
LLDBUtilsTest.cpp
ProtocolTypesTest.cpp
TestBase.cpp
TransportTest.cpp
VariablesTest.cpp

LINK_COMPONENTS
Expand Down
7 changes: 1 addition & 6 deletions lldb/unittests/DAP/TestBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,8 @@ using lldb_private::File;
using lldb_private::NativeFile;
using lldb_private::Pipe;

void PipeBase::SetUp() {
ASSERT_THAT_ERROR(input.CreateNew(false).ToError(), Succeeded());
ASSERT_THAT_ERROR(output.CreateNew(false).ToError(), Succeeded());
}

void TransportBase::SetUp() {
PipeBase::SetUp();
PipeTest::SetUp();
to_dap = std::make_unique<Transport>(
"to_dap", nullptr,
std::make_shared<NativeFile>(input.GetReadFileDescriptor(),
Expand Down
13 changes: 2 additions & 11 deletions lldb/unittests/DAP/TestBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,17 @@

#include "DAP.h"
#include "Protocol/ProtocolBase.h"
#include "TestingSupport/Host/PipeTestUtilities.h"
#include "Transport.h"
#include "lldb/Host/Pipe.h"
#include "llvm/ADT/StringRef.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace lldb_dap_tests {

/// A base class for tests that need a pair of pipes for communication.
class PipeBase : public testing::Test {
protected:
lldb_private::Pipe input;
lldb_private::Pipe output;

void SetUp() override;
};

/// A base class for tests that need transport configured for communicating DAP
/// messages.
class TransportBase : public PipeBase {
class TransportBase : public PipeTest {
protected:
std::unique_ptr<lldb_dap::Transport> to_dap;
std::unique_ptr<lldb_dap::Transport> from_dap;
Expand Down
98 changes: 0 additions & 98 deletions lldb/unittests/DAP/TransportTest.cpp
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions lldb/unittests/Host/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ set (FILES
HostInfoTest.cpp
HostTest.cpp
MainLoopTest.cpp
JSONTransportTest.cpp
NativeProcessProtocolTest.cpp
PipeTest.cpp
ProcessLaunchInfoTest.cpp
Expand Down
146 changes: 146 additions & 0 deletions lldb/unittests/Host/JSONTransportTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
//===-- JSONTransportTest.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 "TestingSupport/Host/PipeTestUtilities.h"
#include "lldb/Host/File.h"

using namespace llvm;
using namespace lldb_private;

namespace {
template <typename T> class JSONTransportTest : public PipeTest {
protected:
std::unique_ptr<JSONTransport> transport;

void SetUp() override {
PipeTest::SetUp();
transport = std::make_unique<T>(
std::make_shared<NativeFile>(input.GetReadFileDescriptor(),
File::eOpenOptionReadOnly,
NativeFile::Unowned),
std::make_shared<NativeFile>(output.GetWriteFileDescriptor(),
File::eOpenOptionWriteOnly,
NativeFile::Unowned));
}
};

class HTTPDelimitedJSONTransportTest
: public JSONTransportTest<HTTPDelimitedJSONTransport> {
public:
using JSONTransportTest::JSONTransportTest;
};

class JSONRPCTransportTest : public JSONTransportTest<JSONRPCTransport> {
public:
using JSONTransportTest::JSONTransportTest;
};

struct JSONTestType {
std::string str;
};

llvm::json::Value toJSON(const JSONTestType &T) {
return llvm::json::Object{{"str", T.str}};
}

bool fromJSON(const llvm::json::Value &V, JSONTestType &T, llvm::json::Path P) {
llvm::json::ObjectMapper O(V, P);
return O && O.map("str", T.str);
}
} // namespace

TEST_F(HTTPDelimitedJSONTransportTest, MalformedRequests) {
std::string malformed_header = "COnTent-LenGth: -1{}\r\n\r\nnotjosn";
ASSERT_THAT_EXPECTED(
input.Write(malformed_header.data(), malformed_header.size()),
Succeeded());
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
FailedWithMessage(
"expected 'Content-Length: ' and got 'COnTent-LenGth: '"));
}

TEST_F(HTTPDelimitedJSONTransportTest, Read) {
std::string json = R"json({"str": "foo"})json";
std::string message =
formatv("Content-Length: {0}\r\n\r\n{1}", json.size(), json).str();
ASSERT_THAT_EXPECTED(input.Write(message.data(), message.size()),
Succeeded());
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
HasValue(testing::FieldsAre(/*str=*/"foo")));
}

TEST_F(HTTPDelimitedJSONTransportTest, ReadWithTimeout) {
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
Failed<TransportTimeoutError>());
}

TEST_F(HTTPDelimitedJSONTransportTest, ReadWithEOF) {
input.CloseWriteFileDescriptor();
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
Failed<TransportEOFError>());
}

TEST_F(HTTPDelimitedJSONTransportTest, Write) {
ASSERT_THAT_ERROR(transport->Write(JSONTestType{"foo"}), Succeeded());
output.CloseWriteFileDescriptor();
char buf[1024];
Expected<size_t> bytes_read =
output.Read(buf, sizeof(buf), std::chrono::milliseconds(1));
ASSERT_THAT_EXPECTED(bytes_read, Succeeded());
ASSERT_EQ(StringRef(buf, *bytes_read), StringRef("Content-Length: 13\r\n\r\n"
R"json({"str":"foo"})json"));
}

TEST_F(JSONRPCTransportTest, MalformedRequests) {
std::string malformed_header = "notjson\n";
ASSERT_THAT_EXPECTED(
input.Write(malformed_header.data(), malformed_header.size()),
Succeeded());
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
llvm::Failed());
}

TEST_F(JSONRPCTransportTest, Read) {
std::string json = R"json({"str": "foo"})json";
std::string message = formatv("{0}\n", json).str();
ASSERT_THAT_EXPECTED(input.Write(message.data(), message.size()),
Succeeded());
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
HasValue(testing::FieldsAre(/*str=*/"foo")));
}

TEST_F(JSONRPCTransportTest, ReadWithTimeout) {
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
Failed<TransportTimeoutError>());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just realized, should this test be skipped on Windows?

The lldb SelectHelper only works with sockets on Windows and under the hood the Pipe is making a named socket, but I'm not sure if we're using the right API to realize that or not. When we create the handle we're using NativeFile that may be handling this correctly.

This all might be working correctly on Windows, but I'll be honest that I cannot tell for sure.

The frustrating (for me) part is that on Windows, stdin/stdout are anonymous pipes and you cannot use overlapping IO operations on an anonymous pipe on Windows. Libraries like libdispatch (used by swift), libuv (used by nodejs and others), pythons asyncio, etc. on Windows emulate this behavior by spawning a thread to do the reading, but at the moment we don't have that sort of abstraction in lldb yet.

If we're planning on making more improvements in this area for a json-rpc server, we may need to add this to lldb so we can properly handle stopping these read threads.

I'll be honest though that I haven't done much programing on Windows and I tried seeing if I could figure this out when I was working on lldb-dap server mode and kept running into issues. I could be wrong on some of this, since I don't know the Windows APIs all that well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, yeah I'm far from an expert myself, but it's good context, thanks.


TEST_F(JSONRPCTransportTest, ReadWithEOF) {
input.CloseWriteFileDescriptor();
ASSERT_THAT_EXPECTED(
transport->Read<JSONTestType>(std::chrono::milliseconds(1)),
Failed<TransportEOFError>());
}

TEST_F(JSONRPCTransportTest, Write) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have a test where we close the input or output handle before the read/write to trigger the TransportClosedError?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test and renamed the error as the current name isn't really accurate: this error occurs when the underlying IO object is invalid. If you close the write side, you get an EOF error, which we have a test for. If you close the read side, then you get a bad file descriptor error, which I've added a new test for too.

ASSERT_THAT_ERROR(transport->Write(JSONTestType{"foo"}), Succeeded());
output.CloseWriteFileDescriptor();
char buf[1024];
Expected<size_t> bytes_read =
output.Read(buf, sizeof(buf), std::chrono::milliseconds(1));
ASSERT_THAT_EXPECTED(bytes_read, Succeeded());
ASSERT_EQ(StringRef(buf, *bytes_read), StringRef(R"json({"str":"foo"})json"
"\n"));
}
Loading
Loading