Skip to content

Commit

Permalink
ServerSocket shall not throw (EOL) on accept returning nullptr; Simpl…
Browse files Browse the repository at this point in the history 
…ify 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

May consider handling temporary EMFILE, ENFILE, ENOBUFS and ENOMEM errors?

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
  • Loading branch information
Sven Göthel committed Nov 7, 2024
1 parent 7844e02 commit 36dad03
Show file tree
Hide file tree
Showing 6 changed files with 451 additions and 45 deletions.
9 changes: 9 additions & 0 deletions common/Unit.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,15 @@ class UnitWSD : public UnitBase
return false;
}

// ---------------- ServerSocket hooks ----------------
/// Simulate `::accept` errors for external `ServerSocket::accept`. Implement unrecoverable errors by throwing an exception.
virtual bool simulateExternalAcceptError()
{
return false;
}
/// Simulate exceptions during `StreamSocket` constructor for external `ServerSocket::accept`.
virtual void simulateExternalSocketCtorException(std::shared_ptr<Socket>& /*socket*/) { }

// ---------------- TileCache hooks ----------------
/// Called before the lookupTile call returns. Should always be called to fire events.
virtual void lookupTile(int part, int mode, int width, int height, int tilePosX, int tilePosY,
Expand Down
17 changes: 7 additions & 10 deletions net/ServerSocket.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ class SocketFactory
class ServerSocket : public Socket
{
public:
/// Socket ::accept() recoverable retry errors.
/// See [accept(2)](https://www.man7.org/linux/man-pages/man2/accept.2.html) errors leading to retry
static bool isRetryAcceptError(const int cause);

ServerSocket(Socket::Type type,
std::chrono::steady_clock::time_point creationTime,
SocketPoll& clientPoller, std::shared_ptr<SocketFactory> sockFactory) :
Expand Down Expand Up @@ -104,18 +108,11 @@ class ServerSocket : public Socket
if (events & POLLIN)
{
std::shared_ptr<Socket> clientSocket = accept();
if (!clientSocket)
{
const std::string msg = "Failed to accept. (errno: ";
throw std::runtime_error(msg + std::strerror(errno) + ')');
}
const size_t extConnCount = StreamSocket::getExternalConnectionCount();
if( 0 == net::Defaults.maxExtConnections || extConnCount <= net::Defaults.maxExtConnections )
if (clientSocket)
{
LOG_TRC("Accepted client #" << clientSocket->getFD());
LOGA_TRC(Socket, "Accepted client #" << clientSocket->getFD() << ", " << *clientSocket);
_clientPoller.insertNewSocket(std::move(clientSocket));
} else
LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: " << *clientSocket);
}
}
}

Expand Down
144 changes: 109 additions & 35 deletions net/Socket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "TraceEvent.hpp"
#include "Util.hpp"

#include <cerrno>
#include <chrono>
#include <cstring>
#include <cctype>
Expand All @@ -26,6 +27,7 @@
#include <cstdio>
#include <string>
#include <unistd.h>
#include <sysexits.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/un.h>
Expand Down Expand Up @@ -1149,63 +1151,119 @@ bool ServerSocket::bind([[maybe_unused]] Type type, [[maybe_unused]] int port)
#endif
}

bool ServerSocket::isRetryAcceptError(const int cause)
{
switch(cause)
{
case EINTR:
case EAGAIN: // == EWOULDBLOCK
case ENETDOWN: // man accept(2): to be treated like EAGAIN
case EPROTO: // man accept(2): to be treated like EAGAIN
case ENOPROTOOPT: // man accept(2): to be treated like EAGAIN
case EHOSTDOWN: // man accept(2): to be treated like EAGAIN
#ifdef ENONET
case ENONET: // man accept(2): to be treated like EAGAIN (undef on BSD, APPLE, ..)
#endif
case EHOSTUNREACH: // man accept(2): to be treated like EAGAIN
case EOPNOTSUPP: // man accept(2): to be treated like EAGAIN
case ENETUNREACH: // man accept(2): to be treated like EAGAIN
case ECONNABORTED: // OK
case ETIMEDOUT: // OK, should not happen due ppoll
return true;
default:
return false;
}
}

