[MG_Remote, MG_Test, scripts] (P7): bound the pre-auth cap and pause accepting when out of descriptors

MobileGL/MG_Remote/Server/PreAuthGate.h

@@ -89,6 +89,11 @@ namespace MobileGL::MG_Remote::Server {
     // port probe; one that held its slot longer is a failure. A real client writes its first frame
     // the moment it connects, so it is never on the wrong side of this.
     inline constexpr std::uint32_t kPreAuthProbeGraceMs = 250;
+    // MOBILEGL_IPC_PREAUTH_MAX never exceeds this. Every pending connection is a descriptor the
+    // supervisor holds, and an unbounded knob would let a connection flood walk it into
+    // RLIMIT_NOFILE (the supervisor also survives that now - it pauses accepting - but a knob
+    // should not be the way in).
+    inline constexpr std::uint32_t kPreAuthMaxPendingCeiling = 256;
 
     // The refusal details a peer (and the log) sees. Named once so the supervisor and the unit test
     // cannot drift on the words; the TCP lane's scripts mirror them rather than parse them, so a
@@ -108,8 +113,9 @@ namespace MobileGL::MG_Remote::Server {
         // MOBILEGL_IPC_PREAUTH_MS: how long a new connection has to present a complete first
         // frame. A real client writes its Hello (or DataBind) the moment it connects.
         std::uint32_t deadlineMs = 2000;
-        // MOBILEGL_IPC_PREAUTH_MAX: connections the supervisor holds unauthenticated at once; one
-        // more is refused Busy at accept.
+        // MOBILEGL_IPC_PREAUTH_MAX: connections the supervisor holds unauthenticated at once, at
+        // most kPreAuthMaxPendingCeiling. When they are all taken a newcomer displaces the busiest
+        // address's oldest one, or is refused Busy at accept (ChoosePendingToDisplace).
         std::uint32_t maxPending = 8;
         // MOBILEGL_IPC_AUTH_BACKOFF_AFTER: failures from one address before it is refused at
         // accept. 0 turns the backoff off (the fuzz arm does, to send many malformed frames from
@@ -124,7 +130,8 @@ namespace MobileGL::MG_Remote::Server {
         static PreAuthKnobs FromEnvironment() {
             PreAuthKnobs knobs;
             knobs.deadlineMs = Read("MOBILEGL_IPC_PREAUTH_MS", knobs.deadlineMs, false);
-            knobs.maxPending = Read("MOBILEGL_IPC_PREAUTH_MAX", knobs.maxPending, false);
+            knobs.maxPending =
+                std::min(Read("MOBILEGL_IPC_PREAUTH_MAX", knobs.maxPending, false), kPreAuthMaxPendingCeiling);
             knobs.backoffAfter = Read("MOBILEGL_IPC_AUTH_BACKOFF_AFTER", knobs.backoffAfter, true);
             knobs.backoffBaseMs = Read("MOBILEGL_IPC_AUTH_BACKOFF_MS", knobs.backoffBaseMs, false);
             return knobs;

MobileGL/MG_Remote/Server/ServerMain.cpp

@@ -714,16 +714,23 @@ public:
         std::vector<pollfd> fds;
         for (;;) {
             Reap();
-            fds.clear();
-            fds.push_back({m_listener, POLLIN, 0});
-            for (const auto& peer : m_pending) fds.push_back({peer.fd, POLLIN, 0});
             auto now = Clock::now();
+            // While accepting is paused (out of descriptors, AcceptNew) the listener sits out the
+            // poll - it stays readable, and would spin the loop - and the pause's end is a wakeup.
+            const bool acceptPaused = now < m_acceptResumeAt;
+            fds.clear();
+            fds.push_back({acceptPaused ? -1 : m_listener, POLLIN, 0});
+            for (const auto& peer : m_pending) fds.push_back({peer.fd, POLLIN, 0});
             // 250 ms is the old accept timeout, i.e. how often a finished child is reaped while
             // nothing arrives; a pending deadline that falls sooner shortens it.
             std::int64_t timeout = 250;
             for (const auto& peer : m_pending)
                 timeout = std::clamp<std::int64_t>(
                     std::chrono::duration_cast<std::chrono::milliseconds>(peer.deadline - now).count(), 0, timeout);
+            if (acceptPaused)
+                timeout = std::clamp<std::int64_t>(
+                    std::chrono::duration_cast<std::chrono::milliseconds>(m_acceptResumeAt - now).count() + 1, 0,
+                    timeout);
             const int polled = ::poll(fds.data(), static_cast<nfds_t>(fds.size()), static_cast<int>(timeout));
             if (polled < 0 && errno != EINTR) {
                 WireLogError("MG_Remote server: supervisor poll failed: %s", std::strerror(errno));
@@ -823,6 +830,18 @@ private:
             int fd = -1;
             const auto accepted = SocketTransport::AcceptOne(m_listener, 0, &fd);
             if (accepted == MOBILEGL_ERR_TIMEOUT) return 0;
+            if (accepted == MOBILEGL_ERR_OUT_OF_MEMORY) {
+                // f2-auth fix round. Out of descriptors (or memory) is not a dead listener: the
+                // connection waits in the backlog, and pending connections give descriptors back
+                // as they are judged or reach their deadline. The first cut returned 73 here, so
+                // a connection flood against a low RLIMIT_NOFILE (or a large pending cap) took the
+                // supervisor down. Now accepting pauses for kAcceptPauseMs and the loop goes on.
+                m_acceptResumeAt = now + std::chrono::milliseconds(kAcceptPauseMs);
+                WireLogError("MG_Remote server: accept paused for %u ms: out of descriptors or memory with %zu "
+                             "connections awaiting authentication",
+                             kAcceptPauseMs, m_pending.size());
+                return 0;
+            }
             if (accepted != MOBILEGL_OK) return 73;
             std::string address = PeerAddress(fd);
             if (const std::uint32_t retry = m_backoff.RetryAfterMs(address, now); retry != 0) {
@@ -979,6 +998,9 @@ private:
     int m_handoff = -1;
     std::vector<PendingPeer> m_pending;
     std::vector<PendingPeer> m_ready;
+    // Accepting resumes at this time after accept(2) ran out of descriptors or memory.
+    static constexpr std::uint32_t kAcceptPauseMs = 100;
+    Clock::time_point m_acceptResumeAt{};
 };
 }
 

MobileGL/MG_Remote/Transport/SocketTransport.cpp

@@ -486,7 +486,13 @@ namespace MobileGL::MG_Remote::Transport {
             if (fd < 0) {
                 // The same two non-failures AcceptPair re-polls on.
                 if (errno == EINTR || errno == ECONNABORTED) continue;
-                WireLogError("MG_Remote SocketTransport: accept failed: %s", std::strerror(errno));
+                const int error = errno;
+                WireLogError("MG_Remote SocketTransport: accept failed: %s", std::strerror(error));
+                // Out of descriptors or memory is the PROCESS's state, not the listener's: the
+                // connection waits in the backlog (f2-auth fix round; the TCP supervisor pauses
+                // and retries instead of exiting 73).
+                if (error == EMFILE || error == ENFILE || error == ENOBUFS || error == ENOMEM)
+                    return MOBILEGL_ERR_OUT_OF_MEMORY;
                 return MOBILEGL_ERR_TRANSPORT_CLOSED;
             }
             ::fcntl(fd, F_SETFD, FD_CLOEXEC);

MobileGL/MG_Remote/Transport/SocketTransport.h

@@ -123,6 +123,10 @@ namespace MobileGL::MG_Remote::Transport {
         static MobileGLResult ConnectDataConnection(const std::string& path, std::uint32_t timeoutMs,
                                                     MobileGLByteSpan firstFrame, int* outFd);
         // Accepts ONE connection (TCP options applied, CLOEXEC). MOBILEGL_ERR_TIMEOUT when none.
+        // MOBILEGL_ERR_OUT_OF_MEMORY when accept(2) ran out of descriptors or memory (EMFILE,
+        // ENFILE, ENOBUFS, ENOMEM): the connection stays in the backlog and the listener is fine,
+        // so a caller that serves many peers can wait and try again (the TCP supervisor does);
+        // TRANSPORT_CLOSED for every other failure.
         static MobileGLResult AcceptOne(int listenFd, std::uint32_t timeoutMs, int* outFd);
         // Reads EXACTLY one framed message from a raw socket - the header, then the payload,
         // and not one byte more. A DataBind is followed on the same connection by StreamLink's

MobileGL/MG_Test/Wire/PreAuthGateTest.cpp

@@ -303,4 +303,14 @@ TEST(PreAuthGateTest, TheKnobsReadTheirEnvironmentAndKeepTheirDefaultsOtherwise)
         EXPECT_EQ(knobs.backoffAfter, 5u);
         EXPECT_EQ(knobs.backoffBaseMs, 1000u);
     }
+    // f2-auth fix round: the pending cap is bounded, whatever it is set to - every pending
+    // connection is a descriptor. Red once by dropping the clamp: 100000 comes back.
+    {
+        ScopedEnvironment b("MOBILEGL_IPC_PREAUTH_MAX", "100000");
+        EXPECT_EQ(Server::PreAuthKnobs::FromEnvironment().maxPending, Server::kPreAuthMaxPendingCeiling);
+    }
+    {
+        ScopedEnvironment b("MOBILEGL_IPC_PREAUTH_MAX", "256");
+        EXPECT_EQ(Server::PreAuthKnobs::FromEnvironment().maxPending, 256u);
+    }
 }

scripts/ci/tcp_supervisor_smoke.py

@@ -22,7 +22,9 @@ refusal (`data_bind_queued_before_bound`). The f2-auth fix round added that the
 shared between addresses (`pending_queue_is_shared`) and that an address holding slots with silent
 connections is backed off (`silent_slot_holder_backed_off`), that routing DataBinds does not stall
 the supervisor (`data_bind_routing_does_not_stall`), and that a child which refuses its Hello
-refuses the DataBinds queued to it (`data_bind_queued_before_refused_hello`, ID-P7-44). The five
+refuses the DataBinds queued to it (`data_bind_queued_before_refused_hello`, ID-P7-44), and that
+running out of descriptors pauses accepting instead of ending the supervisor
+(`accept_out_of_descriptors`). The five
 malformed-frame shapes are fuzz arm 1's own entry, scripts/ci/ph_fuzz_control_frames.py.
 
 It needs flatc, which is deliberately absent from the default build graph (gen_protocol.py's
@@ -38,6 +40,7 @@ import json
 import os
 from pathlib import Path
 import re
+import resource
 import shutil
 import signal
 import socket
@@ -234,18 +237,25 @@ def refused_listen(server, log, endpoint, token):
 
 
 @contextmanager
-def supervisor(server, log, same_build=False, serve=True, extra_env=None):
+def supervisor(server, log, same_build=False, serve=True, extra_env=None, nofile=None):
     """A supervisor on its own free loopback port. `serve=False` is the single-session shape
     (no fork; the process IS the session and exits with it), which `single_session_listener`
     below needs and nothing else does. `extra_env` sets supervisor knobs (PH-7 (5)'s pre-auth
-    deadline, pending cap and backoff) for the controls that need non-default ones."""
+    deadline, pending cap and backoff) for the controls that need non-default ones. `nofile`
+    lowers the supervisor's soft RLIMIT_NOFILE, for the descriptor-exhaustion control."""
     port = free_loopback_port()
     base = log.with_suffix('.library.log')
     env = supervisor_environment(base, kToken, same_build=same_build)
     env.update(extra_env or {})
+
+    def limit():
+        if nofile is not None:
+            resource.setrlimit(resource.RLIMIT_NOFILE, (nofile, resource.getrlimit(resource.RLIMIT_NOFILE)[1]))
+
     with log.open('wb') as output:
         child = subprocess.Popen([server, f'tcp://127.0.0.1:{port}'] + (['--serve'] if serve else []),
-                                 env=env, stdout=output, stderr=output, start_new_session=True)
+                                 env=env, stdout=output, stderr=output, start_new_session=True,
+                                 preexec_fn=limit if nofile is not None else None)
     try:
         # Log.cpp's RoleLogPath: the base name is a BASE, and the server role writes
         # `<stem>.server<ext>`. Yielded rather than recomputed at the call site, so the one
@@ -972,6 +982,51 @@ def main():
                 l.strip() for l in named.splitlines() if reason in l)}
             assert process.poll() is None, 'the pre-auth limits killed the supervisor'
 
+        # ---- f2-auth fix round: OUT OF DESCRIPTORS, THE SUPERVISOR PAUSES ACCEPTING, NOT EXITS ----
+        #
+        # accept(2) failing with EMFILE used to be "the listener failed": the supervisor logged
+        # its summary and exited 73, so a connection flood against a low RLIMIT_NOFILE (or a large
+        # MOBILEGL_IPC_PREAUTH_MAX - now clamped to 256) took the server down for everybody. Here
+        # the supervisor runs with a soft limit of 16 descriptors (five in use when idle) and a
+        # pending cap it cannot reach (64), and 48 silent connections are opened (the listen
+        # backlog is 8, so they arrive in waves - the first waves alone pass the limit): it must
+        # name the pause, stay up, and - once the flood is gone - still answer a Hello
+        # (fingerprint 0, so the answer is the child's Refuse{WireFingerprint}). The backoff is
+        # off: the flood's failures are not this control's subject. RED before the fix: the
+        # supervisor has exited.
+        exhaust = {'MOBILEGL_IPC_PREAUTH_MAX': '64', 'MOBILEGL_IPC_PREAUTH_MS': '3000',
+                   'MOBILEGL_IPC_AUTH_BACKOFF_AFTER': '0'}
+        with supervisor(args.server, args.out / 'descriptors.log', extra_env=exhaust, nofile=16) as (
+                port, process, serverLog):
+            flood = []
+            try:
+                # The first connect waits for the supervisor to be listening (connect_control's
+                # retry); the rest are non-blocking: a blocking one stalls this loop for its whole
+                # timeout once the backlog is full, and the flood would never outrun the deadlines.
+                flood.append(connect_control(port))
+                for _ in range(47):
+                    peer = socket.socket()
+                    flood.append(peer)
+                    peer.setblocking(False)
+                    try:
+                        peer.connect(('127.0.0.1', port))
+                    except (BlockingIOError, ConnectionRefusedError):
+                        pass  # refused is the red shape: the supervisor is gone
+                paused = wait_for_log(serverLog, 'accept paused for', seconds=5)
+                alive = process.poll() is None
+            finally:
+                for peer in flood:
+                    peer.close()
+            assert alive, f'the supervisor exited ({process.poll()}) when accept ran out of descriptors; see {serverLog}'
+            assert paused is not None, f'the supervisor never named its accept pause in {serverLog}'
+            time.sleep(1.0)  # every flood connection has been read as closed (or is still in the backlog)
+            after_flood = exchange(port, schema, hello(schema, flatbuffers))
+            assert after_flood.get('code') == 2 and after_flood.get('expected', 0), (
+                f'after the descriptor flood a Hello was answered {after_flood}')
+            assert process.poll() is None, 'the supervisor died after the descriptor flood'
+            evidence['accept_out_of_descriptors'] = {'line': next(
+                l.strip() for l in paused.splitlines() if 'accept paused for' in l), 'afterFlood': after_flood}
+
         # ---- ID-P7-44, f2-auth fix round: A CHILD THAT REFUSES ITS HELLO REFUSES WHAT WAS QUEUED --
         #
         # The supervisor forwards DataBinds to a child from the moment it forks it. A child that