http: limit connected clients to 16

src/httpserver.cpp

@@ -949,11 +949,13 @@ void HTTPServer::SocketHandlerListening(const Sock::EventsPerSock& events_per_so
         }
         const auto it = events_per_sock.find(sock);
         if (it != events_per_sock.end() && it->second.occurred & Sock::RecvEvent) {
-            CService addr_accepted;
-
-            auto sock_accepted{AcceptConnection(*sock, addr_accepted)};
-
-            if (sock_accepted) {
+            // Drain all pending connections from this socket up to the limit.
+            // Stop early if the kernel queue is empty (AcceptConnection returns null)
+            // or if accepting the last connection brought us to the limit.
+            while (GetConnectionsCount() < MAX_HTTP_CONNECTIONS) {
+                CService addr_accepted;
+                auto sock_accepted{AcceptConnection(*sock, addr_accepted)};
+                if (!sock_accepted) break;
                 NewSockAccepted(std::move(sock_accepted), addr_accepted);
             }
         }
@@ -964,8 +966,14 @@ HTTPServer::IOReadiness HTTPServer::GenerateWaitSockets() const
 {
     IOReadiness io_readiness;
 
-    for (const auto& sock : m_listen) {
-        io_readiness.events_per_sock.emplace(sock, Sock::Events{Sock::RecvEvent});
+    // If the server is already handling its max connected clients count,
+    // don't bother checking the listening sockets for new inbound connections.
+    // Leave them in the kernel's queue until space in the application opens
+    // up (or the client times out on its own).
+    if (GetConnectionsCount() < MAX_HTTP_CONNECTIONS) {
+        for (const auto& sock : m_listen) {
+            io_readiness.events_per_sock.emplace(sock, Sock::Events{Sock::RecvEvent});
+        }
     }
 
     for (const auto& http_client : m_connected) {

src/httpserver.h

@@ -41,6 +41,11 @@ inline constexpr int DEFAULT_HTTP_WORKQUEUE=64;
 
 inline constexpr int DEFAULT_HTTP_SERVER_TIMEOUT=30;
 
+/**
+ * Maximum number of connected HTTP clients
+ */
+inline constexpr int MAX_HTTP_CONNECTIONS = 16;
+
 enum class HTTPRequestMethod {
     UNKNOWN,
     GET,

test/functional/interface_http.py

@@ -131,6 +131,7 @@ class HTTPBasicsTest (BitcoinTestFramework):
         self.check_null_byte_in_uri()
         self.check_invalid_http_version()
         self.check_whitespace_in_headers()
+        self.check_connection_limit()
 
 
     def check_default_connection(self):
@@ -609,5 +610,50 @@ class HTTPBasicsTest (BitcoinTestFramework):
         assert_equal(response.status, http.client.BAD_REQUEST)
 
 
+    def check_connection_limit(self):
+        self.log.info("Check connection limits")
+
+        # Disable timeout so the initial batch of clients stays connected
+        # until the end of the test.
+        self.restart_node(0, extra_args=["-rpcservertimeout=0"])
+
+        # Close the persistent HTTP connection to this node by replacing it with
+        # a new AuthServiceProxy, reducing HTTPServer::GetConnectionsCount() to 0.
+        # The new AuthServiceProxy won't actually open an HTTP connection until
+        # it needs to send an RPC (for example, to stop the node at the end of the test).
+        self.node._rpc = self.node.create_new_rpc_connection(mode="AUTHPROXY")
+
+        MAX_HTTP_CONNECTIONS = 16
+        connections = []
+
+        # Connections all succeed up to the limit
+        with self.node.assert_debug_log(
+            expected_msgs = [f"method=invalidrpc_{i}" for i in range(1, MAX_HTTP_CONNECTIONS + 1)]
+        ):
+            for i in range(1, MAX_HTTP_CONNECTIONS + 1):
+                conn = BitcoinHTTPConnection(self.node)
+                # Each client makes a unique request so it's easy to find in the log
+                conn.post('/', f'{{"method": "invalidrpc_{i}"}}', connection_header='keep-alive').read()
+                connections.append(conn)
+
+        # The next connection is over the limit, expect it to timeout
+        with self.node.assert_debug_log(
+            expected_msgs = [],
+            unexpected_msgs = ["method=never_accepted"]
+        ):
+            conn = BitcoinHTTPConnection(self.node)
+            conn.set_timeout(5)
+            try:
+                conn.post('/', '{"method": "never_accepted"}', connection_header='keep-alive').read()
+                assert False, "Connection succeeded unexpectedly"
+            except TimeoutError:
+                pass
+
+        # Original 16 clients are still connected
+        assert_equal(len(connections), MAX_HTTP_CONNECTIONS)
+        for client in connections:
+            assert not client.sock_closed()
+
+
 if __name__ == '__main__':
     HTTPBasicsTest(__file__).main()