[#1282] Addressed comments

diff --git a/src/bin/agent/simple_parser.cc b/src/bin/agent/simple_parser.cc
index c5eeababc45a307808f8cc99260819369cf209f9..7f79439e46a7e07f3577ef470a075c9da5fe7bac 100644 (file)
--- a/src/bin/agent/simple_parser.cc
+++ b/src/bin/agent/simple_parser.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2017-2019 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2017-2020 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -9,6 +9,7 @@
 #include <agent/simple_parser.h>
 #include <cc/data.h>
 #include <cc/dhcp_config_error.h>
+#include <hooks/hooks_manager.h>
 #include <hooks/hooks_parser.h>
 #include <boost/foreach.hpp>
 
@@ -118,9 +119,11 @@ AgentSimpleParser::parse(const CtrlAgentCfgContextPtr& ctx,
         // This occurs last as if it succeeds, there is no easy way
         // revert it.  As a result, the failure to commit a subsequent
         // change causes problems when trying to roll back.
+        HooksManager::prepareUnloadLibraries();
+        static_cast<void>(HooksManager::unloadLibraries());
         libraries.loadLibraries();
     }
 }
 
-};
-};
+}
+}

diff --git a/src/bin/netconf/simple_parser.cc b/src/bin/netconf/simple_parser.cc
index 0a4734b05865e08c9921c20457f655c873bf5f1a..723f2fcdf5e28c365f965e831d3e83974b9610c0 100644 (file)
--- a/src/bin/netconf/simple_parser.cc
+++ b/src/bin/netconf/simple_parser.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -10,6 +10,7 @@
 #include <netconf/netconf_config.h>
 #include <cc/data.h>
 #include <cc/dhcp_config_error.h>
+#include <hooks/hooks_manager.h>
 #include <hooks/hooks_parser.h>
 #include <boost/foreach.hpp>
 
@@ -183,9 +184,11 @@ NetconfSimpleParser::parse(const NetconfConfigPtr& ctx,
         // This occurs last as if it succeeds, there is no easy way
         // revert it.  As a result, the failure to commit a subsequent
         // change causes problems when trying to roll back.
+        HooksManager::prepareUnloadLibraries();
+        static_cast<void>(HooksManager::unloadLibraries());
         libraries.loadLibraries();
     }
 }
 
-};
-};
+}
+}

