From: Razvan Becheriu <razvan@isc.org>
Date: Wed, 3 Jun 2020 19:21:31 +0000 (+0300)
Subject: [#1239] updating poke time is now Kea thread safe
X-Git-Tag: Kea-1.7.9~66
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4713f23fc25c6f50e07c6bf39b008622bc3de7aa;p=thirdparty%2Fkea.git

[#1239] updating poke time is now Kea thread safe
---

diff --git a/src/hooks/dhcp/high_availability/communication_state.cc b/src/hooks/dhcp/high_availability/communication_state.cc
index 67658b36a1..4888493902 100644
--- a/src/hooks/dhcp/high_availability/communication_state.cc
+++ b/src/hooks/dhcp/high_availability/communication_state.cc
@@ -19,8 +19,11 @@
 #include <dhcp/pkt6.h>
 #include <http/date_time.h>
 #include <util/boost_time_utils.h>
+#include <util/multi_threading_mgr.h>
+
 #include <boost/bind.hpp>
 #include <boost/pointer_cast.hpp>
+
 #include <sstream>
 #include <utility>
 
@@ -29,7 +32,10 @@ using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::http;
 using namespace isc::log;
+using namespace isc::util;
+
 using namespace boost::posix_time;
+using namespace std;
 
 namespace {
 
@@ -54,13 +60,28 @@ CommunicationState::CommunicationState(const IOServicePtr& io_service,
       heartbeat_impl_(0), partner_state_(-1), partner_scopes_(),
       clock_skew_(0, 0, 0, 0), last_clock_skew_warn_(),
       my_time_at_skew_(), partner_time_at_skew_(),
-      analyzed_messages_count_(0) {
+      analyzed_messages_count_(0), mutex_(new mutex()) {
 }
 
 CommunicationState::~CommunicationState() {
     stopHeartbeat();
 }
 
+void
+CommunicationState::modifyPokeTime(const long secs) {
+    if (MultiThreadingMgr::instance().getMode()) {
+        std::lock_guard<std::mutex> lk(*mutex_);
+        modifyPokeTimeInternal(secs);
+    } else {
+        modifyPokeTimeInternal(secs);
+    }
+}
+
+void
+CommunicationState::modifyPokeTimeInternal(const long secs) {
+    poke_time_ += boost::posix_time::seconds(secs);
+}
+
 void
 CommunicationState::setPartnerState(const std::string& state) {
     try {
@@ -151,12 +172,29 @@ CommunicationState::stopHeartbeat() {
     }
 }
 
-void
-CommunicationState::poke() {
+boost::posix_time::time_duration
+CommunicationState::updatePokeTime() {
+    if (MultiThreadingMgr::instance().getMode()) {
+        std::lock_guard<std::mutex> lk(*mutex_);
+        return (updatePokeTimeInternal());
+    } else {
+        return (updatePokeTimeInternal());
+    }
+}
+
+boost::posix_time::time_duration
+CommunicationState::updatePokeTimeInternal() {
     // Remember previous poke time.
     boost::posix_time::ptime prev_poke_time = poke_time_;
     // Set poke time to the current time.
     poke_time_ = boost::posix_time::microsec_clock::universal_time();
+    return (poke_time_ - prev_poke_time);
+}
+
+void
+CommunicationState::poke() {
+    // Update poke time and compute duration.
+    boost::posix_time::time_duration duration_since_poke = updatePokeTime();
 
     // If we have been tracking the DHCP messages directed to the partner,
     // we need to clear any gathered information because the connection
@@ -170,7 +208,6 @@ CommunicationState::poke() {
         // lower than 1s is when we're performing lease updates. In order to avoid the
         // overhead of re-scheduling the timer too frequently we reschedule it only if the
         // duration is 1s or more. This matches the time resolution for heartbeats.
-        boost::posix_time::time_duration duration_since_poke = poke_time_ - prev_poke_time;
         if (duration_since_poke.total_seconds() > 0) {
             // A poke causes the timer to be re-scheduled to prevent it
             // from triggering a heartbeat shortly after confirming the
@@ -183,6 +220,16 @@ CommunicationState::poke() {
 
 int64_t
 CommunicationState::getDurationInMillisecs() const {
+    if (MultiThreadingMgr::instance().getMode()) {
+        std::lock_guard<std::mutex> lk(*mutex_);
+        return (getDurationInMillisecsInternal());
+    } else {
+        return (getDurationInMillisecsInternal());
+    }
+}
+
+int64_t
+CommunicationState::getDurationInMillisecsInternal() const {
     ptime now = boost::posix_time::microsec_clock::universal_time();
     time_duration duration = now - poke_time_;
     return (duration.total_milliseconds());
diff --git a/src/hooks/dhcp/high_availability/communication_state.h b/src/hooks/dhcp/high_availability/communication_state.h
index 0350fbca94..2ffffa4f8a 100644
--- a/src/hooks/dhcp/high_availability/communication_state.h
+++ b/src/hooks/dhcp/high_availability/communication_state.h
@@ -13,6 +13,7 @@
 #include <asiolink/io_service.h>
 #include <cc/data.h>
 #include <dhcp/pkt.h>
+
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/function.hpp>
 #include <boost/multi_index_container.hpp>
@@ -21,8 +22,11 @@
 #include <boost/multi_index/indexed_by.hpp>
 #include <boost/multi_index/member.hpp>
 #include <boost/multi_index/ordered_index.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <boost/shared_ptr.hpp>
+
 #include <map>
+#include <mutex>
 #include <set>
 #include <string>
 
@@ -344,8 +348,45 @@ public:
     /// @return JSON element holding the report.
     data::ElementPtr getReport() const;
 
+    /// @brief Modifies poke time by adding seconds to it.
+    ///
+    /// Used in unittests only.
+    ///
+    /// Should be called in a thread safe context.
+    ///
+    /// @param secs number of seconds to be added to the poke time. If
+    /// the value is negative it will set the poke time in the past
+    /// comparing to current value.
+    void modifyPokeTime(const long secs);
+
+    /// @brief Modifies poke time by adding seconds to it.
+    ///
+    /// Used in unittests only.
+    ///
+    /// @param secs number of seconds to be added to the poke time. If
+    /// the value is negative it will set the poke time in the past
+    /// comparing to current value.
+    void modifyPokeTimeInternal(const long secs);
+
 protected:
 
+    /// @brief Returns duration between the poke time and current time.
+    ///
+    /// @return Duration between the poke time and current time.
+    int64_t getDurationInMillisecsInternal() const;
+
+    /// @brief Update the poke time and compute the duration.
+    ///
+    /// @return The time elapsed.
+    boost::posix_time::time_duration updatePokeTime();
+
+    /// @brief Update the poke time and compute the duration.
+    ///
+    /// Should be called in a thread safe context.
+    ///
+    /// @return The time elapsed.
+    boost::posix_time::time_duration updatePokeTimeInternal();
+
     /// @brief Pointer to the common IO service instance.
     asiolink::IOServicePtr io_service_;
 
@@ -387,6 +428,9 @@ protected:
 
     /// @brief Total number of analyzed messages to be responded by partner.
     size_t analyzed_messages_count_;
+
+    /// @brief The mutex used to protect internal state.
+    const boost::scoped_ptr<std::mutex> mutex_;
 };
 
 /// @brief Type of the pointer to the @c CommunicationState object.
diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.h b/src/hooks/dhcp/high_availability/tests/ha_test.h
index 9b41acfbcc..94d230827b 100644
--- a/src/hooks/dhcp/high_availability/tests/ha_test.h
+++ b/src/hooks/dhcp/high_availability/tests/ha_test.h
@@ -45,15 +45,6 @@ public:
         : StateType(io_service, config) {
     }
 
-    /// @brief Modifies poke time by adding seconds to it.
-    ///
-    /// @param secs number of seconds to be added to the poke time. If
-    /// the value is negative it will set the poke time in the past
-    /// comparing to current value.
-    void modifyPokeTime(const long secs) {
-        StateType::poke_time_ += boost::posix_time::seconds(secs);
-    }
-
     /// @brief Checks if the object was poked recently.
     ///
     /// @return true if the object was poked less than 5 seconds ago,