Skip to content

Commit 6471ca1

Browse files
labathlabath
committed
[lldb] Remove child_process_inherit from the socket classes
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.
1 parent 5eeb3fe commit 6471ca1

File tree

18 files changed

+83
-137
lines changed

18 files changed

+83
-137
lines changed

‎lldb/include/lldb/Host/Socket.h

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ class Socket : public IOObject {
9494
static void Terminate();
9595

9696
static std::unique_ptr<Socket> Create(const SocketProtocol protocol,
97-
bool child_processes_inherit,
9897
Status &error);
9998

10099
virtual Status Connect(llvm::StringRef name) = 0;
@@ -115,14 +114,13 @@ class Socket : public IOObject {
115114
// implemented separately because the caller may wish to manipulate or query
116115
// the socket after it is initialized, but before entering a blocking accept.
117116
static llvm::Expected<std::unique_ptr<TCPSocket>>
118-
TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
119-
int backlog = 5);
117+
TcpListen(llvm::StringRef host_and_port, int backlog = 5);
120118

121119
static llvm::Expected<std::unique_ptr<Socket>>
122-
TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
120+
TcpConnect(llvm::StringRef host_and_port);
123121

124122
static llvm::Expected<std::unique_ptr<UDPSocket>>
125-
UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
123+
UdpConnect(llvm::StringRef host_and_port);
126124

127125
static int GetOption(NativeSocket sockfd, int level, int option_name,
128126
int &option_value);
@@ -154,24 +152,20 @@ class Socket : public IOObject {
154152
virtual std::string GetRemoteConnectionURI() const { return ""; };
155153

156154
protected:
157-
Socket(SocketProtocol protocol, bool should_close,
158-
bool m_child_process_inherit);
155+
Socket(SocketProtocol protocol, bool should_close);
159156

160157
virtual size_t Send(const void *buf, const size_t num_bytes);
161158

162159
static int CloseSocket(NativeSocket sockfd);
163160
static Status GetLastError();
164161
static void SetLastError(Status &error);
165162
static NativeSocket CreateSocket(const int domain, const int type,
166-
const int protocol,
167-
bool child_processes_inherit, Status &error);
163+
const int protocol, Status &error);
168164
static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
169-
socklen_t *addrlen,
170-
bool child_processes_inherit, Status &error);
165+
socklen_t *addrlen, Status &error);
171166