diff --git a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc
index ae9a55534cca9d700ab48e9f23b2b40e82efe061..5bcb12140ca99202948669d3dba5359244e583a3 100644 (file)
--- a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc
+++ b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc
@@ -206,19 +206,17 @@ CloseHATest::runPartners() {
 
         wthread_->markReady(WatchedThread::READY);
 
-        // In fact below only the terminate matters as in fact
-        // accepts have not the time to be called.
         for (;;) {
             int nfd;
             fd_set fds;
             FD_ZERO(&fds);
             FD_SET(wthread_->getWatchFd(WatchedThread::TERMINATE), &fds);
             nfd = wthread_->getWatchFd(WatchedThread::TERMINATE);
-            // FD_SET(accept_partner1, &fds);
+            FD_SET(accept_partner1, &fds);
             if (accept_partner1 > nfd) {
                 nfd = accept_partner1;
             }
-            // FD_SET(accept_partner2, &fds);
+            FD_SET(accept_partner2, &fds);
             if (accept_partner2 > nfd) {
                 nfd = accept_partner2;
             }
@@ -502,7 +500,6 @@ TEST_F(CloseHATest, close6) {
         // Use the query callout handle.
         CalloutHandlePtr handle = query->getCalloutHandle();
         ScopedCalloutHandleState handle_state(handle);
-        handle->setStatus(CalloutHandle::NEXT_STEP_SKIP);
         handle->setArgument("query6", query);
         HooksManager::callCallouts(testHooks.hook_index_buffer6_receive_,
                                    *handle);

diff --git a/src/lib/hooks/hooks_manager.cc b/src/lib/hooks/hooks_manager.cc
index 0f13eeda802c3d0204b91562e4f7350b233c56c4..5c8985c071538dab1efee7bd1fa36fe2e5cb76a5 100644 (file)
--- a/src/lib/hooks/hooks_manager.cc
+++ b/src/lib/hooks/hooks_manager.cc
@@ -28,7 +28,12 @@ namespace hooks {
 // Constructor
 
 HooksManager::HooksManager() : test_mode_(false) {
-    init();
+    // Nothing present, so create the collection with any empty set of
+    // libraries, and get the CalloutManager.
+    HookLibsCollection libraries;
+    lm_collection_.reset(new LibraryManagerCollection(libraries));
+    lm_collection_->loadLibraries();
+    callout_manager_ = lm_collection_->getCalloutManager();
 }
 
 // Return reference to singleton hooks manager.
@@ -118,7 +123,7 @@ HooksManager::loadLibrariesInternal(const HookLibsCollection& libraries) {
     } else {
         // Unable to load libraries, reset to state before this function was
         // called.
-        init();
+        static_cast<void>(unloadLibrariesInternal());
     }
 
     return (status);
@@ -166,7 +171,8 @@ HooksManager::unloadLibrariesInternal() {
     return (true);
 }
 
-bool HooksManager::unloadLibraries() {
+bool
+HooksManager::unloadLibraries() {
     return (getHooksManager().unloadLibrariesInternal());
 }
 
@@ -179,7 +185,8 @@ HooksManager::prepareUnloadLibrariesInternal() {
     static_cast<void>(lm_collection_->prepareUnloadLibraries());
 }
 
-void HooksManager::prepareUnloadLibraries() {
+void
+HooksManager::prepareUnloadLibraries() {
     getHooksManager().prepareUnloadLibrariesInternal();
 }
 
@@ -217,18 +224,6 @@ HooksManager::getLibraryInfo() {
     return (getHooksManager().getLibraryInfoInternal());
 }
 
-// Perform initialization
-
-void
-HooksManager::init() {
-    // Nothing present, so create the collection with any empty set of
-    // libraries, and get the CalloutManager.
-    HookLibsCollection libraries;
-    lm_collection_.reset(new LibraryManagerCollection(libraries));
-    lm_collection_->loadLibraries();
-    callout_manager_ = lm_collection_->getCalloutManager();
-}
-
 // Shell around ServerHooks::registerHook()
 
 int

diff --git a/src/lib/hooks/hooks_manager.h b/src/lib/hooks/hooks_manager.h
index 4946a01bbde5fbba334e200a5932b4675e3bba5b..ea61daf26a109a23cf0a62124b70aa8b87d14f13 100644 (file)
--- a/src/lib/hooks/hooks_manager.h
+++ b/src/lib/hooks/hooks_manager.h
@@ -474,23 +474,16 @@ private:
 
     //@}
 
-    /// @brief Initialization to No Libraries
-    ///
-    /// Initializes the hooks manager with an empty set of libraries.
-    void init();
-
     // Members
 
     /// Set of library managers.
     ///
-    /// @node: This should always be initialized using @ref init or
-    /// @ref loadLibraries.
+    /// @node: This should never be null.
     boost::shared_ptr<LibraryManagerCollection> lm_collection_;
 
     /// Callout manager for the set of library managers.
     ///
-    /// @node: This should always be initialized using @ref init or
-    /// @ref loadLibraries.
+    /// @node: This should never be null.
     boost::shared_ptr<CalloutManager> callout_manager_;
 
     /// Test flag to keep @ref callout_manager_ when calling @ref loadLibraries

diff --git a/src/lib/hooks/hooks_messages.cc b/src/lib/hooks/hooks_messages.cc
index 7d2d0ee878ebf87c7505530f26ac398df2d814d9..a52c63541372f549478604005f965a4f6bec747f 100644 (file)
--- a/src/lib/hooks/hooks_messages.cc
+++ b/src/lib/hooks/hooks_messages.cc
@@ -1,4 +1,4 @@
-// File created from ../../../src/lib/hooks/hooks_messages.mes on Wed Jun 24 2020 22:17
+// File created from ../../../src/lib/hooks/hooks_messages.mes on Tue Jun 30 2020 14:43
 
 #include <cstddef>
 #include <log/message_types.h>
@@ -26,7 +26,6 @@ extern const isc::log::MessageID HOOKS_LIBRARY_MULTI_THREADING_COMPATIBLE = "HOO
 extern const isc::log::MessageID HOOKS_LIBRARY_MULTI_THREADING_NOT_COMPATIBLE = "HOOKS_LIBRARY_MULTI_THREADING_NOT_COMPATIBLE";
 extern const isc::log::MessageID HOOKS_LIBRARY_UNLOADED = "HOOKS_LIBRARY_UNLOADED";
 extern const isc::log::MessageID HOOKS_LIBRARY_UNLOADING = "HOOKS_LIBRARY_UNLOADING";
-extern const isc::log::MessageID HOOKS_LIBRARY_UNLOAD_ALREADY_CALLED = "HOOKS_LIBRARY_UNLOAD_ALREADY_CALLED";
 extern const isc::log::MessageID HOOKS_LIBRARY_VERSION = "HOOKS_LIBRARY_VERSION";
 extern const isc::log::MessageID HOOKS_LOAD_ERROR = "HOOKS_LOAD_ERROR";
 extern const isc::log::MessageID HOOKS_LOAD_EXCEPTION = "HOOKS_LOAD_EXCEPTION";
@@ -69,7 +68,6 @@ const char* values[] = {
     "HOOKS_LIBRARY_MULTI_THREADING_NOT_COMPATIBLE", "hooks library %1 is not compatible with multi-threading",
     "HOOKS_LIBRARY_UNLOADED", "hooks library %1 successfully unloaded",
     "HOOKS_LIBRARY_UNLOADING", "unloading library %1",
-    "HOOKS_LIBRARY_UNLOAD_ALREADY_CALLED", "hooks library %1 unload() was already called",
     "HOOKS_LIBRARY_VERSION", "hooks library %1 reports its version as %2",
     "HOOKS_LOAD_ERROR", "'load' function in hook library %1 returned error %2",
     "HOOKS_LOAD_EXCEPTION", "'load' function in hook library %1 threw an exception",

diff --git a/src/lib/hooks/hooks_messages.h b/src/lib/hooks/hooks_messages.h
index a1b0ac32159e4158c5ca546af42d45824f7e6002..2998bea7866f687ad56c96a235e6b88cb48bfdef 100644 (file)
--- a/src/lib/hooks/hooks_messages.h
+++ b/src/lib/hooks/hooks_messages.h
@@ -1,4 +1,4 @@
-// File created from ../../../src/lib/hooks/hooks_messages.mes on Wed Jun 24 2020 22:17
+// File created from ../../../src/lib/hooks/hooks_messages.mes on Tue Jun 30 2020 14:43
 
 #ifndef HOOKS_MESSAGES_H
 #define HOOKS_MESSAGES_H
@@ -27,7 +27,6 @@ extern const isc::log::MessageID HOOKS_LIBRARY_MULTI_THREADING_COMPATIBLE;
 extern const isc::log::MessageID HOOKS_LIBRARY_MULTI_THREADING_NOT_COMPATIBLE;
 extern const isc::log::MessageID HOOKS_LIBRARY_UNLOADED;
 extern const isc::log::MessageID HOOKS_LIBRARY_UNLOADING;
-extern const isc::log::MessageID HOOKS_LIBRARY_UNLOAD_ALREADY_CALLED;
 extern const isc::log::MessageID HOOKS_LIBRARY_VERSION;
 extern const isc::log::MessageID HOOKS_LOAD_ERROR;
 extern const isc::log::MessageID HOOKS_LOAD_EXCEPTION;

diff --git a/src/lib/hooks/hooks_messages.mes b/src/lib/hooks/hooks_messages.mes
index 061d457adeb6df72363e57a14fbba5923ef3ad58..e33e7e4a2edf73f771bd69ab41a8c077d716bf05 100644 (file)
--- a/src/lib/hooks/hooks_messages.mes
+++ b/src/lib/hooks/hooks_messages.mes
@@ -107,10 +107,6 @@ from the configuration or the multi-threading disabled.
 This information message is issued when a user-supplied hooks library
 has been successfully unloaded.
 
-% HOOKS_LIBRARY_UNLOAD_ALREADY_CALLED hooks library %1 unload() was already called
-This warning message is issued when the unload() function of a
-user-supplied hooks library was already called.
-
 % HOOKS_LIBRARY_UNLOADING unloading library %1
 This is a debug message called when the specified library is
 being unloaded.  If all is successful, it will be followed by the

diff --git a/src/lib/hooks/library_manager.cc b/src/lib/hooks/library_manager.cc
index cd5bc677dfcc9cfd413d7a026ddfa125448c8383..447c9ebb883975df7cce5d10d27220547a8730b4 100644 (file)
--- a/src/lib/hooks/library_manager.cc
+++ b/src/lib/hooks/library_manager.cc
@@ -253,9 +253,7 @@ LibraryManager::prepareUnloadLibrary() {
 
     // Call once.
     if (index_ < 0) {
-        LOG_WARN(hooks_logger, HOOKS_LIBRARY_UNLOAD_ALREADY_CALLED)
-            .arg(library_name_);
-        return (false);
+        return (true);
     }
 
     // Get the pointer to the "load" function.