HTTPServer: disconnect after idle timeout (-rpcservertimeout)

src/httpserver.cpp

@@ -24,6 +24,7 @@
 #include <util/thread.h>
 #include <util/threadnames.h>
 #include <util/threadpool.h>
+#include <util/time.h>
 #include <util/translation.h>
 
 #include <condition_variable>
@@ -1360,6 +1361,9 @@ void HTTPServer::SocketHandlerConnected(const IOReadiness& io_readiness) const
                     client->m_id);
                 client->m_disconnect = true;
             } else {
+                // Reset idle timeout
+                client->m_idle_since = Now<SteadySeconds>();
+
                 // Prevent disconnect until all requests are completely handled.
                 client->m_connection_busy = true;
 
@@ -1509,12 +1513,27 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr<HTTPRemot
 
 void HTTPServer::DisconnectClients()
 {
+    const auto now{Now<SteadySeconds>()};
     size_t erased = std::erase_if(m_connected,
                                   [&](auto& client) {
-                                        // Disconnect this client due to error or end of communication.
+                                        // First check for idle timeout. We reset the timer when we send and receive data,
+                                        // but if the server is busy handling a request we should ignore the timeout until
+                                        // the reply is sent. If we did erase the shared_ptr<HTTPRemoteClient> reference in m_connected
+                                        // while the server is busy with a request, there would still be a reference in a worker
+                                        // thread keeping the socket open even after "disconnecting".
+                                        const bool is_idle{m_rpcservertimeout.count() > 0 &&
+                                                           now - client->m_idle_since.load() > m_rpcservertimeout &&
+                                                           !client->m_req_busy};
+
+                                        // Disconnect this client due to error, end of communication, or idle timeout.
                                         // May drop unsent data if we are closing due to error.
-                                        if (client->m_disconnect) {
-                                            ;
+                                        if (client->m_disconnect || is_idle) {
+                                            if (is_idle) {
+                                                LogDebug(BCLog::HTTP,
+                                                         "HTTP client idle timeout %s (id=%llu)",
+                                                         client->m_origin,
+                                                         client->m_id);
+                                            }
                                         } else {
                                             // Disconnect this client because the server is shutting
                                             // down and we need to disconnect all clients...
@@ -1652,6 +1671,9 @@ bool HTTPRemoteClient::MaybeSendBytesFromBuffer()
             m_send_ready = true;
             m_connection_busy = true;
         }
+
+        // Finally, reset idle timeout
+        m_idle_since = Now<SteadySeconds>();
     }
 
     return true;
@@ -1666,6 +1688,8 @@ bool InitHTTPServer()
     // Create HTTPServer, using a dummy request handler just for this commit
     g_http_server = std::make_unique<HTTPServer>([&](std::unique_ptr<HTTPRequest> req){});
 
+    g_http_server->SetServerTimeout(std::chrono::seconds(gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)));
+
     // Bind HTTP server to specified addresses
     std::vector<std::pair<std::string, uint16_t>> endpoints{GetBindAddresses()};
     bool bind_success{false};

src/httpserver.h

@@ -23,6 +23,7 @@
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/threadinterrupt.h>
+#include <util/time.h>
 
 namespace util {
 class SignalInterrupt;
@@ -413,6 +414,11 @@ public:
      */
     void StopAccepting() { m_stop_accepting = true; }
 
+    /**
+     * Set the idle client timeout (-rpcservertimeout)
+     */
+    void SetServerTimeout(std::chrono::seconds seconds) { m_rpcservertimeout = seconds; }
+
     /**
      * Force-remove all remaining clients from m_connected without waiting for
      * graceful disconnection. Must only be called after JoinSocketsThreads().
@@ -503,6 +509,11 @@ private:
     std::function<void(std::unique_ptr<HTTPRequest>&&)> m_request_dispatcher GUARDED_BY(m_request_dispatcher_mutex);
     /// @}
 
+    /**
+     * Idle timeout after which clients are disconnected
+     */
+    std::chrono::seconds m_rpcservertimeout{DEFAULT_HTTP_SERVER_TIMEOUT};
+
     /**
      * Accept a connection.
      * @param[in] listen_sock Socket on which to accept the connection.
@@ -652,8 +663,13 @@ public:
     //! possibly overriding all other disconnect flags.
     std::atomic_bool m_disconnect{false};
 
+    //! Timestamp of last send or receive activity, used for -rpcservertimeout.
+    //! Due to optimistic sends it may be updated in either a worker thread or in the
+    //! I/O thread. It is checked in the I/O thread to disconnect idle clients.
+    std::atomic<SteadySeconds> m_idle_since;
+
     explicit HTTPRemoteClient(HTTPServer::Id id, const CService& addr, std::unique_ptr<Sock> socket)
-        : m_id(id), m_addr(addr), m_origin(addr.ToStringAddrPort()), m_sock{std::move(socket)} {};
+        : m_id(id), m_addr(addr), m_origin(addr.ToStringAddrPort()), m_sock{std::move(socket)}, m_idle_since{Now<SteadySeconds>()} {}
 
     // Disable copies (should only be used as shared pointers)
     HTTPRemoteClient(const HTTPRemoteClient&) = delete;