diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index d8befd211..39fa2d79d 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -18,7 +18,6 @@ #include "base/timer.hpp" #include "base/tlsstream.hpp" #include "base/utility.hpp" -#include #include #include #include @@ -41,7 +40,7 @@ HttpServerConnection::HttpServerConnection(const WaitGroup::Ptr& waitGroup, cons } HttpServerConnection::HttpServerConnection(const WaitGroup::Ptr& waitGroup, const String& identity, bool authenticated, const Shared::Ptr& stream, boost::asio::io_context& io) - : m_WaitGroup(waitGroup), m_Stream(stream), m_Seen(Utility::GetTime()), m_IoStrand(io), m_ShuttingDown(false), m_ConnectionReusable(true), m_CheckLivenessTimer(io) + : m_WaitGroup(waitGroup), m_Stream(stream), m_IoStrand(io), m_ShuttingDown(false), m_ConnectionReusable(true), m_CheckLivenessTimer(io) { if (authenticated) { m_ApiUser = ApiUser::GetByClientCN(identity); @@ -152,6 +151,11 @@ bool HttpServerConnection::Disconnected() return m_ShuttingDown; } +void HttpServerConnection::SetLivenessTimeout(std::chrono::milliseconds timeout) +{ + m_LivenessTimeout = timeout; +} + static inline bool EnsureValidHeaders( boost::beast::flat_buffer& buf, @@ -453,7 +457,7 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) beast::flat_buffer buf; while (m_WaitGroup->IsLockable()) { - m_Seen = Utility::GetTime(); + m_Seen = ch::steady_clock::now(); HttpRequest request(m_Stream); HttpResponse response(m_Stream, this); @@ -467,7 +471,7 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) break; } - m_Seen = Utility::GetTime(); + m_Seen = ch::steady_clock::now(); auto start (ch::steady_clock::now()); { @@ -519,7 +523,7 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) break; } - m_Seen = std::numeric_limits::max(); + m_Seen = ch::steady_clock::time_point::max(); ProcessRequest(request, response, m_WaitGroup, cpuBoundWorkTime, yc); @@ -542,16 +546,21 @@ void HttpServerConnection::CheckLiveness(boost::asio::yield_context yc) boost::system::error_code ec; for (;;) { - m_CheckLivenessTimer.expires_from_now(boost::posix_time::seconds(5)); + // Wait for the half of the liveness timeout to give the connection some leeway to do other work. + // But never wait longer than 5 seconds to ensure timely shutdowns. + auto sleepTime = std::min(5000ms, m_LivenessTimeout / 2); + m_CheckLivenessTimer.expires_from_now(boost::posix_time::milliseconds(sleepTime.count())); m_CheckLivenessTimer.async_wait(yc[ec]); if (m_ShuttingDown) { break; } - if (m_Seen < Utility::GetTime() - 10) { + if (m_LivenessTimeout < std::chrono::steady_clock::now() - m_Seen) { Log(LogInformation, "HttpServerConnection") - << "No messages for HTTP connection have been received in the last 10 seconds."; + << "No messages for HTTP connection have been received in the last " + << std::chrono::duration_cast(m_LivenessTimeout).count() + << " seconds, disconnecting (from " << m_PeerAddress << ")."; Disconnect(yc); break; diff --git a/lib/remote/httpserverconnection.hpp b/lib/remote/httpserverconnection.hpp index 1f3d5d7f9..4436897f1 100644 --- a/lib/remote/httpserverconnection.hpp +++ b/lib/remote/httpserverconnection.hpp @@ -12,10 +12,13 @@ #include #include #include +#include namespace icinga { +using namespace std::chrono_literals; + /** * An API client connection. * @@ -33,11 +36,23 @@ public: void StartDetectClientSideShutdown(); bool Disconnected(); + /** + * Sets the liveness timeout. + * + * If we don't receive any data from the client in this time frame, we consider the connection + * dead and close it. The default is 10 seconds. This function should only be used for unit tests + * to speed them up. + * + * @param timeout The timeout duration. + */ + void SetLivenessTimeout(std::chrono::milliseconds timeout); + private: WaitGroup::Ptr m_WaitGroup; ApiUser::Ptr m_ApiUser; Shared::Ptr m_Stream; - double m_Seen; + std::chrono::steady_clock::time_point m_Seen{std::chrono::steady_clock::now()}; + std::chrono::milliseconds m_LivenessTimeout{10s}; String m_PeerAddress; boost::asio::io_context::strand m_IoStrand; bool m_ShuttingDown; diff --git a/test/base-timer.cpp b/test/base-timer.cpp index 696a474b4..f0fffb553 100644 --- a/test/base-timer.cpp +++ b/test/base-timer.cpp @@ -22,40 +22,40 @@ BOOST_AUTO_TEST_CASE(interval) BOOST_CHECK(timer->GetInterval() == 1.5); } -int counter = 0; - -static void Callback(const Timer * const&) -{ - counter++; -} - BOOST_AUTO_TEST_CASE(invoke) { - Timer::Ptr timer = Timer::Create(); - timer->OnTimerExpired.connect(&Callback); - timer->SetInterval(1); + int counter = 0; + + Timer::Ptr timer = Timer::Create(); + timer->OnTimerExpired.connect([&counter](const Timer* const&) { counter++; }); + timer->SetInterval(.1); - counter = 0; timer->Start(); - Utility::Sleep(5.5); + Utility::Sleep(.55); timer->Stop(); - BOOST_CHECK(counter >= 4 && counter <= 6); + // At this point, the timer should have fired exactly 5 times (0.5 / 0.1) and the sixth time + // should not have fired yet as we stopped the timer after 0.55 seconds (0.6 would be needed). + BOOST_CHECK_EQUAL(5, counter); } BOOST_AUTO_TEST_CASE(scope) { + int counter = 0; + Timer::Ptr timer = Timer::Create(); - timer->OnTimerExpired.connect(&Callback); - timer->SetInterval(1); + timer->OnTimerExpired.connect([&counter](const Timer* const&) { counter++; }); + timer->SetInterval(.1); - counter = 0; timer->Start(); - Utility::Sleep(5.5); + Utility::Sleep(.55); timer.reset(); - Utility::Sleep(5.5); + Utility::Sleep(.1); - BOOST_CHECK(counter >= 4 && counter <= 6); + // At this point, the timer should have fired exactly 5 times (0.5 / 0.1) and the sixth time + // should not have fired yet as we destroyed the timer after 0.55 seconds (0.6 would be needed), + // and even if we wait another 0.1 seconds after its destruction, it should not fire again. + BOOST_CHECK_EQUAL(5, counter); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/remote-httpserverconnection.cpp b/test/remote-httpserverconnection.cpp index f0b45125d..ac0f895b8 100644 --- a/test/remote-httpserverconnection.cpp +++ b/test/remote-httpserverconnection.cpp @@ -45,10 +45,11 @@ struct HttpServerConnectionFixture : TlsStreamFixture, ConfigurationCacheDirFixt user->Register(); } - void SetupHttpServerConnection(bool authenticated) + void SetupHttpServerConnection(bool authenticated, std::chrono::milliseconds livenessTimeout = std::chrono::milliseconds(10000)) { String identity = authenticated ? "client" : "invalid"; m_Connection = new HttpServerConnection(m_WaitGroup, identity, authenticated, server); + m_Connection->SetLivenessTimeout(livenessTimeout); m_Connection->Start(); } @@ -552,11 +553,11 @@ BOOST_AUTO_TEST_CASE(handler_throw_streaming) BOOST_AUTO_TEST_CASE(liveness_disconnect) { - SetupHttpServerConnection(false); + SetupHttpServerConnection(false, std::chrono::milliseconds(300)); // 300ms liveness timeout is more than enough! - BOOST_REQUIRE(AssertServerDisconnected(std::chrono::seconds(11))); + BOOST_REQUIRE(AssertServerDisconnected(std::chrono::milliseconds(450))); // Give some leeway to Asio's timers BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*")); - BOOST_REQUIRE(ExpectLogPattern("No messages for HTTP connection have been received in the last 10 seconds.")); + BOOST_REQUIRE(ExpectLogPattern("No messages for HTTP connection have been received in the last \\d+ seconds, disconnecting .*")); BOOST_REQUIRE(Shutdown(client)); }