[lldb] Remove child_process_inherit from the socket classes (#117699)

It's never set to true. Also, using inheritable FDs in a multithreaded
process pretty much guarantees descriptor leaks. It's better to
explicitly pass a specific FD to a specific subprocess, which we already
mostly can do using the ProcessLaunchInfo FileActions.

Author: labath

lldb/include/lldb/Host/Socket.h

@@ -94,7 +94,6 @@ class Socket : public IOObject {
   static void Terminate();
 
   static std::unique_ptr<Socket> Create(const SocketProtocol protocol,
-                                        bool child_processes_inherit,
                                         Status &error);
 
   virtual Status Connect(llvm::StringRef name) = 0;
@@ -115,14 +114,13 @@ class Socket : public IOObject {
   // implemented separately because the caller may wish to manipulate or query
   // the socket after it is initialized, but before entering a blocking accept.
   static llvm::Expected<std::unique_ptr<TCPSocket>>
-  TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
-            int backlog = 5);
+  TcpListen(llvm::StringRef host_and_port, int backlog = 5);
 
   static llvm::Expected<std::unique_ptr<Socket>>
-  TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+  TcpConnect(llvm::StringRef host_and_port);
 
   static llvm::Expected<std::unique_ptr<UDPSocket>>
-  UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+  UdpConnect(llvm::StringRef host_and_port);
 
   static int GetOption(NativeSocket sockfd, int level, int option_name,
                        int &option_value);
@@ -154,24 +152,20 @@ class Socket : public IOObject {
   virtual std::string GetRemoteConnectionURI() const { return ""; };
 
 protected:
-  Socket(SocketProtocol protocol, bool should_close,
-         bool m_child_process_inherit);
+  Socket(SocketProtocol protocol, bool should_close);
 
   virtual size_t Send(const void *buf, const size_t num_bytes);
 
   static int CloseSocket(NativeSocket sockfd);
   static Status GetLastError();
   static void SetLastError(Status &error);
   static NativeSocket CreateSocket(const int domain, const int type,
-                                   const int protocol,
-                                   bool child_processes_inherit, Status &error);
+                                   const int protocol, Status &error);
   static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
-                                   socklen_t *addrlen,
-                                   bool child_processes_inherit, Status &error);
+                                   socklen_t *addrlen, Status &error);
 
   SocketProtocol m_protocol;
   NativeSocket m_socket;
-  bool m_child_processes_inherit;
   bool m_should_close_fd;
 };
 

lldb/include/lldb/Host/common/TCPSocket.h

