net: SocketServer keeps accepting after running out of file descriptors

CHANGELOG.md

@@ -430,6 +430,18 @@ API surface).
 
 ### Fixed
 
+- **`net::SocketServer` goes on accepting after the process runs out of file
+  descriptors.** Any `accept(2)` failure but `EINTR` and `EAGAIN` ended the
+  accept thread, so a single `EMFILE` stopped the server accepting for good,
+  while the port stayed bound and clients hung in their Upgrade read. The loop
+  now backs off on exhaustion (`EMFILE`, `ENFILE`, `ENOBUFS`, `ENOMEM`), from
+  10 ms to at most 1 s, and serves again once descriptors are free; `close()`
+  still ends it at once. A connection that failed while pending — the network
+  errors Linux `accept(2)` reports for it, and a firewall's `EPERM` — is
+  skipped like `EAGAIN`. The loop ends only for a listener that cannot accept,
+  and now logs why. `TcpSocket::tryAccept()` throws `std::system_error`
+  carrying the `errno`, a `std::runtime_error` as before.
+
 - **An installed `qt_forms` component compiles.** `forms_controller_core.hpp`
   includes `morph/qt/qt_executor.hpp`, which was installed only by the `qt`
   component (`MORPH_BUILD_QT`, which needs Qt WebSockets), so an install built

docs/spec/core/backend.md

@@ -2012,6 +2012,37 @@ ready connection with `TcpSocket::tryAccept()`, which answers `std::nullopt`
 rather than parking when a readiness report has gone stale. `close()` signals
 the wakeup before `join()`, which is what ends the loop.
 
+**A failed accept ends the loop only when the listener cannot accept.**
+`accept(2)` answers for three different things, and `kAcceptErrors` in
+`detail/tcp_socket.hpp` sorts each `errno` into one of them:
+
+- **The pending connection failed** (`AcceptFailure::NoConnection`). Linux
+  `accept(2)` hands back a network error already pending on the new connection
+  — `ENETDOWN`, `EPROTO`, `ENOPROTOOPT`, `EHOSTDOWN`, `ENONET`, `EHOSTUNREACH`,
+  `EOPNOTSUPP`, `ENETUNREACH` — and says to treat them like `EAGAIN`; `EPERM` is
+  a firewall rule refusing that one connection. `tryAccept()` answers these
+  `std::nullopt`, exactly as for a stale readiness report, so the loop waits
+  for the next connection through the branch it already takes for `EAGAIN`.
+  `ECONNABORTED` is not among them: a connection reset while pending is still
+  accepted on Linux, and its reset reaches the first `recvSome()` as an
+  orderly close.
+- **The process or the system ran out of something** (`Exhausted`: `EMFILE`,
+  `ENFILE`, `ENOBUFS`, `ENOMEM`). The same `accept` succeeds once it is
+  released, so the loop waits and tries again: 10 ms after the first failure,
+  doubling to 1 s, and back to none once an accept stops failing. The wait is a
+  `poll()` on the wakeup alone — the listener stays readable while the
+  connection that failed is queued, so polling it too would spin — which is
+  also why `close()` still ends a loop that is waiting. The first failure of a
+  run is logged at warn.
+- **The listener cannot accept** (`ListenerUnusable`: `EBADF`, `ENOTSOCK`,
+  `EINVAL`, `EFAULT`, and any `errno` no row names). The loop ends and logs
+  why at warn, since the port stays bound until `close()` and a client would
+  otherwise only hang in its Upgrade read.
+
+`tests/net/test_socket_server.cpp` exhausts `RLIMIT_NOFILE` under a running
+server, restores it, and requires a connection that queued in between to
+complete its handshake.
+
 **The listener's non-blocking mode stops at the listener.**
 `TcpSocket`'s fd-adopting constructor clears `O_NONBLOCK` on every descriptor it
 takes ownership of, so a connection from `accept()` or `tryAccept()` is always

include/morph/net/detail/tcp_socket.hpp