std::shared_ptr<Socket> ServerSocket::accept()
{
// Accept a connection (if any) and set it to non-blocking.
// There still need the client's address to filter request from POST(call from REST) here.
#if !MOBILEAPP
assert(_type != Socket::Type::Unix);

#if ENABLE_DEBUG
UnitWSD* const unitWsd = UnitWSD::isUnitTesting() ? &UnitWSD::get() : nullptr;
if (unitWsd && unitWsd->simulateExternalAcceptError())
return nullptr; // Recoverable error, ignore to retry
#endif

struct sockaddr_in6 clientInfo;
socklen_t addrlen = sizeof(clientInfo);
const int rc = ::accept4(getFD(), (struct sockaddr *)&clientInfo, &addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
#else
const int rc = fakeSocketAccept4(getFD());
#endif
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");
try
if (rc < 0)
{
// Create a socket object using the factory.
if (rc != -1)
const int cause = errno;
constexpr const char * messagePrefix = "Failed to accept. (errno: ";
if (isRetryAcceptError(cause))
{
#if !MOBILEAPP
char addrstr[INET6_ADDRSTRLEN];

Socket::Type type;
const void *inAddr;
if (clientInfo.sin6_family == AF_INET)
{
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
inAddr = &(ipv4->sin_addr);
type = Socket::Type::IPv4;
}
else
{
struct sockaddr_in6 *ipv6 = &clientInfo;
inAddr = &(ipv6->sin6_addr);
type = Socket::Type::IPv6;
}
// Recoverable error, ignore to retry
LOG_DBG(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
}
else
{
// Unrecoverable error, forceExit instead of throw (closing `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`)
// May allow recovery from temporary EMFILE, ENFILE, ENOBUFS and ENOMEM errors?
LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
Util::forcedExit(EX_SOFTWARE);
}
return nullptr;
}
LOG_TRC("Accepted socket #" << rc << ", creating socket object.");

std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
#if !MOBILEAPP
char addrstr[INET6_ADDRSTRLEN];

::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));
_socket->setClientAddress(addrstr, clientInfo.sin6_port);
Socket::Type type;
const void *inAddr;
if (clientInfo.sin6_family == AF_INET)
{
struct sockaddr_in *ipv4 = (struct sockaddr_in *)&clientInfo;
inAddr = &(ipv4->sin_addr);
type = Socket::Type::IPv4;
}
else
{
struct sockaddr_in6 *ipv6 = &clientInfo;
inAddr = &(ipv6->sin6_addr);
type = Socket::Type::IPv6;
}
::inet_ntop(clientInfo.sin6_family, inAddr, addrstr, sizeof(addrstr));

LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
<< clientInfo.sin6_family << ", " << *_socket);
#else
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
const size_t extConnCount = StreamSocket::getExternalConnectionCount();
if (0 < net::Defaults.maxExtConnections && extConnCount >= net::Defaults.maxExtConnections)
{
LOG_WRN("Limiter rejected extConn[" << extConnCount << "/" << net::Defaults.maxExtConnections << "]: #"
<< rc << " has family "
<< clientInfo.sin6_family << ", address " << addrstr << ":" << clientInfo.sin6_port);
::close(rc);
return nullptr;
}
try
{
// Create a socket object using the factory.
std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, type);
#if ENABLE_DEBUG
if (unitWsd)
unitWsd->simulateExternalSocketCtorException(_socket);
#endif
return _socket;
}
return std::shared_ptr<Socket>(nullptr);
_socket->setClientAddress(addrstr, clientInfo.sin6_port);

LOG_TRC("Accepted socket #" << _socket->getFD() << " has family "
<< clientInfo.sin6_family << ", " << *_socket);
return _socket;
}
catch (const std::exception& ex)
{
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
}

return nullptr;
#else
return createSocketFromAccept(rc, Socket::Type::Unix);
#endif
}

#if !MOBILEAPP
Expand Down Expand Up @@ -1251,11 +1309,27 @@ bool Socket::isLocal() const
std::shared_ptr<Socket> LocalServerSocket::accept()
{
const int rc = ::accept4(getFD(), nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
if (rc < 0)
{
constexpr const char * messagePrefix = "Failed to accept. (errno: ";
const int cause = errno;
if (isRetryAcceptError(cause))
{
// Recoverable error, ignore to retry
LOG_DBG(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
}
else
{
// Unrecoverable error, throw, which will close `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`
// May allow recovery from temporary EMFILE, ENFILE, ENOBUFS and ENOMEM errors?
LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
Util::forcedExit(EX_SOFTWARE);
}
return nullptr;
}
try
{
LOG_DBG("Accepted prisoner socket #" << rc << ", creating socket object.");
if (rc < 0)
return std::shared_ptr<Socket>(nullptr);

std::shared_ptr<Socket> _socket = createSocketFromAccept(rc, Socket::Type::Unix);
// Sanity check this incoming socket
Expand Down Expand Up @@ -1305,8 +1379,8 @@ std::shared_ptr<Socket> LocalServerSocket::accept()
catch (const std::exception& ex)
{
LOG_ERR("Failed to create client socket #" << rc << ". Error: " << ex.what());
return std::shared_ptr<Socket>(nullptr);
}
return nullptr;
}

/// Returns true on success only.
Expand Down
6 changes: 6 additions & 0 deletions test/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ all_la_unit_tests = \
unit-timeout_inactive.la \
unit-timeout_conn.la \
unit-timeout_none.la \
unit-serversock_accept1.la \
unit-streamsock_ctor1.la \
unit-base.la
# unit-admin.la
# unit-tilecache.la # Empty test.
Expand Down Expand Up @@ -232,6 +234,10 @@ unit_timeout_conn_la_SOURCES = UnitTimeoutConnections.cpp
unit_timeout_conn_la_LIBADD = $(CPPUNIT_LIBS)
unit_timeout_none_la_SOURCES = UnitTimeoutNone.cpp
unit_timeout_none_la_LIBADD = $(CPPUNIT_LIBS)
unit_serversock_accept1_la_SOURCES = UnitServerSocketAcceptFailure1.cpp
unit_serversock_accept1_la_LIBADD = $(CPPUNIT_LIBS)
unit_streamsock_ctor1_la_SOURCES = UnitStreamSocketCtorFailure1.cpp
unit_streamsock_ctor1_la_LIBADD = $(CPPUNIT_LIBS)
unit_prefork_la_SOURCES = UnitPrefork.cpp
unit_prefork_la_LIBADD = $(CPPUNIT_LIBS)
unit_storage_la_SOURCES = UnitStorage.cpp
Expand Down
Loading

0 comments on commit 36dad03

Please sign in to comment.