Skip to content

Commit 4dbee2e

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 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 558a4fd commit 4dbee2e

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>
@@ -1149,63 +1151,115 @@ bool ServerSocket::bind([[maybe_unused]] Type type, [[maybe_unused]] int port)
11491151
#endif
11501152
}
11511153

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

1196+
UnitWSD* const unitWsd = UnitWSD::isUnitTesting() ? &UnitWSD::get() : nullptr;
1197+
if (unitWsd && unitWsd->simulateExternalAcceptError())
1198+
return nullptr; // Recoverable error, ignore to retry
1199+
11591200
struct sockaddr_in6 clientInfo;
11601201
socklen_t addrlen = sizeof(clientInfo);
11611202
const int rc = ::accept4(getFD(), (struct sockaddr *)&clientInfo, &addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
11621203
#else
11631204
const int rc = fakeSocketAccept4(getFD());
11641205
#endif
1165-
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
1166-
try
1206+
if (rc < 0)
11671207
{
1168-
// Create a socket object using the factory.
1169-
if (rc != -1)
1170-
{
1208+
if (isUnrecoverableAcceptError(errno))
1209+
Util::forcedExit(EX_SOFTWARE);
1210+
return nullptr;
1211+
}
1212+
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
1213+
11711214
#if !MOBILEAPP
1172-
char addrstr[INET6_ADDRSTRLEN];
1215+
char addrstr[INET6_ADDRSTRLEN];
11731216

1174-
Socket::Type type;
1175-
const void *inAddr;
1176-
if (clientInfo.sin6_family == AF_INET)
1177-
{
1178-
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
1179-
inAddr = &(ipv4->sin_addr);
1180-
type = Socket::Type::IPv4;
1181-
}
1182-
else
1183-
{
1184-
struct sockaddr_in6 *ipv6 = &clientInfo;
1185-
inAddr = &(ipv6->sin6_addr);
1186-
type = Socket::Type::IPv6;
1187-
}
1217+
Socket::Type type;
1218+
const void *inAddr;
1219+
if (clientInfo.sin6_family == AF_INET)
1220+
{
1221+
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
1222+
inAddr = &(ipv4->sin_addr);
1223+
type = Socket::Type::IPv4;
1224+
}
1225+
else
1226+
{
1227+
struct sockaddr_in6 *ipv6 = &clientInfo;
1228+
inAddr = &(ipv6->sin6_addr);
1229+
type = Socket::Type::IPv6;
1230+
}
1231+
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
11881232

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

1191-
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
1192-
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
1249+
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
11931250

1194-
LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
1195-
<< clientInfo.sin6_family << ", " << *_socket);
1196-
#else
1197-
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
1198-
#endif
1199-
return _socket;
1200-
}
1201-
return std::shared_ptr<Socket>(nullptr);
1251+
LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
1252+
<< clientInfo.sin6_family << ", " << *_socket);
1253+
return _socket;
12021254
}
12031255
catch (const std::exception& ex)
12041256
{
12051257
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
12061258
}
1207-
12081259
return nullptr;
1260+
#else
1261+
return createSocketFromAccept(rc, Socket::Type::Unix);
1262+
#endif
12091263
}
12101264

12111265
#if !MOBILEAPP
@@ -1251,11 +1305,15 @@ bool Socket::isLocal() const
12511305
std::shared_ptr<Socket> LocalServerSocket::accept()
12521306
{
12531307
const int rc = ::accept4(getFD(), nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
1308+
if (rc < 0)
1309+
{
1310+
if (isUnrecoverableAcceptError(errno))
1311+
Util::forcedExit(EX_SOFTWARE);
1312+
return nullptr;
1313+
}
12541314
try
12551315
{
12561316
LOG_DBG("Accepted prisoner socket #" << rc << ", creating socket object.");
1257-
if (rc < 0)
1258-
return std::shared_ptr<Socket>(nullptr);
12591317

12601318
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
12611319
// Sanity check this incoming socket
@@ -1305,8 +1363,8 @@ std::shared_ptr<Socket> LocalServerSocket::accept()
13051363
catch (const std::exception& ex)
13061364
{
13071365
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
1308-
return std::shared_ptr<Socket>(nullptr);
13091366
}
1367+
return nullptr;
13101368
}
13111369

13121370
/// 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)