Skip to content

Commit a8f061b

Browse files
author
Sven Göthel
committed
ServerSocket shall not throw (EOL) on accept returning nullptr; Simplify accept.
Have `ServerSocket::accept` determine whether to `Util::forcedExit(EX_SOFTWARE)` on unrecoverable errors: - Exit application if `::accept` returns `errno` other than EINTR || EAGAIN || EWOULDBLOCK. - tolerate `Socket` ctor exceptions - tolerate `::accept` EINTR || EAGAIN || EWOULDBLOCK - Note, the following are `no throw` - `::close` - `::inet_ntop` Also reject exceeding external connections right within `ServerSocket::accept`, avoiding creation of `Socket` objects - hence saving resources. Signed-off-by: Sven Göthel <[email protected]> Change-Id: I939df8e8462d2c29d19214a9b5fb70ad37dc7500
1 parent 5cd0c39 commit a8f061b

File tree

2 files changed

+66
-45
lines changed

2 files changed

+66
-45
lines changed

net/ServerSocket.hpp

+3-10
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,11 @@ class ServerSocket : public Socket
104104
if (events & POLLIN)
105105
{
106106
std::shared_ptr<Socket> clientSocket = accept();
107-
if (!clientSocket)
107+
if (clientSocket)
108108
{
109-
const std::string msg = "Failed to accept. (errno: ";
110-
throw std::runtime_error(msg + std::strerror(errno) + ')');
111-
}
112-
const size_t extConnCount = StreamSocket::getExternalConnectionCount();
113-
if( 0 == net::Defaults.maxExtConnections || extConnCount <= net::Defaults.maxExtConnections )
114-
{
115-
LOG_TRC("Accepted client #" << clientSocket->getFD());
109+
LOGA_TRC(Socket, "Accepted client #" << clientSocket->getFD() << ", " << *clientSocket);
116110
_clientPoller.insertNewSocket(std::move(clientSocket));
117-
} else
118-
LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: " << *clientSocket);
111+
}
119112
}
120113
}
121114

net/Socket.cpp

+63-35
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "TraceEvent.hpp"
1616
#include "Util.hpp"
1717

18+
#include <cerrno>
1819
#include <chrono>
1920
#include <cstring>
2021
#include <cctype>
@@ -26,6 +27,7 @@
2627
#include <cstdio>
2728
#include <string>
2829
#include <unistd.h>
30+
#include <sysexits.h>
2931
#include <sys/stat.h>
3032
#include <sys/types.h>
3133
#include <sys/un.h>
@@ -1161,50 +1163,66 @@ std::shared_ptr<Socket> ServerSocket::accept()
11611163
#else
11621164
const int rc = fakeSocketAccept4(getFD());
11631165
#endif
1164-
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
1165-
try
1166+
if (rc < 0)
11661167
{
1167-
// Create a socket object using the factory.
1168-
if (rc != -1)
1168+
const int cause = errno;
1169+
if( cause != EINTR && cause != EAGAIN && cause != EWOULDBLOCK )
11691170
{
1170-
#if !MOBILEAPP
1171-
char addrstr[INET6_ADDRSTRLEN];
1171+
// Unrecoverable error, throw, which will close `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`
1172+
constexpr const char * messagePrefix = "Failed to accept. (errno: ";
1173+
LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
1174+
Util::forcedExit(EX_SOFTWARE);
1175+
}
1176+
return nullptr;
1177+
}
1178+
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
11721179

1173-
Socket::Type type;
1174-
const void *inAddr;
1175-
if (clientInfo.sin6_family == AF_INET)
1176-
{
1177-
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
1178-
inAddr = &(ipv4->sin_addr);
1179-
type = Socket::Type::IPv4;
1180-
}
1181-
else
1182-
{
1183-
struct sockaddr_in6 *ipv6 = &clientInfo;
1184-
inAddr = &(ipv6->sin6_addr);
1185-
type = Socket::Type::IPv6;
1186-
}
1180+
#if !MOBILEAPP
1181+
char addrstr[INET6_ADDRSTRLEN];
11871182

1188-
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
1183+
Socket::Type type;
1184+
const void *inAddr;
1185+
if (clientInfo.sin6_family == AF_INET)
1186+
{
1187+
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
1188+
inAddr = &(ipv4->sin_addr);
1189+
type = Socket::Type::IPv4;
1190+
}
1191+
else
1192+
{
1193+
struct sockaddr_in6 *ipv6 = &clientInfo;
1194+
inAddr = &(ipv6->sin6_addr);
1195+
type = Socket::Type::IPv6;
1196+
}
1197+
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
11891198

1190-
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
1191-
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
1199+
const size_t extConnCount = StreamSocket::getExternalConnectionCount();
1200+
if( 0 < net::Defaults.maxExtConnections && extConnCount >= net::Defaults.maxExtConnections )
1201+
{
1202+
LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: #"
1203+
<< rc << " has family "
1204+
<< clientInfo.sin6_family << ", address " << addrstr << ":" << clientInfo.sin6_port);
1205+
::close(rc);
1206+
return nullptr;
1207+
}
1208+
try
1209+
{
1210+
// Create a socket object using the factory.
1211+
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
1212+
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
11921213

1193-
LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
1194-
<< clientInfo.sin6_family << ", " << *_socket);
1195-
#else
1196-
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
1197-
#endif
1198-
return _socket;
1199-
}
1200-
return std::shared_ptr<Socket>(nullptr);
1214+
LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
1215+
<< clientInfo.sin6_family << ", " << *_socket);
1216+
return _socket;
12011217
}
12021218
catch (const std::exception& ex)
12031219
{
12041220
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
12051221
}
1206-
12071222
return nullptr;
1223+
#else
1224+
return createSocketFromAccept(rc, Socket::Type::Unix);
1225+
#endif
12081226
}
12091227

12101228
#if !MOBILEAPP
@@ -1250,11 +1268,21 @@ bool Socket::isLocal() const
12501268
std::shared_ptr<Socket> LocalServerSocket::accept()
12511269
{
12521270
const int rc = ::accept4(getFD(), nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
1271+
if (rc < 0)
1272+
{
1273+
const int cause = errno;
1274+
if( cause != EINTR && cause != EAGAIN && cause != EWOULDBLOCK )
1275+
{
1276+
// Unrecoverable error, throw, which will close `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`
1277+
constexpr const char * messagePrefix = "Failed to accept. (errno: ";
1278+
LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
1279+
Util::forcedExit(EX_SOFTWARE);
1280+
}
1281+
return nullptr;
1282+
}
12531283
try
12541284
{
12551285
LOG_DBG("Accepted prisoner socket #" << rc << ", creating socket object.");
1256-
if (rc < 0)
1257-
return std::shared_ptr<Socket>(nullptr);
12581286

12591287
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
12601288
// Sanity check this incoming socket
@@ -1304,8 +1332,8 @@ std::shared_ptr<Socket> LocalServerSocket::accept()
13041332
catch (const std::exception& ex)
13051333
{
13061334
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
1307-
return std::shared_ptr<Socket>(nullptr);
13081335
}
1336+
return nullptr;
13091337
}
13101338

13111339
/// Returns true on success only.

0 commit comments

Comments
 (0)