@@ -17,9 +17,8 @@
 namespace lldb_private {
 class TCPSocket : public Socket {
 public:
-  TCPSocket(bool should_close, bool child_processes_inherit);
-  TCPSocket(NativeSocket socket, bool should_close,
-            bool child_processes_inherit);
+  explicit TCPSocket(bool should_close);
+  TCPSocket(NativeSocket socket, bool should_close);
   ~TCPSocket() override;
 
   // returns port number or 0 if error

lldb/include/lldb/Host/common/UDPSocket.h

@@ -14,10 +14,10 @@
 namespace lldb_private {
 class UDPSocket : public Socket {
 public:
-  UDPSocket(bool should_close, bool child_processes_inherit);
+  explicit UDPSocket(bool should_close);
 
   static llvm::Expected<std::unique_ptr<UDPSocket>>
-  Connect(llvm::StringRef name, bool child_processes_inherit);
+  CreateConnected(llvm::StringRef name);
 
   std::string GetRemoteConnectionURI() const override;
 

lldb/include/lldb/Host/linux/AbstractSocket.h

@@ -14,7 +14,7 @@
 namespace lldb_private {
 class AbstractSocket : public DomainSocket {
 public:
-  AbstractSocket(bool child_processes_inherit);
+  AbstractSocket();
 
 protected:
   size_t GetNameOffset() const override;

lldb/include/lldb/Host/posix/DomainSocket.h

@@ -14,9 +14,8 @@
 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);
+  DomainSocket(NativeSocket socket, bool should_close);
+  explicit DomainSocket(bool should_close);
 
   Status Connect(llvm::StringRef name) override;
   Status Listen(llvm::StringRef name, int backlog) override;
@@ -29,7 +28,7 @@ class DomainSocket : public Socket {
   std::string GetRemoteConnectionURI() const override;
 
 protected:
-  DomainSocket(SocketProtocol protocol, bool child_processes_inherit);
+  DomainSocket(SocketProtocol protocol);
 
   virtual size_t GetNameOffset() const;
   virtual void DeleteSocketFile(llvm::StringRef name);

lldb/source/Host/common/Socket.cpp

@@ -180,12 +180,9 @@ bool Socket::FindProtocolByScheme(const char *scheme,
   return false;
 }
 
-Socket::Socket(SocketProtocol protocol, bool should_close,
-               bool child_processes_inherit)
+Socket::Socket(SocketProtocol protocol, bool should_close)
     : IOObject(eFDTypeSocket), m_protocol(protocol),
-      m_socket(kInvalidSocketValue),
-      m_child_processes_inherit(child_processes_inherit),
-      m_should_close_fd(should_close) {}
+      m_socket(kInvalidSocketValue), m_should_close_fd(should_close) {}
 
 Socket::~Socket() { Close(); }
 
@@ -214,33 +211,29 @@ void Socket::Terminate() {
 }
 
 std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
-                                       bool child_processes_inherit,
                                        Status &error) {
   error.Clear();
 
+  const bool should_close = true;
   std::unique_ptr<Socket> socket_up;
   switch (protocol) {
   case ProtocolTcp:
-    socket_up =
-        std::make_unique<TCPSocket>(true, child_processes_inherit);
+    socket_up = std::make_unique<TCPSocket>(should_close);
     break;
   case ProtocolUdp:
-    socket_up =
-        std::make_unique<UDPSocket>(true, child_processes_inherit);
+    socket_up = std::make_unique<UDPSocket>(should_close);
     break;
   case ProtocolUnixDomain:
 #if LLDB_ENABLE_POSIX
-    socket_up =
-        std::make_unique<DomainSocket>(true, child_processes_inherit);
+    socket_up = std::make_unique<DomainSocket>(should_close);
 #else
     error = Status::FromErrorString(
         "Unix domain sockets are not supported on this platform.");
 #endif
     break;
   case ProtocolUnixAbstract:
 #ifdef __linux__
-    socket_up =
-        std::make_unique<AbstractSocket>(child_processes_inherit);
+    socket_up = std::make_unique<AbstractSocket>();
 #else
     error = Status::FromErrorString(
         "Abstract domain sockets are not supported on this platform.");
@@ -255,14 +248,12 @@ std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
 }
 
 llvm::Expected<std::unique_ptr<Socket>>
-Socket::TcpConnect(llvm::StringRef host_and_port,
-                   bool child_processes_inherit) {
+Socket::TcpConnect(llvm::StringRef host_and_port) {
   Log *log = GetLog(LLDBLog::Connection);
   LLDB_LOG(log, "host_and_port = {0}", host_and_port);
 
   Status error;
-  std::unique_ptr<Socket> connect_socket(
-      Create(ProtocolTcp, child_processes_inherit, error));
+  std::unique_ptr<Socket> connect_socket = Create(ProtocolTcp, error);
   if (error.Fail())
     return error.ToError();
 
@@ -274,13 +265,12 @@ Socket::TcpConnect(llvm::StringRef host_and_port,
 }
 
 llvm::Expected<std::unique_ptr<TCPSocket>>
-Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
-                  int backlog) {
+Socket::TcpListen(llvm::StringRef host_and_port, int backlog) {
   Log *log = GetLog(LLDBLog::Connection);
   LLDB_LOG(log, "host_and_port = {0}", host_and_port);
 
   std::unique_ptr<TCPSocket> listen_socket(
-      new TCPSocket(true, child_processes_inherit));
+      new TCPSocket(/*should_close=*/true));
 
   Status error = listen_socket->Listen(host_and_port, backlog);
   if (error.Fail())
@@ -290,9 +280,8 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
 }
 
 llvm::Expected<std::unique_ptr<UDPSocket>>
-Socket::UdpConnect(llvm::StringRef host_and_port,
-                   bool child_processes_inherit) {
-  return UDPSocket::Connect(host_and_port, child_processes_inherit);
+Socket::UdpConnect(llvm::StringRef host_and_port) {
+  return UDPSocket::CreateConnected(host_and_port);
 }
 
 llvm::Expected<Socket::HostAndPort> Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
@@ -445,13 +434,11 @@ int Socket::CloseSocket(NativeSocket sockfd) {
 }
 
 NativeSocket Socket::CreateSocket(const int domain, const int type,
-                                  const int protocol,
-                                  bool child_processes_inherit, Status &error) {
+                                  const int protocol, Status &error) {
   error.Clear();
   auto socket_type = type;
 #ifdef SOCK_CLOEXEC
-  if (!child_processes_inherit)
-    socket_type |= SOCK_CLOEXEC;
+  socket_type |= SOCK_CLOEXEC;
 #endif
   auto sock = ::socket(domain, socket_type, protocol);
   if (sock == kInvalidSocketValue)
@@ -483,8 +470,7 @@ Status Socket::Accept(const Timeout<std::micro> &timeout, Socket *&socket) {
 }
 
 NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
-                                  socklen_t *addrlen,
-                                  bool child_processes_inherit, Status &error) {
+                                  socklen_t *addrlen, Status &error) {
   error.Clear();
 #if defined(ANDROID_USE_ACCEPT_WORKAROUND)
   // Hack:
@@ -494,7 +480,7 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
   // available in older kernels. Using an older libc would fix this issue, but
   // introduce other ones, as the old libraries were quite buggy.
   int fd = syscall(__NR_accept, sockfd, addr, addrlen);
-  if (fd >= 0 && !child_processes_inherit) {
+  if (fd >= 0) {
     int flags = ::fcntl(fd, F_GETFD);
     if (flags != -1 && ::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) != -1)
       return fd;
@@ -503,10 +489,7 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
   }
   return fd;
 #elif defined(SOCK_CLOEXEC) && defined(HAVE_ACCEPT4)
-  int flags = 0;
-  if (!child_processes_inherit) {
-    flags |= SOCK_CLOEXEC;
-  }
+  int flags = SOCK_CLOEXEC;
   NativeSocket fd = llvm::sys::RetryAfterSignal(
       static_cast<NativeSocket>(-1), ::accept4, sockfd, addr, addrlen, flags);
 #else

lldb/source/Host/common/TCPSocket.cpp

@@ -38,18 +38,15 @@ using namespace lldb_private;
 
 static const int kType = SOCK_STREAM;
 
-TCPSocket::TCPSocket(bool should_close, bool child_processes_inherit)
-    : Socket(ProtocolTcp, should_close, child_processes_inherit) {}
+TCPSocket::TCPSocket(bool should_close) : Socket(ProtocolTcp, should_close) {}
 
 TCPSocket::TCPSocket(NativeSocket socket, const TCPSocket &listen_socket)
-    : Socket(ProtocolTcp, listen_socket.m_should_close_fd,
-             listen_socket.m_child_processes_inherit) {
+    : Socket(ProtocolTcp, listen_socket.m_should_close_fd) {
   m_socket = socket;
 }
 
-TCPSocket::TCPSocket(NativeSocket socket, bool should_close,
-                     bool child_processes_inherit)
-    : Socket(ProtocolTcp, should_close, child_processes_inherit) {
+TCPSocket::TCPSocket(NativeSocket socket, bool should_close)
+    : Socket(ProtocolTcp, should_close) {
   m_socket = socket;
 }
 
@@ -124,8 +121,7 @@ Status TCPSocket::CreateSocket(int domain) {
     error = Close();
   if (error.Fail())
     return error;
-  m_socket = Socket::CreateSocket(domain, kType, IPPROTO_TCP,
-                                  m_child_processes_inherit, error);
+  m_socket = Socket::CreateSocket(domain, kType, IPPROTO_TCP, error);
   return error;
 }
 
@@ -183,8 +179,8 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
   std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
       host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
   for (SocketAddress &address : addresses) {
-    int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
-                                  m_child_processes_inherit, error);
+    int fd =
+        Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP, error);
     if (error.Fail() || fd < 0)
       continue;
 
@@ -241,14 +237,13 @@ TCPSocket::Accept(MainLoopBase &loop,
   std::vector<MainLoopBase::ReadHandleUP> handles;
   for (auto socket : m_listen_sockets) {
     auto fd = socket.first;
-    auto io_sp =
-        std::make_shared<TCPSocket>(fd, false, this->m_child_processes_inherit);
+    auto io_sp = std::make_shared<TCPSocket>(fd, false);
     auto cb = [this, fd, sock_cb](MainLoopBase &loop) {
       lldb_private::SocketAddress AcceptAddr;
       socklen_t sa_len = AcceptAddr.GetMaxLength();
       Status error;
-      NativeSocket sock = AcceptSocket(fd, &AcceptAddr.sockaddr(), &sa_len,
-                                       m_child_processes_inherit, error);
+      NativeSocket sock =
+          AcceptSocket(fd, &AcceptAddr.sockaddr(), &sa_len, error);
       Log *log = GetLog(LLDBLog::Host);
       if (error.Fail()) {
         LLDB_LOG(log, "AcceptSocket({0}): {1}", fd, error);

lldb/source/Host/common/UDPSocket.cpp

@@ -27,12 +27,12 @@ static const int kType = SOCK_DGRAM;
 
 static const char *g_not_supported_error = "Not supported";
 
-UDPSocket::UDPSocket(NativeSocket socket) : Socket(ProtocolUdp, true, true) {
+UDPSocket::UDPSocket(NativeSocket socket)
+    : Socket(ProtocolUdp, /*should_close=*/true) {
   m_socket = socket;
 }
 
-UDPSocket::UDPSocket(bool should_close, bool child_processes_inherit)
-    : Socket(ProtocolUdp, should_close, child_processes_inherit) {}
+UDPSocket::UDPSocket(bool should_close) : Socket(ProtocolUdp, should_close) {}
 
 size_t UDPSocket::Send(const void *buf, const size_t num_bytes) {
   return ::sendto(m_socket, static_cast<const char *>(buf), num_bytes, 0,
@@ -48,7 +48,7 @@ Status UDPSocket::Listen(llvm::StringRef name, int backlog) {
 }
 
 llvm::Expected<std::unique_ptr<UDPSocket>>
-UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
+UDPSocket::CreateConnected(llvm::StringRef name) {
   std::unique_ptr<UDPSocket> socket;
 
   Log *log = GetLog(LLDBLog::Connection);
@@ -84,9 +84,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
   for (struct addrinfo *service_info_ptr = service_info_list;
        service_info_ptr != nullptr;
        service_info_ptr = service_info_ptr->ai_next) {
-    auto send_fd = CreateSocket(
-        service_info_ptr->ai_family, service_info_ptr->ai_socktype,
-        service_info_ptr->ai_protocol, child_processes_inherit, error);
+    auto send_fd =
+        CreateSocket(service_info_ptr->ai_family, service_info_ptr->ai_socktype,
+                     service_info_ptr->ai_protocol, error);
     if (error.Success()) {
       socket.reset(new UDPSocket(send_fd));
       socket->m_sockaddr = service_info_ptr;

lldb/source/Host/linux/AbstractSocket.cpp

@@ -13,8 +13,7 @@
 using namespace lldb;
 using namespace lldb_private;
 
-AbstractSocket::AbstractSocket(bool child_processes_inherit)
-    : DomainSocket(ProtocolUnixAbstract, child_processes_inherit) {}
+AbstractSocket::AbstractSocket() : DomainSocket(ProtocolUnixAbstract) {}
 
 size_t AbstractSocket::GetNameOffset() const { return 1; }