172167
SocketProtocol m_protocol;
173168
NativeSocket m_socket;
174-
bool m_child_processes_inherit;
175169
bool m_should_close_fd;
176170
};
177171

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@
1717
namespace lldb_private {
1818
class TCPSocket : public Socket {
1919
public:
20-
TCPSocket(bool should_close, bool child_processes_inherit);
21-
TCPSocket(NativeSocket socket, bool should_close,
22-
bool child_processes_inherit);
20+
explicit TCPSocket(bool should_close);
21+
TCPSocket(NativeSocket socket, bool should_close);
2322
~TCPSocket() override;
2423

2524
// returns port number or 0 if error

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
namespace lldb_private {
1515
class UDPSocket : public Socket {
1616
public:
17-
UDPSocket(bool should_close, bool child_processes_inherit);
17+
explicit UDPSocket(bool should_close);
1818

1919
static llvm::Expected<std::unique_ptr<UDPSocket>>
20-
Connect(llvm::StringRef name, bool child_processes_inherit);
20+
CreateConnected(llvm::StringRef name);
2121

2222
std::string GetRemoteConnectionURI() const override;
2323

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
namespace lldb_private {
1515
class AbstractSocket : public DomainSocket {
1616
public:
17-
AbstractSocket(bool child_processes_inherit);
17+
AbstractSocket();
1818

1919
protected:
2020
size_t GetNameOffset() const override;

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@
1414
namespace lldb_private {
1515
class DomainSocket : public Socket {
1616
public:
17-
DomainSocket(NativeSocket socket, bool should_close,
18-
bool child_processes_inherit);
19-
DomainSocket(bool should_close, bool child_processes_inherit);
17+
DomainSocket(NativeSocket socket, bool should_close);
18+
explicit DomainSocket(bool should_close);
2019

2120
Status Connect(llvm::StringRef name) override;
2221
Status Listen(llvm::StringRef name, int backlog) override;
@@ -29,7 +28,7 @@ class DomainSocket : public Socket {
2928
std::string GetRemoteConnectionURI() const override;
3029

3130
protected:
32-
DomainSocket(SocketProtocol protocol, bool child_processes_inherit);
31+
DomainSocket(SocketProtocol protocol);
3332

3433
virtual size_t GetNameOffset() const;
3534
virtual void DeleteSocketFile(llvm::StringRef name);

‎lldb/source/Host/common/Socket.cpp

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,9 @@ bool Socket::FindProtocolByScheme(const char *scheme,
180180
return false;
181181
}
182182

183-
Socket::Socket(SocketProtocol protocol, bool should_close,
184-
bool child_processes_inherit)
183+
Socket::Socket(SocketProtocol protocol, bool should_close)
185184
: IOObject(eFDTypeSocket), m_protocol(protocol),
186-
m_socket(kInvalidSocketValue),
187-
m_child_processes_inherit(child_processes_inherit),
188-
m_should_close_fd(should_close) {}
185+
m_socket(kInvalidSocketValue), m_should_close_fd(should_close) {}
189186

190187
Socket::~Socket() { Close(); }
191188

@@ -214,33 +211,29 @@ void Socket::Terminate() {
214211
}
215212

216213
std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
217-
bool child_processes_inherit,
218214
Status &error) {
219215
error.Clear();
220216

217+
const bool should_close = true;
221218
std::unique_ptr<Socket> socket_up;
222219
switch (protocol) {
223220
case ProtocolTcp:
224-
socket_up =
225-
std::make_unique<TCPSocket>(true, child_processes_inherit);
221+
socket_up = std::make_unique<TCPSocket>(should_close);
226222
break;
227223
case ProtocolUdp:
228-
socket_up =
229-
std::make_unique<UDPSocket>(true, child_processes_inherit);
224+
socket_up = std::make_unique<UDPSocket>(should_close);
230225
break;
231226
case ProtocolUnixDomain:
232227
#if LLDB_ENABLE_POSIX
233-
socket_up =
234-
std::make_unique<DomainSocket>(true, child_processes_inherit);
228+
socket_up = std::make_unique<DomainSocket>(should_close);
235229
#else
236230
error = Status::FromErrorString(
237231
"Unix domain sockets are not supported on this platform.");
238232
#endif
239233
break;
240234
case ProtocolUnixAbstract:
241235
#ifdef __linux__
242-
socket_up =
243-
std::make_unique<AbstractSocket>(child_processes_inherit);
236+
socket_up = std::make_unique<AbstractSocket>();
244237
#else
245238
error = Status::FromErrorString(
246239
"Abstract domain sockets are not supported on this platform.");
@@ -255,14 +248,12 @@ std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
255248
}
256249

257250
llvm::Expected<std::unique_ptr<Socket>>
258-
Socket::TcpConnect(llvm::StringRef host_and_port,
259-
bool child_processes_inherit) {
251+
Socket::TcpConnect(llvm::StringRef host_and_port) {
260252
Log *log = GetLog(LLDBLog::Connection);
261253
LLDB_LOG(log, "host_and_port = {0}", host_and_port);
262254

263255
Status error;
264-
std::unique_ptr<Socket> connect_socket(
265-
Create(ProtocolTcp, child_processes_inherit, error));
256+
std::unique_ptr<Socket> connect_socket = Create(ProtocolTcp, error);
266257
if (error.Fail())
267258
return error.ToError();
268259

@@ -274,13 +265,12 @@ Socket::TcpConnect(llvm::StringRef host_and_port,
274265
}
275266

276267
llvm::Expected<std::unique_ptr<TCPSocket>>
277-
Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
278-
int backlog) {
268+
Socket::TcpListen(llvm::StringRef host_and_port, int backlog) {
279269
Log *log = GetLog(LLDBLog::Connection);
280270
LLDB_LOG(log, "host_and_port = {0}", host_and_port);
281271

282272
std::unique_ptr<TCPSocket> listen_socket(
283-
new TCPSocket(true, child_processes_inherit));
273+
new TCPSocket(/*should_close=*/true));
284274

285275
Status error = listen_socket->Listen(host_and_port, backlog);
286276
if (error.Fail())
@@ -290,9 +280,8 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
290280
}
291281

292282
llvm::Expected<std::unique_ptr<UDPSocket>>
293-
Socket::UdpConnect(llvm::StringRef host_and_port,
294-
bool child_processes_inherit) {
295-
return UDPSocket::Connect(host_and_port, child_processes_inherit);
283+
Socket::UdpConnect(llvm::StringRef host_and_port) {
284+
return UDPSocket::CreateConnected(host_and_port);
296285
}
297286

298287
llvm::Expected<Socket::HostAndPort> Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
@@ -445,13 +434,11 @@ int Socket::CloseSocket(NativeSocket sockfd) {
445434
}
446435

447436
NativeSocket Socket::CreateSocket(const int domain, const int type,
448-
const int protocol,
449-
bool child_processes_inherit, Status &error) {
437+
const int protocol, Status &error) {
450438
error.Clear();
451439
auto socket_type = type;
452440
#ifdef SOCK_CLOEXEC
453-
if (!child_processes_inherit)
454-
socket_type |= SOCK_CLOEXEC;
441+
socket_type |= SOCK_CLOEXEC;
455442
#endif
456443
auto sock = ::socket(domain, socket_type, protocol);
457444
if (sock == kInvalidSocketValue)
@@ -483,8 +470,7 @@ Status Socket::Accept(const Timeout<std::micro> &timeout, Socket *&socket) {
483470
}
484471

485472
NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
486-
socklen_t *addrlen,
487-
bool child_processes_inherit, Status &error) {
473+
socklen_t *addrlen, Status &error) {
488474
error.Clear();
489475
#if defined(ANDROID_USE_ACCEPT_WORKAROUND)
490476
// Hack:
@@ -494,7 +480,7 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
494480
// available in older kernels. Using an older libc would fix this issue, but
495481
// introduce other ones, as the old libraries were quite buggy.
496482
int fd = syscall(__NR_accept, sockfd, addr, addrlen);
497-
if (fd >= 0 && !child_processes_inherit) {
483+
if (fd >= 0) {
498484
int flags = ::fcntl(fd, F_GETFD);
499485
if (flags != -1 && ::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) != -1)
500486
return fd;
@@ -503,10 +489,7 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
503489
}
504490
return fd;
505491
#elif defined(SOCK_CLOEXEC) && defined(HAVE_ACCEPT4)
506-
int flags = 0;
507-
if (!child_processes_inherit) {
508-
flags |= SOCK_CLOEXEC;
509-
}
492+
int flags = SOCK_CLOEXEC;
510493
NativeSocket fd = llvm::sys::RetryAfterSignal(
511494
static_cast<NativeSocket>(-1), ::accept4, sockfd, addr, addrlen, flags);
512495
#else

‎lldb/source/Host/common/TCPSocket.cpp

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,15 @@ using namespace lldb_private;
3838

3939
static const int kType = SOCK_STREAM;
4040

41-
TCPSocket::TCPSocket(bool should_close, bool child_processes_inherit)
42-
: Socket(ProtocolTcp, should_close, child_processes_inherit) {}
41+
TCPSocket::TCPSocket(bool should_close) : Socket(ProtocolTcp, should_close) {}
4342

4443
TCPSocket::TCPSocket(NativeSocket socket, const TCPSocket &listen_socket)
45-
: Socket(ProtocolTcp, listen_socket.m_should_close_fd,
46-
listen_socket.m_child_processes_inherit) {
44+
: Socket(ProtocolTcp, listen_socket.m_should_close_fd) {
4745
m_socket = socket;
4846
}
4947

50-
TCPSocket::TCPSocket(NativeSocket socket, bool should_close,
51-
bool child_processes_inherit)
52-
: Socket(ProtocolTcp, should_close, child_processes_inherit) {
48+
TCPSocket::TCPSocket(NativeSocket socket, bool should_close)
49+
: Socket(ProtocolTcp, should_close) {
5350
m_socket = socket;
5451
}
5552

@@ -124,8 +121,7 @@ Status TCPSocket::CreateSocket(int domain) {
124121
error = Close();
125122
if (error.Fail())
126123
return error;
127-
m_socket = Socket::CreateSocket(domain, kType, IPPROTO_TCP,
128-
m_child_processes_inherit, error);
124+
m_socket = Socket::CreateSocket(domain, kType, IPPROTO_TCP, error);
129125
return error;
130126
}
131127

@@ -183,8 +179,8 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
183179
std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
184180
host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
185181
for (SocketAddress &address : addresses) {
186-
int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
187-
m_child_processes_inherit, error);
182+
int fd =
183+
Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP, error);
188184
if (error.Fail() || fd < 0)
189185
continue;
190186

@@ -241,14 +237,13 @@ TCPSocket::Accept(MainLoopBase &loop,
241237
std::vector<MainLoopBase::ReadHandleUP> handles;
242238
for (auto socket : m_listen_sockets) {
243239
auto fd = socket.first;
244-
auto io_sp =
245-
std::make_shared<TCPSocket>(fd, false, this->m_child_processes_inherit);
240+
auto io_sp = std::make_shared<TCPSocket>(fd, false);
246241
auto cb = [this, fd, sock_cb](MainLoopBase &loop) {
247242
lldb_private::SocketAddress AcceptAddr;
248243
socklen_t sa_len = AcceptAddr.GetMaxLength();
249244
Status error;
250-
NativeSocket sock = AcceptSocket(fd, &AcceptAddr.sockaddr(), &sa_len,
251-
m_child_processes_inherit, error);
245+
NativeSocket sock =
246+
AcceptSocket(fd, &AcceptAddr.sockaddr(), &sa_len, error);
252247
Log *log = GetLog(LLDBLog::Host);
253248
if (error.Fail()) {
254249
LLDB_LOG(log, "AcceptSocket({0}): {1}", fd, error);

‎lldb/source/Host/common/UDPSocket.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ static const int kType = SOCK_DGRAM;
2727

2828
static const char *g_not_supported_error = "Not supported";
2929

30-
UDPSocket::UDPSocket(NativeSocket socket) : Socket(ProtocolUdp, true, true) {
30+
UDPSocket::UDPSocket(NativeSocket socket)
31+
: Socket(ProtocolUdp, /*should_close=*/true) {
3132
m_socket = socket;
3233
}
3334

34-
UDPSocket::UDPSocket(bool should_close, bool child_processes_inherit)
35-
: Socket(ProtocolUdp, should_close, child_processes_inherit) {}
35+
UDPSocket::UDPSocket(bool should_close) : Socket(ProtocolUdp, should_close) {}
3636

3737
size_t UDPSocket::Send(const void *buf, const size_t num_bytes) {
3838
return ::sendto(m_socket, static_cast<const char *>(buf), num_bytes, 0,
@@ -48,7 +48,7 @@ Status UDPSocket::Listen(llvm::StringRef name, int backlog) {
4848
}
4949

5050
llvm::Expected<std::unique_ptr<UDPSocket>>
51-
UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
51+
UDPSocket::CreateConnected(llvm::StringRef name) {
5252
std::unique_ptr<UDPSocket> socket;
5353

5454
Log *log = GetLog(LLDBLog::Connection);
@@ -84,9 +84,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
8484
for (struct addrinfo *service_info_ptr = service_info_list;
8585
service_info_ptr != nullptr;
8686
service_info_ptr = service_info_ptr->ai_next) {
87-
auto send_fd = CreateSocket(
88-
service_info_ptr->ai_family, service_info_ptr->ai_socktype,
89-
service_info_ptr->ai_protocol, child_processes_inherit, error);
87+
auto send_fd =
88+
CreateSocket(service_info_ptr->ai_family, service_info_ptr->ai_socktype,
89+
service_info_ptr->ai_protocol, error);
9090
if (error.Success()) {
9191
socket.reset(new UDPSocket(send_fd));
9292
socket->m_sockaddr = service_info_ptr;

‎lldb/source/Host/linux/AbstractSocket.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
using namespace lldb;
1414
using namespace lldb_private;
1515

16-
AbstractSocket::AbstractSocket(bool child_processes_inherit)
17-
: DomainSocket(ProtocolUnixAbstract, child_processes_inherit) {}
16+
AbstractSocket::AbstractSocket() : DomainSocket(ProtocolUnixAbstract) {}
1817

1918
size_t AbstractSocket::GetNameOffset() const { return 1; }
2019

0 commit comments

Comments
 (0)