From: Razvan Becheriu <razvan@isc.org>
Date: Wed, 29 Apr 2020 16:43:24 +0000 (+0300)
Subject: [#1188] addressed review
X-Git-Tag: Kea-1.7.8~150
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b8a679adf9e36a1ef03e124329d2e44aed0b3c7;p=thirdparty%2Fkea.git

[#1188] addressed review
---

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index 8b4872fa5d..05cbcd9922 100644
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -415,6 +415,18 @@ ControlledDhcpv4Srv::commandConfigSetHandler(const string&,
         CfgMgr::instance().getCurrentCfg()->applyLoggingCfg();
     }
 
+    // Configure multi threading
+    try {
+        CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading());
+        if (MultiThreadingMgr::instance().getMode()) {
+            LOG_FATAL(dhcp4_logger, DHCP4_MULTI_THREADING_WARNING);
+        }
+    } catch (const std::exception& ex) {
+        err << "Error applying multi threading settings: "
+            << ex.what();
+        return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()));
+    }
+
     return (result);
 }
 
@@ -584,6 +596,8 @@ ControlledDhcpv4Srv::commandConfigBackendPullHandler(const std::string&,
         return (createAnswer(CONTROL_RESULT_EMPTY, "No config backend."));
     }
 
+    MultiThreadingCriticalSection cs;
+
     // Reschedule the periodic CB fetch.
     if (TimerMgr::instance()->isTimerRegistered("Dhcp4CBFetchTimer")) {
         TimerMgr::instance()->cancel("Dhcp4CBFetchTimer");
@@ -883,18 +897,6 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
         // operation.
     }
 
-    // Configure multi threading
-    try {
-        CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading());
-        if (MultiThreadingMgr::instance().getMode()) {
-            LOG_FATAL(dhcp4_logger, DHCP4_MULTI_THREADING_WARNING);
-        }
-    } catch (const std::exception& ex) {
-        err << "Error applying multi threading settings: "
-            << ex.what();
-        return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()));
-    }
-
     return (answer);
 }
 
@@ -1176,6 +1178,8 @@ ControlledDhcpv4Srv::dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) {
 void
 ControlledDhcpv4Srv::cbFetchUpdates(const SrvConfigPtr& srv_cfg,
                                     boost::shared_ptr<unsigned> failure_count) {
+    MultiThreadingCriticalSection cs;
+
     try {
         // Fetch any configuration backend updates since our last fetch.
         server_->getCBControl()->databaseConfigFetch(srv_cfg,
diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index d1ffda7afb..7e49af0c8c 100644
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -418,6 +418,18 @@ ControlledDhcpv6Srv::commandConfigSetHandler(const string&,
         CfgMgr::instance().getCurrentCfg()->applyLoggingCfg();
     }
 
+    // Configure multi threading
+    try {
+        CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading());
+        if (MultiThreadingMgr::instance().getMode()) {
+            LOG_FATAL(dhcp6_logger, DHCP6_MULTI_THREADING_WARNING);
+        }
+    } catch (const std::exception& ex) {
+        err << "Error applying multi threading settings: "
+            << ex.what();
+        return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()));
+    }
+
     return (result);
 }
 
@@ -587,6 +599,8 @@ ControlledDhcpv6Srv::commandConfigBackendPullHandler(const std::string&,
         return (createAnswer(CONTROL_RESULT_EMPTY, "No config backend."));
     }
 
+    MultiThreadingCriticalSection cs;
+
     // Reschedule the periodic CB fetch.
     if (TimerMgr::instance()->isTimerRegistered("Dhcp6CBFetchTimer")) {
         TimerMgr::instance()->cancel("Dhcp6CBFetchTimer");
@@ -904,18 +918,6 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
         // operation.
     }
 
-    // Configure multi threading
-    try {
-        CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading());
-        if (MultiThreadingMgr::instance().getMode()) {
-            LOG_FATAL(dhcp6_logger, DHCP6_MULTI_THREADING_WARNING);
-        }
-    } catch (const std::exception& ex) {
-        err << "Error applying multi threading settings: "
-            << ex.what();
-        return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()));
-    }
-
     return (answer);
 }
 
@@ -1194,6 +1196,8 @@ ControlledDhcpv6Srv::dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) {
 void
 ControlledDhcpv6Srv::cbFetchUpdates(const SrvConfigPtr& srv_cfg,
                                     boost::shared_ptr<unsigned> failure_count) {
+    MultiThreadingCriticalSection cs;
+
     try {
         // Fetch any configuration backend updates since our last fetch.
         server_->getCBControl()->databaseConfigFetch(srv_cfg,
diff --git a/src/lib/dhcpsrv/cfg_multi_threading.h b/src/lib/dhcpsrv/cfg_multi_threading.h
index 5eab92e46c..f7e07fa68f 100644
--- a/src/lib/dhcpsrv/cfg_multi_threading.h
+++ b/src/lib/dhcpsrv/cfg_multi_threading.h
@@ -26,7 +26,7 @@ public:
     /// @param[in] value The multi-threading configuration
     /// @param[out] enabled The enabled flag
     /// @param[out] thread_count The thread count
-    /// @param[out] queue size The queue size
+    /// @param[out] queue_size The queue size
     static void extract(data::ConstElementPtr value, bool& enabled,
                         uint32_t& thread_count, uint32_t& queue_size);
 };
diff --git a/src/lib/dhcpsrv/parsers/multi_threading_config_parser.h b/src/lib/dhcpsrv/parsers/multi_threading_config_parser.h
index 436c5714fa..bd285acbbe 100644
--- a/src/lib/dhcpsrv/parsers/multi_threading_config_parser.h
+++ b/src/lib/dhcpsrv/parsers/multi_threading_config_parser.h
@@ -17,9 +17,13 @@ namespace dhcp  {
 class MultiThreadingConfigParser : public isc::data::SimpleParser {
 public:
 
-    /// @brief parses JSON structure
+    /// @brief parses JSON structure.
     ///
-    /// @param srv_cfg parsed value will be stored here
+    /// This function stores the 'multi-threading' settings in the server
+    /// configuration and applies the MT mode so that is can be checked when
+    /// parsing 'hooks-libraries'.
+    ///
+    /// @param srv_cfg parsed value will be stored here.
     /// @param value a JSON map that contains multi-threading parameters.
     void parse(SrvConfig& srv_cfg, const isc::data::ConstElementPtr& value);
 };
diff --git a/src/lib/dhcpsrv/tests/multi_threading_config_parser_unittest.cc b/src/lib/dhcpsrv/tests/multi_threading_config_parser_unittest.cc
index 1f3b994bbd..4e750f9817 100644
--- a/src/lib/dhcpsrv/tests/multi_threading_config_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/multi_threading_config_parser_unittest.cc
@@ -92,6 +92,8 @@ TEST_F(MultiThreadingConfigParserTest, validContent) {
                 ADD_FAILURE() << "parser threw an exception: " << ex.what();
             }
 
+            EXPECT_EQ(MultiThreadingMgr::instance().getMode(), enabled);
+
             multi_threading_config = srv_config.getDHCPMultiThreading();
             // Verify the resultant configuration.
             ASSERT_TRUE(multi_threading_config);
@@ -103,7 +105,6 @@ TEST_F(MultiThreadingConfigParserTest, validContent) {
             CfgMultiThreading::extract(multi_threading_config, enabled,
                                        thread_count, queue_size);
 
-            EXPECT_EQ(MultiThreadingMgr::instance().getMode(), enabled);
 
             EXPECT_TRUE(multi_threading_config->equals(*config_elems));
         }