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
- 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
- EMFILE         // Temporary: No free file descriptors left (per process)
- ENFILE         // Temporary: No free file descriptors left (system)
- ENOBUFS        // Temporary: No free socket memory left (system)
- ENOMEM         // Temporary: No free socket memory left (system)

Signed-off-by: Sven Göthel <[email protected]>
Change-Id: I939df8e8462d2c29d19214a9b5fb70ad37dc7500
  • Loading branch information
Sven Göthel committed Nov 6, 2024
1 parent a21425b commit 0a29005
Show file tree
Hide file tree
Showing 7 changed files with 491 additions and 49 deletions.
10 changes: 10 additions & 0 deletions net/NetUtil.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ class ProtocolHandlerInterface;
struct addrinfo;
struct sockaddr;

#if !MOBILEAPP && ENABLE_DEBUG
#define TEST_SERVERSOCKET_FAULT_INJECTION 1
#endif

namespace net
{

Expand All @@ -37,6 +41,12 @@ class DefaultValues

/// Maximum number of concurrent external TCP connections. Zero disables instrument.
size_t maxExtConnections;

#if TEST_SERVERSOCKET_FAULT_INJECTION
size_t testExternalStreamSocketCtorFailureInterval; // recoverable error injection
size_t testExternalServerSocketAcceptSimpleErrorInterval; // recoverable error injection
size_t testExternalServerSocketAcceptFatalErrorInterval; // fatal error injection
#endif
};
extern DefaultValues Defaults;

Expand Down
24 changes: 14 additions & 10 deletions net/ServerSocket.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ 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);

/// Socket ::accept() recoverable errors, includes isSocketAcceptRetryError.
/// See [accept(2)](https://www.man7.org/linux/man-pages/man2/accept.2.html) errors leading to retry
static bool isRecoverableAcceptError(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 +112,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 All @@ -132,6 +133,9 @@ class ServerSocket : public Socket
#endif
SocketPoll& _clientPoller;
std::shared_ptr<SocketFactory> _sockFactory;
#if TEST_SERVERSOCKET_FAULT_INJECTION
static std::atomic<size_t> TestExternalServerSocketAcceptCount;
#endif
};

#if !MOBILEAPP
Expand Down
194 changes: 155 additions & 39 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 @@ -67,8 +69,19 @@ std::unique_ptr<Watchdog> SocketPoll::PollWatchdog;

std::atomic<size_t> StreamSocket::ExternalConnectionCount = 0;

net::DefaultValues net::Defaults = { .inactivityTimeout = std::chrono::seconds(3600),
.maxExtConnections = 200000 /* arbitrary value to be resolved */ };
net::DefaultValues net::Defaults = { .inactivityTimeout = std::chrono::seconds(3600)
, .maxExtConnections = 0 /* unlimited default */
#if !MOBILEAPP && ENABLE_DEBUG
, .testExternalStreamSocketCtorFailureInterval = 0
, .testExternalServerSocketAcceptSimpleErrorInterval = 0
, .testExternalServerSocketAcceptFatalErrorInterval = 0
#endif
};

#if TEST_SERVERSOCKET_FAULT_INJECTION
std::atomic<size_t> StreamSocket::TestExternalStreamSocketCount = 0;
std::atomic<size_t> ServerSocket::TestExternalServerSocketAcceptCount = 0;
#endif

#define SOCKET_ABSTRACT_UNIX_NAME "0coolwsd-"

Expand Down Expand Up @@ -1145,63 +1158,156 @@ 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
case ENONET: // man accept(2): to be treated like EAGAIN
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;
}
}

