Skip to content

Commit 2fbdc2a

Browse files
Sven Göthelmmeeks
Sven Göthel
authored and
mmeeks
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 an unrecoverable `errno` (see below) - tolerate `Socket` ctor exceptions - tolerate `::accept` recoverable errors (see below) - 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. Recoverable `::accept` `errno` values following [accept(2)](https://www.man7.org/linux/man-pages/man2/accept.2.html) are - EAGAIN // == EWOULDBLOCK - ENETDOWN // man accept(2): to be treated like EAGAIN - EPROTO // man accept(2): to be treated like EAGAIN - ENOPROTOOPT // man accept(2): to be treated like EAGAIN - EHOSTDOWN // man accept(2): to be treated like EAGAIN - ENONET // man accept(2): to be treated like EAGAIN (undef on BSD, Apple, ..) - EHOSTUNREACH // man accept(2): to be treated like EAGAIN - EOPNOTSUPP // man accept(2): to be treated like EAGAIN - ENETUNREACH // man accept(2): to be treated like EAGAIN - ECONNABORTED // OK - ETIMEDOUT // OK, should not happen due ppoll We also allow retrying on temporary EMFILE, ENFILE, ENOBUFS and ENOMEM errors, which may also be candidates for later fine grained control in case of resource degradation. Unit testing hooks for failure injection is provided via - UnitWSD::simulateExternalAcceptError() - UnitWSD::simulateExternalSocketCtorException() and utilized in - test/UnitServerSocketAcceptFailure1.cpp - test/UnitStreamSocketCtorFailure1.cpp Signed-off-by: Sven Göthel <[email protected]> Change-Id: I939df8e8462d2c29d19214a9b5fb70ad37dc7500
1 parent bb85a6c commit 2fbdc2a

6 files changed

+432
-45
lines changed

‎common/Unit.hpp

+9
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,15 @@ class UnitWSD : public UnitBase
503503
return false;
504504
}
505505

506+
// ---------------- ServerSocket hooks ----------------
507+
/// Simulate `::accept` errors for external `ServerSocket::accept`. Implement unrecoverable errors by throwing an exception.
508+
virtual bool simulateExternalAcceptError()
509+
{
510+
return false;
511+
}
512+
/// Simulate exceptions during `StreamSocket` constructor for external `ServerSocket::accept`.
513+
virtual void simulateExternalSocketCtorException(std::shared_ptr<Socket>& /*socket*/) { }
514+
506515
// ---------------- TileCache hooks ----------------
507516
/// Called before the lookupTile call returns. Should always be called to fire events.
508517
virtual void lookupTile(int part, int mode, int width, int height, int tilePosX, int tilePosY,

‎net/ServerSocket.hpp

+4-10
Original file line numberDiff line numberDiff line change
@@ -104,22 +104,16 @@ 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

122115
protected:
116+
bool isUnrecoverableAcceptError(const int cause);
123117
/// Create a Socket instance from the accepted socket FD.
124118
std::shared_ptr<Socket> createSocketFromAccept(int fd, Socket::Type type) const
125119
{

‎net/Socket.cpp

+93-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>
@@ -1144,63 +1146,115 @@ bool ServerSocket::bind([[maybe_unused]] Type type, [[maybe_unused]] int port)
11441146
#endif
11451147
}
11461148

1149+
bool ServerSocket::isUnrecoverableAcceptError(const int cause)
1150+
{
1151+
constexpr const char * messagePrefix = "Failed to accept. (errno: ";
1152+
switch(cause)
1153+
{
1154+
case EINTR:
1155+
case EAGAIN: // == EWOULDBLOCK
1156+
case ENETDOWN:
1157+
case EPROTO:
1158+
case ENOPROTOOPT:
1159+
case EHOSTDOWN:
1160+
#ifdef ENONET
1161+
case ENONET:
1162+
#endif
1163+
case EHOSTUNREACH:
1164+
case EOPNOTSUPP:
1165+
case ENETUNREACH:
1166+
case ECONNABORTED:
1167+
case ETIMEDOUT:
1168+
case EMFILE:
1169+
case ENFILE:
1170+
case ENOMEM:
1171+
case ENOBUFS:
1172+
{
1173+
LOG_DBG(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
1174+
return false;
1175+
}
1176+
default:
1177+
{
1178+
LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
1179+
return true;
1180+
}
1181+
}
1182+
}
1183+
11471184
std::shared_ptr<Socket> ServerSocket::accept()
11481185
{
11491186
// Accept a connection (if any) and set it to non-blocking.
11501187
// There still need the client's address to filter request from POST(call from REST) here.
11511188
#if !MOBILEAPP
11521189
assert(_type != Socket::Type::Unix);
11531190

1191+
UnitWSD* const unitWsd = UnitWSD::isUnitTesting() ? &UnitWSD::get() : nullptr;
1192+
if (unitWsd && unitWsd->simulateExternalAcceptError())
1193+
return nullptr; // Recoverable error, ignore to retry
1194+
11541195
struct sockaddr_in6 clientInfo;
11551196
socklen_t addrlen = sizeof(clientInfo);
11561197
const int rc = ::accept4(getFD(), (struct sockaddr *)&clientInfo, &addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
11571198
#else
11581199
const int rc = fakeSocketAccept4(getFD());
11591200
#endif
1160-
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
1161-
try
1201+
if (rc < 0)
11621202
{
1163-
// Create a socket object using the factory.
1164-
if (rc != -1)
1165-
{
1203+
if (isUnrecoverableAcceptError(errno))
1204+
Util::forcedExit(EX_SOFTWARE);
1205+
return nullptr;
1206+
}
1207+
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
1208+
11661209
#if !MOBILEAPP
1167-
char addrstr[INET6_ADDRSTRLEN];
1210+
char addrstr[INET6_ADDRSTRLEN];
11681211

1169-
Socket::Type type;
1170-
const void *inAddr;
1171-
if (clientInfo.sin6_family == AF_INET)
1172-
{
1173-
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
1174-
inAddr = &(ipv4->sin_addr);
1175-
type = Socket::Type::IPv4;
1176-
}
1177-
else
1178-
{
1179-
struct sockaddr_in6 *ipv6 = &clientInfo;
1180-
inAddr = &(ipv6->sin6_addr);
1181-
type = Socket::Type::IPv6;
1182-
}
1212+
Socket::Type type;
1213+
const void *inAddr;
1214+
if (clientInfo.sin6_family == AF_INET)
1215+
{
1216+
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
1217+
inAddr = &(ipv4->sin_addr);
1218+
type = Socket::Type::IPv4;
1219+
}
1220+
else
1221+
{
1222+
struct sockaddr_in6 *ipv6 = &clientInfo;
1223+
inAddr = &(ipv6->sin6_addr);
1224+
type = Socket::Type::IPv6;
1225+
}
1226+
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
11831227

1184-
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
1228+
const size_t extConnCount = StreamSocket::getExternalConnectionCount();
1229+
if (net::Defaults.maxExtConnections > 0 && extConnCount >= net::Defaults.maxExtConnections)
1230+
{
1231+
LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: #"
1232+
<< rc << " has family "
1233+
<< clientInfo.sin6_family << ", address " << addrstr << ":" << clientInfo.sin6_port);
1234+
::close(rc);
1235+
return nullptr;
1236+
}
1237+
try
1238+
{
1239+
// Create a socket object using the factory.
1240+
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
1241+
if (unitWsd)
1242+
unitWsd->simulateExternalSocketCtorException(_socket);
11851243

1186-
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
1187-
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
1244+
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
11881245

1189-
LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
1190-
<< clientInfo.sin6_family << ", " << *_socket);
1191-
#else
1192-
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
1193-
#endif
1194-
return _socket;
1195-
}
1196-
return std::shared_ptr<Socket>(nullptr);
1246+
LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
1247+
<< clientInfo.sin6_family << ", " << *_socket);
1248+
return _socket;
11971249
}
11981250
catch (const std::exception& ex)
11991251
{
12001252
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
12011253
}
1202-
12031254
return nullptr;
1255+
#else
1256+
return createSocketFromAccept(rc, Socket::Type::Unix);
1257+
#endif
12041258
}
12051259