@@ -9,6 +9,7 @@
 #include <unistd.h>
 
 #include <algorithm>
+#include <array>
 #include <cerrno>
 #include <chrono>
 #include <cstddef>
@@ -31,6 +32,76 @@
 
 namespace morph::net::detail {
 
+/// @brief What a failed `accept(2)` says, for a loop that keeps accepting.
+enum class AcceptFailure : std::uint8_t {
+    /// The connection that was pending failed before it was taken. Nothing is
+    /// wrong with the listener: wait for the next one, as after `EAGAIN`.
+    NoConnection,
+    /// The process or the system is out of something `accept(2)` needs. The
+    /// same call succeeds once it is released, so the loop waits and retries.
+    Exhausted,
+    /// The listener itself cannot accept, now or later.
+    ListenerUnusable,
+};
+
+/// @brief One `errno` that `accept(2)` can answer, and what it says.
+struct AcceptErrorRow {
+    int err;                ///< The `errno` value.
+    AcceptFailure failure;  ///< What an accept loop does about it.
+};
+
+/// @brief The `errno` values `TcpSocket::tryAccept()` and
+///        `SocketServer`'s accept loop tell apart.
+///
+/// `NoConnection` is Linux `accept(2)`'s own instruction: it hands back a
+/// network error already pending on the new connection from `accept()`
+/// itself, and says to treat the TCP/IP ones like `EAGAIN` by retrying.
+/// `EPERM` is Linux refusing that one connection by firewall rule.
+/// `EOPNOTSUPP` is on the pending-error list too; it would also be the answer
+/// for a listener that is not `SOCK_STREAM`, which `listen()` never creates.
+///
+/// `ECONNABORTED` has no row. A connection reset while pending is still
+/// accepted on Linux, and the reset surfaces on the first `recvSome()`, which
+/// reads it as an orderly close; the `accept()` comment carries the
+/// measurement.
+///
+/// An `errno` no row names is `ListenerUnusable` (see `acceptFailureOf`).
+inline constexpr std::array kAcceptErrors{
+    AcceptErrorRow{.err = ENETDOWN, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = EPROTO, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = ENOPROTOOPT, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = EHOSTDOWN, .failure = AcceptFailure::NoConnection},
+#ifdef ENONET
+    AcceptErrorRow{.err = ENONET, .failure = AcceptFailure::NoConnection},
+#endif
+    AcceptErrorRow{.err = EHOSTUNREACH, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = EOPNOTSUPP, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = ENETUNREACH, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = EPERM, .failure = AcceptFailure::NoConnection},
+    AcceptErrorRow{.err = EMFILE, .failure = AcceptFailure::Exhausted},
+    AcceptErrorRow{.err = ENFILE, .failure = AcceptFailure::Exhausted},
+    AcceptErrorRow{.err = ENOBUFS, .failure = AcceptFailure::Exhausted},
+    AcceptErrorRow{.err = ENOMEM, .failure = AcceptFailure::Exhausted},
+    AcceptErrorRow{.err = EBADF, .failure = AcceptFailure::ListenerUnusable},
+    AcceptErrorRow{.err = ENOTSOCK, .failure = AcceptFailure::ListenerUnusable},
+    AcceptErrorRow{.err = EINVAL, .failure = AcceptFailure::ListenerUnusable},
+    AcceptErrorRow{.err = EFAULT, .failure = AcceptFailure::ListenerUnusable},
+};
+
+/// @brief Looks up @p err in `kAcceptErrors`.
+/// @param err An `errno` value `accept(2)` answered.
+/// @return Its row's `AcceptFailure`; `ListenerUnusable` for a value no row
+///         names, so an answer nobody classified ends an accept loop rather
+///         than being retried as though it passed.
+[[nodiscard]] inline AcceptFailure acceptFailureOf(int err) noexcept {
+    for (auto const& row : kAcceptErrors) {
+        if (row.err == err) {
+            return row.failure;
+        }
+    }
+    return AcceptFailure::ListenerUnusable;
+}
+
 /// @brief RAII wrapper around a POSIX (BSD sockets) TCP file descriptor.
 ///
 /// Linux/macOS only today — see `docs/spec/core/backend.md`'s `morph::net`
@@ -333,10 +404,13 @@ class TcpSocket {
     /// macOS/BSD's inheritance of the listener's flag reaching `recvSome()`
     /// Any rewrite of this function has to keep going through that
     /// constructor, or do the reset itself.
-    /// @return The accepted `TcpSocket`, or `std::nullopt` when no connection
-    ///         was pending — a readiness report that went stale before the
-    ///         `accept`, which the caller answers by waiting again.
-    /// @throws std::runtime_error if `::accept` fails for any other reason.
+    /// @return The accepted `TcpSocket`, or `std::nullopt` when there is no
+    ///         connection to take: none was pending — a readiness report that
+    ///         went stale before the `accept` — or the pending one failed
+    ///         first (`AcceptFailure::NoConnection` in `kAcceptErrors`). The
+    ///         caller answers both by waiting again.
+    /// @throws std::system_error carrying the `errno`, if `::accept` fails for
+    ///         any other reason; `acceptFailureOf()` says what it means.
     // Non-const to match accept(): taking a connection consumes it from the
     // listener's queue, which is state this object owns.
     // NOLINTNEXTLINE(readability-make-member-function-const)
@@ -350,10 +424,10 @@ class TcpSocket {
             if (err == EINTR) {  // retried for the same reason accept() retries it
                 continue;
             }
-            if (wouldBlock(err)) {
+            if (wouldBlock(err) || acceptFailureOf(err) == AcceptFailure::NoConnection) {
                 return std::nullopt;
             }
-            throw std::runtime_error("TcpSocket::tryAccept: " + errnoMessage(err));
+            throw std::system_error(err, std::system_category(), "TcpSocket::tryAccept");
         }
     }
 

include/morph/net/socket_server.hpp

@@ -3,6 +3,7 @@
 #pragma once
 #include <poll.h>
 
+#include <algorithm>
 #include <array>
 #include <atomic>
 #include <cerrno>
@@ -11,10 +12,12 @@
 #include <cstdint>
 #include <exception>
 #include <memory>
+#include <morph/core/logger.hpp>
 #include <morph/core/remote.hpp>
 #include <mutex>
 #include <optional>
 #include <string>
+#include <system_error>
 #include <thread>
 #include <vector>
 
@@ -256,14 +259,75 @@ class SocketServer {
         }
     };
 
+    /// First wait after the process runs out of something `accept(2)` needs;
+    /// each further failure in the same run doubles it.
+    static constexpr std::chrono::milliseconds kAcceptBackoffFirst{10};
+    /// The longest wait: how late a recovered process takes its next
+    /// connection. `close()` is not held by it -- the wait ends on the wakeup.
+    static constexpr std::chrono::milliseconds kAcceptBackoffMax{1000};
+
+    /// Waits out an accept backoff on the wakeup alone: the listener stays
+    /// readable while the connection that failed is still queued, so polling
+    /// it too would end the wait at once and spin.
+    /// @return `false` when the loop must end instead: `close()` signalled the
+    ///         wakeup, or `poll()` itself failed.
+    static bool waitOutBackoff(::core::platform::NativeHandle wakeupHandle, std::chrono::milliseconds backoff) {
+        pollfd wakeupOnly{};
+        wakeupOnly.fd = wakeupHandle;
+        wakeupOnly.events = POLLIN;
+        int const ready = ::poll(&wakeupOnly, 1, static_cast<int>(backoff.count()));
+        if (ready > 0) {
+            return false;  // close() signalled the wakeup
+        }
+        return ready == 0 || errno == EINTR;
+    }
+
+    /// Takes the pending connection, if there is one, into @p clientSocket,
+    /// and sorts a failure by `acceptFailureOf`: running out of something sets
+    /// @p backoff, anything else ends the loop.
+    /// @return `false` when the loop must end.
+    bool acceptOrBackOff(std::optional<::morph::net::detail::TcpSocket>& clientSocket,
+                         std::chrono::milliseconds& backoff) {
+        try {
+            clientSocket = _listenSocket.tryAccept();
+        } catch (const std::system_error& failure) {
+            if (::morph::net::detail::acceptFailureOf(failure.code().value()) ==
+                ::morph::net::detail::AcceptFailure::Exhausted) {
+                // Said once per run of failures, not once per doubling.
+                if (backoff.count() == 0) {
+                    ::morph::log::logWarn(std::string{"[net::SocketServer] accept failed, backing off: "} +
+                                          failure.what());
+                }
+                backoff = backoff.count() == 0 ? kAcceptBackoffFirst : std::min(backoff * 2, kAcceptBackoffMax);
+                return true;
+            }
+            // Not a condition of the moment: a loop still polling this
+            // listener would get the same answer forever. Said, because the
+            // port stays bound and a client would otherwise simply hang.
+            ::morph::log::logWarn(std::string{"[net::SocketServer] accept loop stopped: "} + failure.what());
+            return false;
+        } catch (const std::exception& failure) {
+            ::morph::log::logWarn(std::string{"[net::SocketServer] accept loop stopped: "} + failure.what());
+            return false;
+        }
+        backoff = {};
+        return true;
+    }
+
     void acceptLoop() {
         // listen() opens the wakeup before it starts this loop, and nothing
         // resets it while the loop runs.
         if (!_wakeup.has_value()) {
             return;
         }
         auto const wakeupHandle = _wakeup->nativeHandle();
+        // Zero, or how long to wait before the next accept after the process
+        // ran out of something accept(2) needs.
+        std::chrono::milliseconds backoff{};
         for (;;) {
+            if (backoff.count() > 0 && !waitOutBackoff(wakeupHandle, backoff)) {
+                return;
+            }
             std::array<pollfd, kPollFdCount> fds{};
             pollfd& listenPfd = fds.front();
             pollfd& wakeupPfd = fds.back();
@@ -286,13 +350,11 @@ class SocketServer {
                 continue;
             }
             std::optional<::morph::net::detail::TcpSocket> clientSocket;
-            try {
-                clientSocket = _listenSocket.tryAccept();
-            } catch (const std::exception&) {
-                return;  // the listener is unusable (server closing)
+            if (!acceptOrBackOff(clientSocket, backoff)) {
+                return;
             }
             if (!clientSocket) {
-                continue;  // readiness went stale before the accept; wait again
+                continue;  // nothing to take this time, or backing off; wait again
             }
             if (_closing.load()) {
                 return;

scripts/branch_partial_allowlist.json

@@ -127,22 +127,16 @@
     },
     {
       "file": "include/morph/net/socket_server.hpp",
-      "line": 210,
+      "line": 213,
       "source": "if (t.joinable()) {",
       "reason": "Unreachable by construction (net audit, `socket_server.hpp` finding #6, verified against a `close()` whose whole body is serialized under `_closeMtx`). `_clientThreads` has exactly one push site (`acceptLoop()`, always a freshly-constructed, running `std::thread`) and this loop is the only place any entry is ever joined or detached. With `close()`'s entire body serialized by `_closeMtx`, only one caller's `close()` can ever reach this loop: a second, later call observes `wasAlreadyClosing == true` and `!_acceptThread.joinable()` (already joined by the winner) and takes the early return above, before ever reaching the client-thread swap-and-join section this line is in. So every `std::thread` this loop iterates over is a fresh entry pushed by `acceptLoop()` that nothing has touched yet -- `joinable()` cannot be false here."
     },
     {
       "file": "include/morph/net/socket_server.hpp",
-      "line": 245,
+      "line": 248,
       "source": "if (closed.load() || !socket.valid()) {",
       "reason": "The `!socket.valid()` disjunct is unreachable by construction (net audit, `socket_server.hpp` finding #7). `ClientConnection::socket` is set once at construction and never moved from or reassigned anywhere in this file (only method calls on it, never an assignment or `std::move`); `TcpSocket::valid()` is `_fd >= 0`, and `_fd` only becomes -1 in the move constructor/assignment and the destructor, neither of which can run while `sendText()` holds a `shared_ptr<ClientConnection>`. `shutdownBoth()` does not touch `_fd`. `closed.store(true)` is what every real teardown path sets first, so the `closed.load()` disjunct alone accounts for all of them. This entry survives a gate failure that looks like it retires it, so the artifact is recorded here: without -fprofile-update=atomic, a coverage run can report this disjunct as *taken* and fail the gate with \"include/morph/net/socket_server.hpp:242 is allowlisted as an uncoverable partial branch, but it is not a partial line in this report\", while four neighbouring reports of the same unchanged code all report it untaken (11 partial lines in this file, each time). The cause: llvm-cov derives the second operand of a short-circuit || by subtracting counters rather than counting it, so with non-atomic counters a concurrent update makes that subtraction go negative and wrap to a huge \"taken\" count. Demonstrated with a controlled probe: 12 threads over `if (flag.load() || !alwaysTrue())` reported `True: 18.4E` for the never-taken arm in 8 runs of 8; the same binary single-threaded reported `True: 0` in 3 of 3, and the same 12 threads built with -fprofile-update=atomic reported `True: 0` in 3 of 3. cmake/compiler_options.cmake's apply_coverage() passes that flag, which is what makes this entry stable rather than flaky. Re-measured with it: `Branch (242:17): [True: 69, False: 1.52k]` and `Branch (242:34): [True: 0, False: 1.52k]`. What would make it reachable, and so retire this entry: any code that move-assigns or destroys ClientConnection::socket while another thread can be inside sendText() -- replacing the connection's socket on a reconnect, say, or dropping the shared_ptr discipline. If this gate reports the line non-partial again on a build carrying -fprofile-update=atomic, that is a real change and the entry should be deleted rather than argued with."
     },
-    {
-      "file": "include/morph/net/socket_server.hpp",
-      "line": 294,
-      "source": "if (!clientSocket) {",
-      "reason": "Real, reachable race (`tryAccept()` returning nullopt because the pending connection went away before it was taken), but accepted as documented rather than forced with a flaky test after extensive attempts (net audit, `socket_server.hpp` finding #10). Three different techniques were tried: a single real `TcpSocket::connect()` immediately followed by an abortive (`SO_LINGER{1,0}`) close (0/150 hits); a burst of many such attempts to build backlog depth (still 0 hits); and a burst of bare non-blocking `::connect()`+abort attempts skipping `TcpSocket::connect()`'s `getaddrinfo()`/poll overhead (960 attempts across 15 bursts, still 0 hits, with most connections resetting before the TCP handshake progressed far enough to make the listener readable at all, rather than after). No way was found, from outside the process, to reliably land in the specific narrow window this branch requires on this machine. Reported as attempted-and-left-open rather than forcing something flakier. RE-READ under -fprofile-update=atomic, over several runs rather than one, because a single figure cannot be told apart from run order. Five CI-equivalent coverage runs, same binaries, same 2962 tests, atomic counters: `Branch (285:17): [True: 0, False: 3.80k / 3.80k / 3.82k / 3.75k / 3.78k]`, with line 286 (`continue`) at 0 executions in every run. The denominator moves with how many accepts the suite happens to perform -- 3.75k to 3.82k across the five, and 576 on another machine -- and the numerator does not move at all: tens of thousands of accepts across five runs, none of them nullopt. The disposition stands, and the figure it quotes is one no concurrent counter update can inflate. What would retire it: a True count above 0 in any run."
-    },
     {
       "file": "include/morph/net/socket_backend.hpp",
       "line": 145,