Skip to content

[lldb] Add a MainLoop version of DomainSocket::Accept #108188

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

Merged
merged 2 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 12 additions & 1 deletion lldb/include/lldb/Host/Socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <memory>
#include <string>

#include "lldb/Host/MainLoopBase.h"
#include "lldb/lldb-private.h"

#include "lldb/Host/SocketAddress.h"
Expand Down Expand Up @@ -97,7 +98,17 @@ class Socket : public IOObject {

virtual Status Connect(llvm::StringRef name) = 0;
virtual Status Listen(llvm::StringRef name, int backlog) = 0;
virtual Status Accept(Socket *&socket) = 0;

// Use the provided main loop instance to accept new connections. The callback
// will be called (from MainLoop::Run) for each new connection. This function
// does not block.
virtual llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>>
Accept(MainLoopBase &loop,
std::function<void(std::unique_ptr<Socket> socket)> sock_cb) = 0;

// Accept a single connection and "return" it in the pointer argument. This
// function blocks until the connection arrives.
virtual Status Accept(Socket *&socket);

// Initialize a Tcp Socket object in listening mode. listen and accept are
// implemented separately because the caller may wish to manipulate or query
Expand Down
10 changes: 2 additions & 8 deletions lldb/include/lldb/Host/common/TCPSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,10 @@ class TCPSocket : public Socket {
Status Connect(llvm::StringRef name) override;
Status Listen(llvm::StringRef name, int backlog) override;

// Use the provided main loop instance to accept new connections. The callback
// will be called (from MainLoop::Run) for each new connection. This function
// does not block.
using Socket::Accept;
llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>>
Accept(MainLoopBase &loop,
std::function<void(std::unique_ptr<TCPSocket> socket)> sock_cb);

// Accept a single connection and "return" it in the pointer argument. This
// function blocks until the connection arrives.
Status Accept(Socket *&conn_socket) override;
std::function<void(std::unique_ptr<Socket> socket)> sock_cb) override;

Status CreateSocket(int domain);

Expand Down
8 changes: 7 additions & 1 deletion lldb/include/lldb/Host/common/UDPSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ class UDPSocket : public Socket {
size_t Send(const void *buf, const size_t num_bytes) override;
Status Connect(llvm::StringRef name) override;
Status Listen(llvm::StringRef name, int backlog) override;
Status Accept(Socket *&socket) override;

llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>>
Accept(MainLoopBase &loop,
std::function<void(std::unique_ptr<Socket> socket)> sock_cb) override {
return llvm::errorCodeToError(
std::make_error_code(std::errc::operation_not_supported));
}

SocketAddress m_sockaddr;
};
Expand Down
8 changes: 7 additions & 1 deletion lldb/include/lldb/Host/posix/DomainSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,17 @@
namespace lldb_private {
class DomainSocket : public Socket {
public:
DomainSocket(NativeSocket socket, bool should_close,
bool child_processes_inherit);
DomainSocket(bool should_close, bool child_processes_inherit);

Status Connect(llvm::StringRef name) override;
Status Listen(llvm::StringRef name, int backlog) override;
Status Accept(Socket *&socket) override;

using Socket::Accept;
llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>>
Accept(MainLoopBase &loop,
std::function<void(std::unique_ptr<Socket> socket)> sock_cb) override;

std::string GetRemoteConnectionURI() const override;

Expand Down
14 changes: 14 additions & 0 deletions lldb/source/Host/common/Socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "lldb/Host/Config.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/MainLoop.h"
#include "lldb/Host/SocketAddress.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Host/common/UDPSocket.h"
Expand Down Expand Up @@ -459,6 +460,19 @@ NativeSocket Socket::CreateSocket(const int domain, const int type,
return sock;
}

Status Socket::Accept(Socket *&socket) {
MainLoop accept_loop;
llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> expected_handles =
Accept(accept_loop,
[&accept_loop, &socket](std::unique_ptr<Socket> sock) {
socket = sock.release();
accept_loop.RequestTermination();
});
if (!expected_handles)
return Status::FromError(expected_handles.takeError());
return accept_loop.Run();
}

NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
socklen_t *addrlen,
bool child_processes_inherit, Status &error) {
Expand Down
19 changes: 3 additions & 16 deletions lldb/source/Host/common/TCPSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,9 @@ void TCPSocket::CloseListenSockets() {
m_listen_sockets.clear();
}

llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> TCPSocket::Accept(
MainLoopBase &loop,
std::function<void(std::unique_ptr<TCPSocket> socket)> sock_cb) {
llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>>
TCPSocket::Accept(MainLoopBase &loop,
std::function<void(std::unique_ptr<Socket> socket)> sock_cb) {
if (m_listen_sockets.size() == 0)
return llvm::createStringError("No open listening sockets!");

Expand Down Expand Up @@ -278,19 +278,6 @@ llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> TCPSocket::Accept(
return handles;
}

Status TCPSocket::Accept(Socket *&conn_socket) {
MainLoop accept_loop;
llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> expected_handles =
Accept(accept_loop,
[&accept_loop, &conn_socket](std::unique_ptr<TCPSocket> sock) {
conn_socket = sock.release();
accept_loop.RequestTermination();
});
if (!expected_handles)
return Status::FromError(expected_handles.takeError());
return accept_loop.Run();
}

int TCPSocket::SetOptionNoDelay() {
return SetOption(IPPROTO_TCP, TCP_NODELAY, 1);
}
Expand Down
4 changes: 0 additions & 4 deletions lldb/source/Host/common/UDPSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ Status UDPSocket::Listen(llvm::StringRef name, int backlog) {
return Status::FromErrorStringWithFormat("%s", g_not_supported_error);
}

Status UDPSocket::Accept(Socket *&socket) {
return Status::FromErrorStringWithFormat("%s", g_not_supported_error);
}

llvm::Expected<std::unique_ptr<UDPSocket>>
UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
std::unique_ptr<UDPSocket> socket;
Expand Down
42 changes: 34 additions & 8 deletions lldb/source/Host/posix/DomainSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
//===----------------------------------------------------------------------===//

#include "lldb/Host/posix/DomainSocket.h"
#include "lldb/Utility/LLDBLog.h"

#include "llvm/Support/Errno.h"
#include "llvm/Support/FileSystem.h"

#include <cstddef>
#include <memory>
#include <sys/socket.h>
#include <sys/un.h>

Expand Down Expand Up @@ -57,7 +59,14 @@ static bool SetSockAddr(llvm::StringRef name, const size_t name_offset,
}

DomainSocket::DomainSocket(bool should_close, bool child_processes_inherit)
: Socket(ProtocolUnixDomain, should_close, child_processes_inherit) {}
: DomainSocket(kInvalidSocketValue, should_close, child_processes_inherit) {
}

DomainSocket::DomainSocket(NativeSocket socket, bool should_close,
bool child_processes_inherit)
: Socket(ProtocolUnixDomain, should_close, child_processes_inherit) {
m_socket = socket;
}

DomainSocket::DomainSocket(SocketProtocol protocol,
bool child_processes_inherit)
Expand Down Expand Up @@ -108,14 +117,31 @@ Status DomainSocket::Listen(llvm::StringRef name, int backlog) {
return error;
}

Status DomainSocket::Accept(Socket *&socket) {
Status error;
auto conn_fd = AcceptSocket(GetNativeSocket(), nullptr, nullptr,
m_child_processes_inherit, error);
if (error.Success())
socket = new DomainSocket(conn_fd, *this);
llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> DomainSocket::Accept(
MainLoopBase &loop,
std::function<void(std::unique_ptr<Socket> socket)> sock_cb) {
// TODO: Refactor MainLoop to avoid the shared_ptr requirement.
auto io_sp = std::make_shared<DomainSocket>(GetNativeSocket(), false,
m_child_processes_inherit);
auto cb = [this, sock_cb](MainLoopBase &loop) {
Log *log = GetLog(LLDBLog::Host);
Status error;
auto conn_fd = AcceptSocket(GetNativeSocket(), nullptr, nullptr,
m_child_processes_inherit, error);
if (error.Fail()) {
LLDB_LOG(log, "AcceptSocket({0}): {1}", GetNativeSocket(), error);
return;
}
std::unique_ptr<DomainSocket> sock_up(new DomainSocket(conn_fd, *this));
sock_cb(std::move(sock_up));
};

return error;
Status error;
std::vector<MainLoopBase::ReadHandleUP> handles;
handles.emplace_back(loop.RegisterReadObject(io_sp, cb, error));
if (error.Fail())
return error.ToError();
return handles;
}

size_t DomainSocket::GetNameOffset() const { return 0; }
Expand Down
41 changes: 39 additions & 2 deletions lldb/unittests/Host/SocketTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,43 @@ TEST_P(SocketTest, DomainListenConnectAccept) {
std::unique_ptr<DomainSocket> socket_b_up;
CreateDomainConnectedSockets(Path, &socket_a_up, &socket_b_up);
}

TEST_P(SocketTest, DomainMainLoopAccept) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add #if LLDB_ENABLE_POSIX. Otherwise it will fail on Windows.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's already in a ifdef block, next to the other domain socket test. That's why you don't see an ifdef in the default view.

llvm::SmallString<64> Path;
std::error_code EC =
llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", Path);
ASSERT_FALSE(EC);
llvm::sys::path::append(Path, "test");

// Skip the test if the $TMPDIR is too long to hold a domain socket.
if (Path.size() > 107u)
return;

auto listen_socket_up = std::make_unique<DomainSocket>(
/*should_close=*/true, /*child_process_inherit=*/false);
Status error = listen_socket_up->Listen(Path, 5);
ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
ASSERT_TRUE(listen_socket_up->IsValid());

MainLoop loop;
std::unique_ptr<Socket> accepted_socket_up;
auto expected_handles = listen_socket_up->Accept(
loop, [&accepted_socket_up, &loop](std::unique_ptr<Socket> sock_up) {
accepted_socket_up = std::move(sock_up);
loop.RequestTermination();
});
ASSERT_THAT_EXPECTED(expected_handles, llvm::Succeeded());

auto connect_socket_up = std::make_unique<DomainSocket>(
/*should_close=*/true, /*child_process_inherit=*/false);
ASSERT_THAT_ERROR(connect_socket_up->Connect(Path).ToError(),
llvm::Succeeded());
ASSERT_TRUE(connect_socket_up->IsValid());

loop.Run();
ASSERT_TRUE(accepted_socket_up);
ASSERT_TRUE(accepted_socket_up->IsValid());
}
#endif

TEST_P(SocketTest, TCPListen0ConnectAccept) {
Expand All @@ -109,9 +146,9 @@ TEST_P(SocketTest, TCPMainLoopAccept) {
ASSERT_TRUE(listen_socket_up->IsValid());

MainLoop loop;
std::unique_ptr<TCPSocket> accepted_socket_up;
std::unique_ptr<Socket> accepted_socket_up;
auto expected_handles = listen_socket_up->Accept(
loop, [&accepted_socket_up, &loop](std::unique_ptr<TCPSocket> sock_up) {
loop, [&accepted_socket_up, &loop](std::unique_ptr<Socket> sock_up) {
accepted_socket_up = std::move(sock_up);
loop.RequestTermination();
});
Expand Down
Loading