12061260
#if !MOBILEAPP
@@ -1246,11 +1300,15 @@ bool Socket::isLocal() const
12461300
std::shared_ptr<Socket> LocalServerSocket::accept()
12471301
{
12481302
const int rc = ::accept4(getFD(), nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
1303+
if (rc < 0)
1304+
{
1305+
if (isUnrecoverableAcceptError(errno))
1306+
Util::forcedExit(EX_SOFTWARE);
1307+
return nullptr;
1308+
}
12491309
try
12501310
{
12511311
LOG_DBG("Accepted prisoner socket #" << rc << ", creating socket object.");
1252-
if (rc < 0)
1253-
return std::shared_ptr<Socket>(nullptr);
12541312

12551313
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
12561314
// Sanity check this incoming socket
@@ -1300,8 +1358,8 @@ std::shared_ptr<Socket> LocalServerSocket::accept()
13001358
catch (const std::exception& ex)
13011359
{
13021360
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
1303-
return std::shared_ptr<Socket>(nullptr);
13041361
}
1362+
return nullptr;
13051363
}
13061364

13071365
/// Returns true on success only.

‎test/Makefile.am

+6
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ all_la_unit_tests = \
9494
unit-timeout_inactive.la \
9595
unit-timeout_conn.la \
9696
unit-timeout_none.la \
97+
unit-serversock_accept1.la \
98+
unit-streamsock_ctor1.la \
9799
unit-base.la
98100
# unit-admin.la
99101
# unit-tilecache.la # Empty test.
@@ -232,6 +234,10 @@ unit_timeout_conn_la_SOURCES = UnitTimeoutConnections.cpp
232234
unit_timeout_conn_la_LIBADD = $(CPPUNIT_LIBS)
233235
unit_timeout_none_la_SOURCES = UnitTimeoutNone.cpp
234236
unit_timeout_none_la_LIBADD = $(CPPUNIT_LIBS)
237+
unit_serversock_accept1_la_SOURCES = UnitServerSocketAcceptFailure1.cpp
238+
unit_serversock_accept1_la_LIBADD = $(CPPUNIT_LIBS)
239+
unit_streamsock_ctor1_la_SOURCES = UnitStreamSocketCtorFailure1.cpp
240+
unit_streamsock_ctor1_la_LIBADD = $(CPPUNIT_LIBS)
235241
unit_prefork_la_SOURCES = UnitPrefork.cpp
236242
unit_prefork_la_LIBADD = $(CPPUNIT_LIBS)
237243
unit_storage_la_SOURCES = UnitStorage.cpp

0 commit comments

Comments
 (0)