bool ServerSocket::isRecoverableAcceptError(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
case ENONET: // man accept(2): to be treated like EAGAIN
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
case EMFILE: // Temporary: No free file descriptors left (per process)
case ENFILE: // Temporary: No free file descriptors left (system)
case ENOBUFS: // Temporary: No free socket memory left (system)
case ENOMEM: // Temporary: No free socket memory left (system)
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.
int rc;
#if !MOBILEAPP
assert(_type != Socket::Type::Unix);

struct sockaddr_in6 clientInfo;
socklen_t addrlen = sizeof(clientInfo);
const int rc = ::accept4(getFD(), (struct sockaddr *)&clientInfo, &addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
#if TEST_SERVERSOCKET_FAULT_INJECTION
const size_t acceptCount = ++TestExternalServerSocketAcceptCount;
bool injectedError = false;
if (net::Defaults.testExternalServerSocketAcceptSimpleErrorInterval > 0 &&
acceptCount % net::Defaults.testExternalServerSocketAcceptSimpleErrorInterval == 0)
{
rc = -1;
errno = EAGAIN; // recoverable error
injectedError = true;
LOG_DBG("Injecting recoverable accept failure: EAGAIN: " << acceptCount);
}
else if (net::Defaults.testExternalServerSocketAcceptFatalErrorInterval > 0 &&
acceptCount % net::Defaults.testExternalServerSocketAcceptFatalErrorInterval == 0)
{
rc = -1;
errno = EFAULT; // fatal error
injectedError = true;
LOG_DBG("Injecting fatal accept failure: EAGAIN: " << acceptCount);
}
else
#endif
rc = ::accept4(getFD(), (struct sockaddr *)&clientInfo, &addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
#else
const int rc = fakeSocketAccept4(getFD());
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 (!isRecoverableAcceptError(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;
}
// Unrecoverable error, forceExit instead of throw (closing `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`)
LOG_FTL(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
#if TEST_SERVERSOCKET_FAULT_INJECTION
if (injectedError)
throw std::runtime_error(std::string(messagePrefix) + std::to_string(cause) + ", " + std::strerror(cause) + ')');
else
{
struct sockaddr_in6 *ipv6 = &clientInfo;
inAddr = &(ipv6->sin6_addr);
type = Socket::Type::IPv6;
}
#endif
Util::forcedExit(EX_SOFTWARE);
}
else // Recoverable error, ignore to retry
LOG_DBG(messagePrefix << std::to_string(cause) << ", " << std::strerror(cause) << ')');
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);
#endif
return _socket;
}
return std::shared_ptr<Socket>(nullptr);
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);
_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 @@ -1247,11 +1353,21 @@ bool Socket::isLocal() const
std::shared_ptr<Socket> LocalServerSocket::accept()
{
const int rc = ::accept4(getFD(), nullptr, nullptr, SOCK_NONBLOCK | SOCK_CLOEXEC);
if (rc < 0)
{
const int cause = errno;
if( cause != EINTR && cause != EAGAIN && cause != EWOULDBLOCK )
{
// Unrecoverable error, throw, which will close `ServerSocket` and discard it from `AcceptPoll` during `SocketPoll::poll`
constexpr const char * messagePrefix = "Failed to accept. (errno: ";
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 @@ -1301,8 +1417,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
16 changes: 16 additions & 0 deletions net/Socket.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,19 @@ class StreamSocket : public Socket,
_readType(readType),
_inputProcessingEnabled(true)
{
#if TEST_SERVERSOCKET_FAULT_INJECTION
if (isExternalCountedConnection())
{
const size_t extStreamSocketCount = ++TestExternalStreamSocketCount;
if (net::Defaults.testExternalStreamSocketCtorFailureInterval > 0 &&
extStreamSocketCount% net::Defaults.testExternalStreamSocketCtorFailureInterval == 0)
{
LOG_DBG("Injecting recoverable StreamSocket ctor exception " << extStreamSocketCount
<< "/" << net::Defaults.testExternalStreamSocketCtorFailureInterval);
throw std::runtime_error("Test: StreamSocket exception: fd " + std::to_string(fd));
}
}
#endif
LOG_TRC("StreamSocket ctor");
if (isExternalCountedConnection())
++ExternalConnectionCount;
Expand Down Expand Up @@ -1767,6 +1780,9 @@ class StreamSocket : public Socket,

bool isExternalCountedConnection() const { return !_isClient && isIPType(); }
static std::atomic<size_t> ExternalConnectionCount; // accepted external TCP IPv4/IPv6 socket count
#if TEST_SERVERSOCKET_FAULT_INJECTION
static std::atomic<size_t> TestExternalStreamSocketCount;
#endif
};

enum class WSOpCode : unsigned char {
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 0a29005

Please sign in to comment.