cool#9833: Adding net::Defaults to UT limited connections

Using runtime mutable defaults for network properties allows us to test the connection limitation code. Costs are removal of constexpr properties, using a global instance, hence rendering the code less optimized and more pessimistic. Signed-off-by: Sven Göthel <[email protected]> Change-Id: I5172b4067598c02678dd0cd2f56c0574013623d1
CollaboraOnline · Oct 29, 2024 · 9372993 · 9372993
1 parent 82796a3
commit 9372993
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 51 deletions.
diff --git a/common/Unit.hpp b/common/Unit.hpp
@@ -7,7 +7,6 @@
 
 #pragma once
 
-#include <atomic>
 #include <cassert>
 #include <chrono>
 #include <map>

diff --git a/net/NetUtil.hpp b/net/NetUtil.hpp
@@ -11,6 +11,7 @@
 
 #pragma once
 
+#include <chrono>
 #include <functional>
 #include <string>
 #include <memory>
@@ -28,6 +29,21 @@ struct sockaddr;
 namespace net
 {
 
+class DefaultValues
+{
+public:
+    /// StreamSocket inactivity timeout in us (3600s default). Zero disables instrument.
+    std::chrono::microseconds inactivityTimeout;
+    /// WebSocketHandler average ping timeout in us (12s default). Zero disables instrument.
+    std::chrono::microseconds wsPingAvgTimeout;
+    /// WebSocketHandler ping interval in us (18s default), i.e. duration until next ping. Zero disables instrument.
+    std::chrono::microseconds wsPingInterval;
+
+    /// Maximum number of concurrent TCP connections. Zero disables instrument.
+    size_t maxTCPConnections;
+};
+extern DefaultValues Defaults;
+
 class HostEntry
 {
     std::string _requestName;

diff --git a/net/Socket.cpp b/net/Socket.cpp
@@ -51,16 +51,15 @@
 #include <openssl/x509v3.h>
 #endif
 #include "WebSocketHandler.hpp"
-#include <net/HttpRequest.hpp>
-#include <NetUtil.hpp>
+#include "net/HttpRequest.hpp"
+#include "net/NetUtil.hpp"
 #include <Log.hpp>
 #include <Watchdog.hpp>
 #include <wasm/base64.hpp>
 
 // Bug in pre C++17 where static constexpr must be defined. Fixed in C++17.
 constexpr std::chrono::microseconds SocketPoll::DefaultPollTimeoutMicroS;
 constexpr std::chrono::microseconds WebSocketHandler::InitialPingDelayMicroS;
-constexpr std::chrono::microseconds WebSocketHandler::PingFrequencyMicroS;
 
 std::atomic<bool> SocketPoll::InhibitThreadChecks(false);
 std::atomic<bool> Socket::InhibitThreadChecks(false);
@@ -70,6 +69,11 @@ std::unique_ptr<Watchdog> SocketPoll::PollWatchdog;
 std::mutex SocketPoll::StatsMutex;
 std::atomic<size_t> SocketPoll::StatsConnectionCount(0);
 
+net::DefaultValues net::Defaults = { .inactivityTimeout = std::chrono::seconds(3600),
+                                     .wsPingAvgTimeout = std::chrono::seconds(12),
+                                     .wsPingInterval = std::chrono::seconds(18),
+                                     .maxTCPConnections = 200000 /* arbitrary value to be resolved */ };
+
 size_t SocketPoll::StatsConnectionMod(size_t added, size_t removed) {
     if( added == 0 && removed == 0 ) {
         return GetStatsConnectionCount();
@@ -316,6 +320,8 @@ namespace {
 
 SocketPoll::SocketPoll(std::string threadName)
     : _name(std::move(threadName)),
+      _limitedConnections( false ),
+      _connectionLimit( 0 ),
       _pollStartIndex(0),
       _stop(false),
       _threadStarted(0),
@@ -1487,10 +1493,10 @@ bool StreamSocket::checkRemoval(std::chrono::steady_clock::time_point now)
     // Forced removal on outside-facing IPv[46] network connections only
     const auto durLast =
         std::chrono::duration_cast<std::chrono::milliseconds>(now - getLastSeenTime());
-    /// TO Criteria: Violate maximum idle (DefaultInactivityimeoutMicroS default 3600s)
-    const bool isInactive = SocketPoll::DefaultInactivityimeoutMicroS > std::chrono::microseconds::zero() &&
-        durLast > SocketPoll::DefaultInactivityimeoutMicroS;
-    /// TO Criteria: Shall terminate?
+    /// Timeout criteria: Violate maximum inactivity (default 3600s)
+    const bool isInactive = net::Defaults.inactivityTimeout > std::chrono::microseconds::zero() &&
+        durLast > net::Defaults.inactivityTimeout;
+    /// Timeout criteria: Shall terminate?
     const bool isTermination = SigUtil::getTerminationFlag();
     if (isInactive || isTermination )
     {

diff --git a/net/Socket.hpp b/net/Socket.hpp
@@ -715,8 +715,6 @@ class SocketPoll
 
     /// Default poll time - useful to increase for debugging.
     static constexpr std::chrono::microseconds DefaultPollTimeoutMicroS = std::chrono::seconds(64);
-    static constexpr std::chrono::microseconds DefaultInactivityimeoutMicroS = std::chrono::seconds(3600);
-    static constexpr size_t DefaultMaxTCPConnections = 200000; // arbitrary value to be resolved
     static std::atomic<bool> InhibitThreadChecks;
 
     /// Stop the polling thread.

diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
@@ -13,7 +13,6 @@
 
 #include "NetUtil.hpp"
 #include "Socket.hpp"
-#include "common/Common.hpp"
 #include "common/Log.hpp"
 #include "common/Protocol.hpp"
 #include "common/Unit.hpp"
@@ -62,8 +61,6 @@ class WebSocketHandler : public ProtocolHandlerInterface
     };
 
     static constexpr std::chrono::microseconds InitialPingDelayMicroS = std::chrono::milliseconds(25);
-    static constexpr std::chrono::microseconds PingFrequencyMicroS = std::chrono::seconds(18);
-    static constexpr std::chrono::microseconds PingTimeoutMicroS = std::chrono::seconds(12);
 
 public:
     /// Perform upgrade ourselves, or select a client web socket.
@@ -75,7 +72,7 @@ class WebSocketHandler : public ProtocolHandlerInterface
         :
 #if !MOBILEAPP
         _lastPingSentTime(std::chrono::steady_clock::now() -
-                          PingFrequencyMicroS +
+                          net::Defaults.wsPingInterval +
                           std::chrono::microseconds(InitialPingDelayMicroS))
         , _isMasking(isClient && isMasking)
         , _inFragmentBlock(false)
@@ -603,7 +600,7 @@ class WebSocketHandler : public ProtocolHandlerInterface
             const auto timeSincePingMicroS
                 = std::chrono::duration_cast<std::chrono::microseconds>(now - _lastPingSentTime);
             timeoutMaxMicroS
-                = std::min(timeoutMaxMicroS, (int64_t)(PingFrequencyMicroS - timeSincePingMicroS).count());
+                = std::min(timeoutMaxMicroS, (int64_t)(net::Defaults.wsPingInterval - timeSincePingMicroS).count());
         }
 #endif
         int events = POLLIN;
@@ -670,21 +667,21 @@ class WebSocketHandler : public ProtocolHandlerInterface
         if (_isClient)
             return false;
 
-        if (PingTimeoutMicroS.count() > std::numeric_limits<double>::epsilon() &&
-            _pingMicroS.average() >= PingTimeoutMicroS.count())
+        if (net::Defaults.wsPingAvgTimeout > std::chrono::microseconds::zero() &&
+            _pingMicroS.average() >= net::Defaults.wsPingAvgTimeout.count())
         {
             std::shared_ptr<StreamSocket> socket = _socket.lock();
             if (socket && socket->isIPType()) // Exclude non-IP local sockets
             {
                 LOG_WRN("CheckTimeout: Timeout websocket: Ping: last " << _pingMicroS.last() << "us, avg "
-                        << _pingMicroS.average() << "us >= " << PingTimeoutMicroS.count() << "us over "
+                        << _pingMicroS.average() << "us >= " << net::Defaults.wsPingAvgTimeout.count() << "us over "
                         << (int)_pingMicroS.duration() << "s, " << *socket);
                 shutdownSilent(socket);
                 return true;
             }
         }
-        if (!_pingMicroS.initialized() || (PingFrequencyMicroS > std::chrono::microseconds::zero() &&
-                                           now - _pingMicroS.lastTime() >= PingFrequencyMicroS))
+        if (!_pingMicroS.initialized() || (net::Defaults.wsPingInterval > std::chrono::microseconds::zero() &&
+                                           now - _pingMicroS.lastTime() >= net::Defaults.wsPingInterval))
         {
             const std::shared_ptr<StreamSocket> socket = _socket.lock();
             if (socket)

diff --git a/test/UnitTimeoutConnections.cpp b/test/UnitTimeoutConnections.cpp
@@ -34,8 +34,7 @@ class UnitTimeoutConnections : public UnitTimeoutBase1
 {
     void configure(Poco::Util::LayeredConfiguration& /* config */) override
     {
-        // to be resolved!
-        // defaults.MaxConnections = ConnectionLimit;
+        net::Defaults.maxTCPConnections = ConnectionLimit;
     }
 
 public:
@@ -49,24 +48,20 @@ class UnitTimeoutConnections : public UnitTimeoutBase1
 
 void UnitTimeoutConnections::invokeWSDTest()
 {
-    // to be resolved!
-    if( false )
-    {
-        UnitBase::TestResult result = TestResult::Ok;
+    UnitBase::TestResult result = TestResult::Ok;
 
-        result = testHttp(ConnectionLimit, ConnectionCount);
-        if (result != TestResult::Ok)
-            exitTest(result);
+    result = testHttp(ConnectionLimit, ConnectionCount);
+    if (result != TestResult::Ok)
+        exitTest(result);
 
-        result = testWSPing(ConnectionLimit, ConnectionCount);
-        if (result != TestResult::Ok)
-            exitTest(result);
+    result = testWSPing(ConnectionLimit, ConnectionCount);
+    if (result != TestResult::Ok)
+        exitTest(result);
 
-        result = testWSDChatPing(ConnectionLimit, ConnectionCount);
-        if (result != TestResult::Ok)
-            exitTest(result);
+    result = testWSDChatPing(ConnectionLimit, ConnectionCount);
+    if (result != TestResult::Ok)
+        exitTest(result);
 
-    }
     exitTest(TestResult::Ok);
 }
 

diff --git a/test/UnitTimeoutWSPing.cpp b/test/UnitTimeoutWSPing.cpp
@@ -34,9 +34,8 @@ class UnitTimeoutWSPing : public UnitTimeoutBase0
 
     void configure(Poco::Util::LayeredConfiguration& /* config */) override
     {
-        // to be resolved!
-        // defaults.WSPingTimeout = std::chrono::microseconds(20);
-        // defaults.WSPingPeriod = std::chrono::microseconds(10000);
+        net::Defaults.wsPingAvgTimeout = std::chrono::microseconds(20);
+        net::Defaults.wsPingInterval = std::chrono::milliseconds(10);
     }
 
 public:
@@ -82,15 +81,12 @@ UnitBase::TestResult UnitTimeoutWSPing::testWSPing()
 
 void UnitTimeoutWSPing::invokeWSDTest()
 {
-    // to be resolved!
-    if( false )
-    {
-        UnitBase::TestResult result = TestResult::Ok;
+    UnitBase::TestResult result;
+
+    result = testWSPing();
+    if (result != TestResult::Ok)
+       exitTest(result);
 
-        result = testWSPing();
-        if (result != TestResult::Ok)
-           exitTest(result);
-    }
     exitTest(TestResult::Ok);
 }
 

diff --git a/wsd/COOLWSD.cpp b/wsd/COOLWSD.cpp
@@ -2743,21 +2743,25 @@ void COOLWSD::innerInitialize(Poco::Util::Application& self)
     if (getConfigValue<bool>(conf, "home_mode.enable", false))
     {
         COOLWSD::MaxConnections = 20;
-        net::Defaults::get().MaxConnections = COOLWSD::MaxConnections; // re-align
         COOLWSD::MaxDocuments = 10;
     }
     else
     {
         conf.setString("feedback.show", "true");
         conf.setString("welcome.enable", "true");
-        COOLWSD::MaxConnections = net::Defaults::get().MaxConnections; // aligned w/ MAX_CONNECTIONS above
         COOLWSD::MaxDocuments = MAX_DOCUMENTS;
     }
 #else
     {
         COOLWSD::MaxDocuments = MAX_DOCUMENTS;
     }
 #endif
+    {
+        LOG_DBG("net::Defaults: WSPing[timeout "
+                << net::Defaults.wsPingAvgTimeout << ", interval " << net::Defaults.wsPingInterval
+                << "], Socket[inactivityTimeout " << net::Defaults.inactivityTimeout
+                << ", maxTCPConnections " << net::Defaults.maxTCPConnections << "]");
+    }
 
 #if !MOBILEAPP
     NoSeccomp = Util::isKitInProcess() || !getConfigValue<bool>(conf, "security.seccomp", true);
@@ -2929,7 +2933,7 @@ void COOLWSD::innerInitialize(Poco::Util::Application& self)
 #endif
 
     WebServerPoll = std::make_unique<TerminatingPoll>("websrv_poll");
-    WebServerPoll->setLimiter( SocketPoll::DefaultMaxTCPConnections );
+    WebServerPoll->setLimiter( net::Defaults.maxTCPConnections );
 
 #if !MOBILEAPP
     net::AsyncDNS::startAsyncDNS();

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
@@ -139,7 +139,7 @@ class DocumentBroker::DocumentBrokerPoll final : public TerminatingPoll
         TerminatingPoll(threadName),
         _docBroker(docBroker)
     {
-        setLimiter( SocketPoll::DefaultMaxTCPConnections );
+        setLimiter( net::Defaults.maxTCPConnections );
     }
 
     void pollingThread() override