[2995] Merge branch 'trac2980' into trac2995

- Also code updated to match changes from 2980

Conflicts:
	src/lib/hooks/callout_handle.h

configure.ac

@@ -1402,8 +1402,8 @@ AC_OUTPUT([doc/version.ent
            src/lib/cc/session_config.h.pre
            src/lib/cc/tests/session_unittests_config.h
            src/lib/datasrc/datasrc_config.h.pre
-           src/lib/hooks/tests/library_manager_unittest.cc
            src/lib/hooks/tests/marker_file.h
+           src/lib/hooks/tests/test_libraries.h
            src/lib/log/tests/console_test.sh
            src/lib/log/tests/destination_test.sh
            src/lib/log/tests/init_logger_test.sh

doc/Doxyfile

@@ -679,6 +679,7 @@ INPUT                  = ../src/lib/exceptions \
                          ../src/lib/cache \
                          ../src/lib/server_common/ \
                          ../src/bin/sockcreator/ \
+                         ../src/lib/hooks/ \
                          ../src/lib/util/ \
                          ../src/lib/util/io/ \
                          ../src/lib/util/threads/ \

doc/devel/mainpage.dox

@@ -1,11 +1,33 @@
+// Copyright (C) 2012-2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
 /**
- *
  * @mainpage BIND10 Developer's Guide
  *
  * Welcome to BIND10 Developer's Guide. This documentation is addressed
- * at existing and prospecting developers and programmers, who would like
+ * at existing and prospecting developers and programmers and provides
- * to gain insight into internal workings of BIND 10. It could also be useful
+ * information needed to both extend and maintain BIND 10.
- * for existing and prospective contributors.
+ *
+ * If you wish to write "hook" code - code that is loaded by BIND 10 at
+ * run-time and modifies its behavior you should read the section
+ * @ref hookDevelopersGuide.
+ *
+ * BIND 10 maintanenace information is divided into a number of sections
+ * depending on focus.  DNS-specific issues are covered in the
+ * @ref dnsMaintenanceGuide while information on DHCP-specific topics can
+ * be found in the @ref dhcpMaintenanceGuide.  General BIND 10 topics, not
+ * specific to any protocol, are discussed in @ref miscellaneousTopics.
  *
  * If you are a user or system administrator, rather than software engineer,
  * you should read <a href="http://bind10.isc.org/docs/bind10-guide.html">BIND10
@@ -13,13 +35,15 @@
  *
  * Regardless of your field of expertise, you are encouraged to visit
  * <a href="http://bind10.isc.org/">BIND10 webpage (http://bind10.isc.org)</a>
+ * @section hooksFramework Hooks Framework
+ * - @subpage hooksComponentDeveloperGuide
  *
- * @section DNS
+ * @section dnsMaintenanceGuide DNS Maintenance Guide
  * - Authoritative DNS (todo)
  * - Recursive resolver (todo)
  * - @subpage DataScrubbing
  *
- * @section DHCP
+ * @section dhcpMaintenanceGuide DHCP Maintenance Guide
  * - @subpage dhcp4
  *   - @subpage dhcpv4Session
  *   - @subpage dhcpv4ConfigParser
@@ -40,7 +64,7 @@
  * - @subpage dhcpDatabaseBackends
  * - @subpage perfdhcpInternals
  *
- * @section misc Miscellaneous topics
+ * @section miscellaneousTopics Miscellaneous topics
  * - @subpage LoggingApi
  *   - @subpage LoggingApiOverview
  *   - @subpage LoggingApiLoggerNames
@@ -48,7 +72,10 @@
  * - @subpage SocketSessionUtility
  * - <a href="./doxygen-error.log">Documentation warnings and errors</a>
  *
- * @todo: Move this logo to the right (and possibly up). Not sure what
- * is the best way to do it in Doxygen, without using CSS hacks.
  * @image html isc-logo.png
  */
+/*
+ * @todo: Move the logo to the right (and possibly up). Not sure what
+ * is the best way to do it in Doxygen, without using CSS hacks.
+ */
+

src/bin/dhcp6/dhcp6_srv.cc

@@ -37,7 +37,6 @@
 #include <util/io_utilities.h>
 #include <util/range_utilities.h>
 #include <util/encode/hex.h>
-#include <hooks/server_hooks.h>
 #include <hooks/hooks_manager.h>
 #include <hooks/callout_handle.h>
 
@@ -113,13 +112,13 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
         alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
 
         // Register hook points
-        hook_index_pkt6_receive_  = ServerHooks::getServerHooks().registerHook("pkt6_receive");
+        hook_index_pkt6_receive_   = HooksManager::registerHook("pkt6_receive");
-        hook_index_subnet6_select_ = ServerHooks::getServerHooks().registerHook("subnet6_select");
+        hook_index_subnet6_select_ = HooksManager::registerHook("subnet6_select");
-        hook_index_pkt6_send_      = ServerHooks::getServerHooks().registerHook("pkt6_send");
+        hook_index_pkt6_send_      = HooksManager::registerHook("pkt6_send");
 
         /// @todo call loadLibraries() when handling configuration changes
         vector<string> libraries; // no libraries at this time
-        HooksManager::getHooksManager().loadLibraries(libraries);
+        HooksManager::loadLibraries(libraries);
 
     } catch (const std::exception &e) {
         LOG_ERROR(dhcp6_logger, DHCP6_SRV_CONSTRUCT_ERROR).arg(e.what());

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -36,7 +36,6 @@
 #include <util/range_utilities.h>
 
 #include <hooks/server_hooks.h>
-#include <hooks/callout_manager.h>
 #include <hooks/hooks_manager.h>
 
 #include <boost/scoped_ptr.hpp>
@@ -1912,9 +1911,6 @@ public:
 
         // clear static buffers
         resetCalloutBuffers();
-
-        // Let's pretent we're the library 0
-        EXPECT_NO_THROW(HooksManager::getCalloutManager()->setLibraryIndex(0));
     }
 
     ~HooksDhcpv6SrvTest() {
@@ -2116,8 +2112,8 @@ vector<string> HooksDhcpv6SrvTest::callback_argument_names_;
 TEST_F(HooksDhcpv6SrvTest, simple_pkt6_receive) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_receive",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                                                                       pkt6_receive_callout));
+                        "pkt6_receive", pkt6_receive_callout));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2149,8 +2145,8 @@ TEST_F(HooksDhcpv6SrvTest, simple_pkt6_receive) {
 TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_receive) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_receive",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                                                       pkt6_receive_change_clientid));
+                        "pkt6_receive", pkt6_receive_change_clientid));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2185,8 +2181,8 @@ TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_receive) {
 TEST_F(HooksDhcpv6SrvTest, deleteClientId_pkt6_receive) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_receive",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                    pkt6_receive_delete_clientid));
+                        "pkt6_receive", pkt6_receive_delete_clientid));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2209,8 +2205,8 @@ TEST_F(HooksDhcpv6SrvTest, deleteClientId_pkt6_receive) {
 TEST_F(HooksDhcpv6SrvTest, skip_pkt6_receive) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_receive",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                    pkt6_receive_skip));
+                        "pkt6_receive", pkt6_receive_skip));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2234,8 +2230,8 @@ TEST_F(HooksDhcpv6SrvTest, skip_pkt6_receive) {
 TEST_F(HooksDhcpv6SrvTest, simple_pkt6_send) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_send",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                                                                       pkt6_send_callout));
+                        "pkt6_send", pkt6_send_callout));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2270,8 +2266,8 @@ TEST_F(HooksDhcpv6SrvTest, simple_pkt6_send) {
 TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_send) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_send",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                                                       pkt6_send_change_serverid));
+                        "pkt6_send", pkt6_send_change_serverid));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2307,8 +2303,8 @@ TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_send) {
 TEST_F(HooksDhcpv6SrvTest, deleteServerId_pkt6_send) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_send",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                    pkt6_send_delete_serverid));
+                        "pkt6_send", pkt6_send_delete_serverid));
 
     // Let's create a simple SOLICIT
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2338,8 +2334,8 @@ TEST_F(HooksDhcpv6SrvTest, deleteServerId_pkt6_send) {
 TEST_F(HooksDhcpv6SrvTest, skip_pkt6_send) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("pkt6_send",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                    pkt6_send_skip));
+                        "pkt6_send", pkt6_send_skip));
 
     // Let's create a simple REQUEST
     Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
@@ -2362,8 +2358,8 @@ TEST_F(HooksDhcpv6SrvTest, skip_pkt6_send) {
 TEST_F(HooksDhcpv6SrvTest, subnet6_select) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("subnet6_select",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                    subnet6_select_callout));
+                        "subnet6_select", subnet6_select_callout));
 
     // Configure 2 subnets, both directly reachable over local interface
     // (let's not complicate the matter with relays)
@@ -2430,8 +2426,8 @@ TEST_F(HooksDhcpv6SrvTest, subnet6_select) {
 TEST_F(HooksDhcpv6SrvTest, subnet_select_change) {
 
     // Install pkt6_receive_callout
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("subnet6_select",
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-                    subnet6_select_different_subnet_callout));
+                        "subnet6_select", subnet6_select_different_subnet_callout));
 
     // Configure 2 subnets, both directly reachable over local interface
     // (let's not complicate the matter with relays)

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -1154,10 +1154,8 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     HooksManager::getHooksManager().loadLibraries(libraries);
 
     // Install pkt6_receive_callout
-    ASSERT_TRUE(HooksManager::getCalloutManager());
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->setLibraryIndex(0));
+                        "lease6_select", lease6_select_callout));
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("lease6_select",
-                                                                       lease6_select_callout));
 
     CalloutHandlePtr callout_handle = HooksManager::getHooksManager().createCalloutHandle();
 
@@ -1217,10 +1215,8 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
     HooksManager::getHooksManager().loadLibraries(libraries);
 
     // Install a callout
-    ASSERT_TRUE(HooksManager::getCalloutManager());
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->setLibraryIndex(0));
+                        "lease6_select", lease6_select_different_callout));
-    EXPECT_NO_THROW(HooksManager::getCalloutManager()->registerCallout("lease6_select",
-                    lease6_select_different_callout));
 
     // Normally, dhcpv6_srv would passed the handle when calling allocateAddress6,
     // but in tests we need to create it on our own.

src/lib/hooks/Makefile.am

@@ -31,10 +31,11 @@ libb10_hooks_la_SOURCES += callout_handle.cc callout_handle.h
 libb10_hooks_la_SOURCES += callout_manager.cc callout_manager.h
 libb10_hooks_la_SOURCES += hooks.h
 libb10_hooks_la_SOURCES += hooks_log.cc hooks_log.h
+libb10_hooks_la_SOURCES += hooks_manager.cc hooks_manager.h
 libb10_hooks_la_SOURCES += library_handle.cc library_handle.h
 libb10_hooks_la_SOURCES += library_manager.cc library_manager.h
+libb10_hooks_la_SOURCES += library_manager_collection.cc library_manager_collection.h
 libb10_hooks_la_SOURCES += server_hooks.cc server_hooks.h
-libb10_hooks_la_SOURCES += hooks_manager.cc hooks_manager.h
 
 nodist_libb10_hooks_la_SOURCES = hooks_messages.cc hooks_messages.h
 

src/lib/hooks/callout_handle.cc

@@ -27,8 +27,10 @@ namespace isc {
 namespace hooks {
 
 // Constructor.
-CalloutHandle::CalloutHandle(const boost::shared_ptr<CalloutManager>& manager)
+CalloutHandle::CalloutHandle(const boost::shared_ptr<CalloutManager>& manager,
-    : arguments_(), context_collection_(), manager_(manager), skip_(false) {
+                    const boost::shared_ptr<LibraryManagerCollection>& lmcoll)
+    : lm_collection_(lmcoll), arguments_(), context_collection_(),
+      manager_(manager), skip_(false) {
 
     // Call the "context_create" hook.  We should be OK doing this - although
     // the constructor has not finished running, all the member variables
@@ -43,6 +45,24 @@ CalloutHandle::~CalloutHandle() {
     // the destructor is being called, all the member variables are still in
     // existence.
     manager_->callCallouts(ServerHooks::CONTEXT_DESTROY, *this);
+
+    // Explicitly clear the argument and context objects.  This should free up
+    // all memory that could have been allocated by libraries that were loaded.
+    arguments_.clear();
+    context_collection_.clear();
+
+    // Normal destruction of the remaining variables will include the
+    // destruction of lm_collection_, an action that decrements the reference
+    // count on the library manager collection (which holds the libraries that
+    // could have allocated memory in the argument and context members.)  When
+    // that goes to zero, the libraries will be unloaded: at that point nothing
+    // in the hooks framework will be pointing to memory in the libraries'
+    // address space.
+    //
+    // It is possible that some other data structure in the server (the program
+    // using the hooks library) still references the address space and attempts
+    // to access it causing a segmentation fault. That issue is outside the
+    // scope of this framework and is not addressed by it.
 }
 
 // Return the name of all argument items.
@@ -130,8 +150,9 @@ CalloutHandle::getHookName() const {
     try {
         hook = ServerHooks::getServerHooks().getName(index);
     } catch (const NoSuchHook&) {
-        // Hook index is invalid, so probably called outside of a callout.
+        // Hook index is invalid, so this methods probably called from outside
-        // This is a no-op.
+        // a callout being executed via a call to CalloutManager::callCallouts.
+        // In this case, the empty string is returned.
     }
 
     return (hook);

src/lib/hooks/callout_handle.h

@@ -54,6 +54,7 @@ public:
 
 class CalloutManager;
 class LibraryHandle;
+class LibraryManagerCollection;
 
 /// @brief Per-packet callout handle
 ///
@@ -79,11 +80,11 @@ class LibraryHandle;
 ///   "context_destroy" callout.  The information is accessed through the
 ///   {get,set}Context() methods.
 ///
-/// - Per-library handle.  Allows the callout to dynamically register and
+/// - Per-library handle (LibraryHandle). The library handle allows the
-///   deregister callouts. (In the latter case, only functions registered by
+///   callout to dynamically register and deregister callouts. In the latter
-///   functions in the same library as the callout doing the deregistration
+///   case, only functions registered by functions in the same library as the
-///   can be removed: callouts registered by other libraries cannot be
+///   callout doing the deregistration can be removed: callouts registered by
-///   modified.)
+///   other libraries cannot be modified.
 
 class CalloutHandle {
 public:
@@ -110,12 +111,27 @@ public:
     /// Creates the object and calls the callouts on the "context_create"
     /// hook.
     ///
+    /// Of the two arguments passed, only the pointer to the callout manager is
+    /// actively used.  The second argument, the pointer to the library manager
+    /// collection, is used for lifetime control: after use, the callout handle
+    /// may contain pointers to memory allocated by the loaded libraries.  The
+    /// used of a shared pointer to the collection of library managers means
+    /// that the libraries that could have allocated memory in a callout handle
+    /// will not be unloaded until all such handles have been destroyed.  This
+    /// issue is discussed in more detail in the documentation for
+    /// isc::hooks::LibraryManager.
+    ///
     /// @param manager Pointer to the callout manager object.
-    CalloutHandle(const boost::shared_ptr<CalloutManager>& manager);
+    /// @param lmcoll Pointer to the library manager collection.  This has a
+    ///        null default for testing purposes.
+    CalloutHandle(const boost::shared_ptr<CalloutManager>& manager,
+                  const boost::shared_ptr<LibraryManagerCollection>& lmcoll =
+                        boost::shared_ptr<LibraryManagerCollection>());
 
     /// @brief Destructor
     ///
     /// Calls the context_destroy callback to release any per-packet context.
+    /// It also clears stored data to avoid problems during member destruction.
     ~CalloutHandle();
 
     /// @brief Set argument
@@ -152,7 +168,7 @@ public:
 
         value = boost::any_cast<T>(element_ptr->second);
     }
-    
+
     /// @brief Get argument names
     ///
     /// Returns a vector holding the names of arguments in the argument
@@ -257,7 +273,7 @@ public:
 
         value = boost::any_cast<T>(element_ptr->second);
     }
-    
+
     /// @brief Get context names
     ///
     /// Returns a vector holding the names of items in the context associated
@@ -340,6 +356,10 @@ private:
 
     // Member variables
 
+    /// Pointer to the collection of libraries for which this handle has been
+    /// created.
+    boost::shared_ptr<LibraryManagerCollection> lm_collection_;
+
     /// Collection of arguments passed to the callouts
     ElementCollection arguments_;
 
@@ -353,10 +373,10 @@ private:
     bool skip_;
 };
 
-/// a shared pointer to CalloutHandle object
+/// A shared pointer to a CalloutHandle object.
 typedef boost::shared_ptr<CalloutHandle> CalloutHandlePtr;
 
-} // namespace util
+} // namespace hooks
 } // namespace isc
 
 

src/lib/hooks/callout_manager.cc

@@ -19,6 +19,7 @@
 #include <boost/static_assert.hpp>
 
 #include <algorithm>
+#include <climits>
 #include <functional>
 #include <utility>
 
@@ -27,12 +28,41 @@ using namespace std;
 namespace isc {
 namespace hooks {
 
+// Check that the index of a library is valid.  It can range from 1 - n
+// (n is the number of libraries), 0 (pre-user library callouts), or INT_MAX
+// (post-user library callouts).  It can also be -1 to indicate an invalid
+// value.
+
+void
+CalloutManager::checkLibraryIndex(int library_index) const {
+    if (((library_index >= -1) && (library_index <= num_libraries_)) ||
+        (library_index == INT_MAX)) {
+        return;
+    }
+
+    isc_throw(NoSuchLibrary, "library index " << library_index <<
+              " is not valid for the number of loaded libraries (" <<
+              num_libraries_ << ")");
+}
+
+// Set the number of libraries handled by the CalloutManager.
+
+void
+CalloutManager::setNumLibraries(int num_libraries) {
+    if (num_libraries < 0) {
+        isc_throw(isc::BadValue, "number of libraries passed to the "
+                  "CalloutManager must be >= 0");
+    }
+
+    num_libraries_ = num_libraries;
+}
+
 // Register a callout for the current library.
 
 void
 CalloutManager::registerCallout(const std::string& name, CalloutPtr callout) {
     // Note the registration.
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_REGISTER_CALLOUT)
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUT_REGISTRATION)
         .arg(current_library_).arg(name);
 
     // Sanity check that the current library index is set to a valid value.
@@ -108,15 +138,24 @@ CalloutManager::callCallouts(int hook_index, CalloutHandle& callout_handle) {
             current_library_ = i->first;
 
             // Call the callout
-            // @todo Log the return status if non-zero
+            try {
-            int status = (*i->second)(callout_handle);
+                int status = (*i->second)(callout_handle);
-            if (status == 0) {
+                if (status == 0) {
-                LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS, HOOKS_CALLOUT)
+                    LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
-                    .arg(current_library_)
+                              HOOKS_CALLOUT_CALLED).arg(current_library_)
-                    .arg(ServerHooks::getServerHooks().getName(current_hook_))
+                        .arg(ServerHooks::getServerHooks()
-                    .arg(reinterpret_cast<void*>(i->second));
+                            .getName(current_hook_))
-            } else {
+                        .arg(reinterpret_cast<void*>(i->second));
-                LOG_WARN(hooks_logger, HOOKS_CALLOUT_ERROR)
+                } else {
+                    LOG_ERROR(hooks_logger, HOOKS_CALLOUT_ERROR)
+                        .arg(current_library_)
+                        .arg(ServerHooks::getServerHooks()
+                            .getName(current_hook_))
+                        .arg(reinterpret_cast<void*>(i->second));
+                }
+            } catch (...) {
+                // Any exception, not just ones based on isc::Exception
+                LOG_ERROR(hooks_logger, HOOKS_CALLOUT_EXCEPTION)
                     .arg(current_library_)
                     .arg(ServerHooks::getServerHooks().getName(current_hook_))
                     .arg(reinterpret_cast<void*>(i->second));
@@ -170,7 +209,7 @@ CalloutManager::deregisterCallout(const std::string& name, CalloutPtr callout) {
     bool removed = initial_size != hook_vector_[hook_index].size();
     if (removed) {
         LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
-                  HOOKS_DEREGISTER_CALLOUT).arg(current_library_).arg(name);
+                  HOOKS_CALLOUT_DEREGISTERED).arg(current_library_).arg(name);
     }
 
     return (removed);
@@ -205,7 +244,7 @@ CalloutManager::deregisterAllCallouts(const std::string& name) {
     bool removed = initial_size != hook_vector_[hook_index].size();
     if (removed) {
         LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
-                  HOOKS_DEREGISTER_ALL_CALLOUTS).arg(current_library_)
+                  HOOKS_ALL_CALLOUTS_DEREGISTERED).arg(current_library_)
                                                 .arg(name);
     }
 

src/lib/hooks/callout_manager.h

@@ -21,6 +21,7 @@
 
 #include <boost/shared_ptr.hpp>
 
+#include <climits>
 #include <map>
 #include <string>
 
@@ -37,11 +38,11 @@ public:
         isc::Exception(file, line, what) {}
 };
 
-
 /// @brief Callout Manager
 ///
 /// This class manages the registration, deregistration and execution of the
-/// library callouts.
+/// library callouts.  It is part of the hooks framework used by the BIND 10
+/// server, and is not for use by user-written code in a hooks library.
 ///
 /// In operation, the class needs to know two items of data:
 ///
@@ -59,14 +60,51 @@ public:
 ///   deregister callouts in the same library (including themselves): they
 ///   cannot affect callouts registered by another library.  When calling a
 ///   callout, the callout manager maintains the idea of a "current library
-///   index": if the callout calls one of the callout registration functions 
+///   index": if the callout calls one of the callout registration functions
 ///   (indirectly via the @ref LibraryHandle object), the registration
 ///   functions use the "current library index" in their processing.
 ///
 /// These two items of data are supplied when an object of this class is
-/// constructed.
+/// constructed.  The latter (number of libraries) can be updated after the
+/// class is constructed.  (Such an update is used during library loading where
+/// the CalloutManager has to be constructed before the libraries are loaded,
+/// but one of the libraries subsequently fails to load.)
 ///
-/// Note that the callout function do not access the library manager: instead,
+/// The library index is important because it determines in what order callouts
+/// on a particular hook are called.  For each hook, the CalloutManager
+/// maintains a vector of callouts ordered by library index.  When a callout
+/// is added to the list, it is added at the end of the callouts associated
+/// with the current library.  To clarify this further, suppose that three
+/// libraries are loaded, A (assigned an index 1), B (assigned an index 2) and
+/// C (assigned an index 3).  Suppose A registers two callouts on a given hook,
+/// A1 and A2 (in that order) B registers B1 and B2 (in that order) and C
+/// registers C1 and C2 (in that order).  Internally, the callouts are stored
+/// in the order A1, A2, B1, B2, C1, and C2: this is also the order in which
+/// the are called.  If B now registers another callout (B3), it is added to
+/// the vector after the list of callouts associated with B: the new order is
+/// therefore A1, A2, B1, B2, B3, C1 and C2.
+///
+/// Indexes range between 1 and n (where n is the number of the libraries
+/// loaded) and are assigned to libraries based on the order the libraries
+/// presented to the hooks framework for loading (something that occurs in the
+/// isc::util::HooksManager) class.  However, two other indexes are recognised,
+/// 0 and INT_MAX.  These are used when the server itself registers callouts -
+/// the server is able to register callouts that get called before any
+/// user-library callouts, and ones that get called after user-library callouts.
+/// In other words, assuming the callouts on a hook are A1, A2, B1, B2, B3, C2,
+/// C2 as before, and that the server registers S1 (to run before the
+/// user-registered callouts) and S2 (to run after them), the callouts are
+/// stored (and executed) in the order S1, A1, A2, B1, B2, B3, C2, C2, S2.  In
+/// summary, the recognised index values are:
+///
+/// - < 0: invalid.
+/// - 0: used for server-registered callouts that are called before
+///   user-registered callouts.
+/// - 1 - n: callouts from user libraries.
+/// - INT_MAX: used for server-registered callouts called after
+///   user-registered callouts.
+///
+/// Note that the callout functions do not access the CalloutManager: instead,
 /// they use a LibraryHandle object.  This contains an internal pointer to
 /// the CalloutManager, but provides a restricted interface.  In that way,
 /// callouts are unable to affect callouts supplied by other libraries.
@@ -96,17 +134,16 @@ public:
     ///
     /// @throw isc::BadValue if the number of libraries is less than or equal
     ///        to 0, or if the pointer to the server hooks object is empty.
-    CalloutManager(int num_libraries)
+    CalloutManager(int num_libraries = 0)
-        : current_hook_(-1), current_library_(-1), hook_vector_(),
+        : current_hook_(-1), current_library_(-1),
-          library_handle_(this), num_libraries_(num_libraries)
+          hook_vector_(ServerHooks::getServerHooks().getCount()),
+          library_handle_(this), pre_library_handle_(this, 0),
+          post_library_handle_(this, INT_MAX), num_libraries_(num_libraries)
     {
-        if (num_libraries <= 0) {
+        // Check that the number of libraries is OK.  (This does a redundant
-            isc_throw(isc::BadValue, "number of libraries passed to the "
+        // set of the number of libraries, but it's only a single assignment
-                      "CalloutManager must be >= 0");
+        // and avoids the need for a separate "check" method.
-        }
+        setNumLibraries(num_libraries);
-
-        // Parameters OK, do operations that depend on them.
-        hook_vector_.resize(ServerHooks::getServerHooks().getCount());
     }
 
     /// @brief Register a callout on a hook for the current library
@@ -187,12 +224,28 @@ public:
         return (current_hook_);
     }
 
+    /// @brief Set number of libraries
+    ///
+    /// Sets the number of libraries.  Although the value is passed to the
+    /// constructor, in some cases that is only an estimate and the number
+    /// can only be determined after the CalloutManager is created.
+    ///
+    /// @note If the number if libraries is reset, it must be done *before*
+    ///       any callouts are registered.
+    ///
+    /// @param num_libraries Number of libraries served by this CalloutManager.
+    ///
+    /// @throw BadValue Number of libraries must be >= 0.
+    /// @throw LibraryCountChanged Number of libraries has been changed after
+    ///        callouts have been registered.
+    void setNumLibraries(int num_libraries);
+
     /// @brief Get number of libraries
     ///
     /// Returns the number of libraries that this CalloutManager is expected
     /// to serve.  This is the number passed to its constructor.
     ///
-    /// @return Number of libraries server by this CalloutManager.
+    /// @return Number of libraries served by this CalloutManager.
     int getNumLibraries() const {
         return (num_libraries_);
     }
@@ -215,9 +268,14 @@ public:
 
     /// @brief Set current library index
     ///
-    /// Sets the current library index.  This must be in the range 0 to
+    /// Sets the current library index.  This has the following valid values:
-    /// (numlib - 1), where "numlib" is the number of libraries loaded in the
+    ///
-    /// system, this figure being passed to this object at construction time.
+    /// - -1: invalidate current index.
+    /// - 0: pre-user library callout.
+    /// - 1 - numlib: user-library callout (where "numlib" is the number of
+    ///   libraries loaded in the system, this figure being passed to this
+    ///   object at construction time).
+    /// - INT_MAX: post-user library callout.
     ///
     /// @param library_index New library index.
     ///
@@ -227,6 +285,19 @@ public:
         current_library_ = library_index;
     }
 
+    /// @defgroup calloutManagerLibraryHandles Callout manager library handles
+    ///
+    /// The CalloutManager offers three library handles:
+    ///
+    /// - a "standard" one, used to register and deregister callouts for
+    ///   the library index that is marked as current in the CalloutManager.
+    ///   When a callout is called, it is passed this one.
+    /// - a pre-library callout handle, used by the server to register
+    //    callouts to run prior to user-library callouts.
+    /// - a post-library callout handle, used by the server to register
+    ///   callouts to run after the user-library callouts.
+    //@{
+
     /// @brief Return library handle
     ///
     /// The library handle is available to the user callout via the callout
@@ -235,11 +306,33 @@ public:
     /// library of which it is part, whilst denying access to anything that
     /// may affect other libraries.
     ///
-    /// @return Reference to callout handle for this manager
+    /// @return Reference to library handle for this manager
     LibraryHandle& getLibraryHandle() {
         return (library_handle_);
     }
 
+    /// @brief Return pre-user callouts library handle
+    ///
+    /// The LibraryHandle to affect callouts that will run before the
+    /// user-library callouts.
+    ///
+    /// @return Reference to pre-user library handle for this manager
+    LibraryHandle& getPreLibraryHandle() {
+        return (pre_library_handle_);
+    }
+
+    /// @brief Return post-user callouts library handle
+    ///
+    /// The LibraryHandle to affect callouts that will run before the
+    /// user-library callouts.
+    ///
+    /// @return Reference to post-user library handle for this manager
+    LibraryHandle& getPostLibraryHandle() {
+        return (post_library_handle_);
+    }
+
+    //@}
+
 private:
     /// @brief Check library index
     ///
@@ -247,15 +340,11 @@ private:
     /// the hook registration functions.
     ///
     /// @param library_index Value to check for validity as a library index.
+    ///        Valid values are 0 - numlib+1 and -1: see @ref setLibraryIndex
+    ///        for the meaning of the various values.
     ///
     /// @throw NoSuchLibrary Library index is not valid.
-    void checkLibraryIndex(int library_index) const {
+    void checkLibraryIndex(int library_index) const;
-        if ((library_index < 0) || (library_index >= num_libraries_)) {
-            isc_throw(NoSuchLibrary, "library index " << library_index <<
-                      " is not valid for the number of loaded libraries (" <<
-                      num_libraries_ << ")");
-        }
-    }
 
     /// @brief Compare two callout entries for library equality
     ///
@@ -289,12 +378,22 @@ private:
     /// Vector of callout vectors.  There is one entry in this outer vector for
     /// each hook. Each element is itself a vector, with one entry for each
     /// callout registered for that hook.
-    std::vector<CalloutVector>  hook_vector_;
+    std::vector<CalloutVector> hook_vector_;
 
     /// LibraryHandle object user by the callout to access the callout
-    /// registration methods on this CalloutManager object.
+    /// registration methods on this CalloutManager object.  The object is set
+    /// such that the index of the library associated with any operation is
+    /// whatever is currently set in the CalloutManager.
     LibraryHandle library_handle_;
 
+    /// LibraryHandle for callouts to be registered as being called before
+    /// the user-registered callouts.
+    LibraryHandle pre_library_handle_;
+
+    /// LibraryHandle for callouts to be registered as being called after
+    /// the user-registered callouts.
+    LibraryHandle post_library_handle_;
+
     /// Number of libraries.
     int num_libraries_;
 };

src/lib/hooks/hooks.h

@@ -18,7 +18,21 @@
 #include <hooks/callout_handle.h>
 #include <hooks/library_handle.h>
 
-// Version 1.0
+namespace {
-static const int BIND10_HOOKS_VERSION = 1;
+
+// Version 1 of the hooks framework.
+const int BIND10_HOOKS_VERSION = 1;
+
+// Names of the framework functions.
+const char* LOAD_FUNCTION_NAME = "load";
+const char* UNLOAD_FUNCTION_NAME = "unload";
+const char* VERSION_FUNCTION_NAME = "version";
+
+// Typedefs for pointers to the framework functions.
+typedef int (*version_function_ptr)();
+typedef int (*load_function_ptr)(isc::hooks::LibraryHandle&);
+typedef int (*unload_function_ptr)();
+
+} // Anonymous namespace
 
 #endif  // HOOKS_H

src/lib/hooks/hooks_component_developer.dox

@@ -0,0 +1,483 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+/**
+@page hooksComponentDeveloperGuide Guide to Hooks for the BIND 10 Component Developer
+
+@section hooksComponentIntroduction Introduction
+
+The hooks framework is a BIND 10 system that simplifies the way that
+users can write code to modify the behavior of BIND 10.  Instead of
+altering the BIND 10 source code, they write functions that are compiled
+and linked into a shared library.  The library is specified in the BIND 10
+configuration database and run time, BIND 10 dynamically loads the library
+into its address space.  At various points in the processing, the component
+"calls out" to functions in the library, passing to them the data is it
+currently working on.  They can examine and modify the data as required.
+
+This guide is aimed at BIND 10 developers who want to write or modify a
+BIND 10 component to use hooks.  It shows how the component should be written
+to load a shared library at run-time and how to call functions in it.
+
+For information about writing a hooks library containing functions called by BIND 10
+during its execution, see the document @ref hooksDevelopersGuide.
+
+@subsection hooksComponentTerminology Terminology
+
+In the remainder of this guide, the following terminology is used:
+
+- Component - a BIND 10 process, e.g. the authoritative DNS server or the
+DHCPv4 server.
+
+- Hook/Hook Point - used interchageably, this is a point in the code at
+which a call to user-written functions is made. Each hook has a name and
+each hook can have any number (including 0) of user-written functions
+attached to it.
+
+- Callout - a user-written function called by the component at a hook
+point. This is so-named because the component "calls out" to the library
+to execute a user-written function.
+
+- User code/user library - non-BIND 10 code that is compiled into a
+shared library and loaded by BIND 10 into its address space.  Multiple
+user libraries can be loaded at the same time, each containing callouts for
+the same hooks.  The hooks framework calls these libraries one after the
+other. (See the document @ref hooksDevelopersGuide for more details.)
+
+@subsection hooksComponentLanguages Languages
+
+The core of BIND 10 is written in C++ with some parts in Python.  While it is
+the intention to provide the hooks framework for all languages, the initial
+version is for C++.  All examples in this guide are in that language.
+
+@section hooksComponentBasicIdeas Basic Ideas
+
+From the point of view of the component author, the basic ideas of the hooks
+framework are quite simple:
+
+- The location of hook points in the code need to be determined.
+
+- Name the hook points and register them.
+
+- At each hook point, the component needs to complete the following steps to
+  execute callouts registered by the user-library:
+  -# copy data into the object used to pass information to the callout.
+  -# call the callout.
+  -# copy data back from the object used to exchange information.
+  -# take action based on information returned.
+
+Of course, to set up the system the libraries need to be loaded in the first
+place.  The component also needs to:
+
+- Define the configuration item that specifies the user libraries for this
+component.
+
+- Handle configuration changes and load/unload the user libraries.
+
+The following sections will describe these tasks in more detail.
+
+@section hooksComponentDefinition Determing the Hook Points
+
+Before any other action takes place, the location of the hook points
+in the code need to be determined.  This of course depends on the
+component but as a general guideline, hook locations should be chosen
+where a callout is able to obtain useful information from BIND 10 and/or
+affect processing.  Typically this means at the start or end of a major
+step in the processing of a request, at a point where either useful
+information can be passed to a callout and/or the callout can affect
+the processing of the component. The latter is achieved in either or both
+of the following eays:
+
+- Setting the "skip" flag.  This is a boolean flag that the callout can set
+  and is a quick way of passing information back to the component.  It is used
+  to indicate that the component should skip the processing step associated with
+  the hook.  The exact action is up to the component, but is likely to be one
+  of skipping the processing step (probably because the callout has
+  done its own processing for the action) or dropping the current packet
+  and starting on a new request.
+
+- Modifying data passed to it.  The component should be prepared to continue
+  processing with the data returned by the callout.  It is up to the component
+  author whether the data is validated before being used, but doing so will
+  have performance implications.
+
+@section hooksComponentRegistration Naming and Registering the Hooks Points
+
+Once the location of the hook point has been determined, it should be
+given a name.  This name should be unique amongst all hook points and is
+subject to certain restrictions (see below).
+
+Before the callouts at any hook point are called and any user libraries
+loaded - so typically during component initialization - the component must
+register the names of all the hooks.  The registration is done using
+the static method isc::hooks::HooksManager::registerHook():
+
+@code
+
+#include <hooks/hooks_manager.h>
+    :
+    int example_index = HooksManager::registerHook("lease_allocate");
+@endcode
+
+The name of the hook is passed as the sole argument to the registerHook()
+method.  The value returned is the index of that hook point and should
+be retained - it is needed to call the callouts attached to that hook.
+
+Note that a hook only needs to be registered once.  There is no mechanism for
+unregistering a hook and there is no need to do so.
+
+@subsection hooksComponentAutomaticRegistration Automatic Registration of Hooks
+
+In some components, it may be convenient to set up a single initialization
+function that registers all hooks.  For others, it may be more convenient
+for each module within the component to perform its own initialization.
+Since the isc::hooks::HooksManager object is a singleton and is created when first
+accessed, a useful trick is to automatically register the hooks when
+the module is loaded.
+
+This technique involves declaring an object outside of any execution
+unit in the module.  When the module is loaded, the object's constructor
+is run.  By placing the hook registration calls in the constructor,
+the hooks in the module are defined at load time, before any function in
+the module is run.  The code for such an initialization sequence would
+be similar to:
+
+@code
+#include <hooks/hooks_manager.h>
+
+namespace {
+
+// Declare structure to perform initialization and store the hook indexes.
+//
+struct MyHooks {
+    int pkt_rcvd;   // Index of "packet received" hook
+    int pkt_sent;   // Index of "packet sent" hook
+
+    // Constructor
+    MyHooks() {
+        pkt_rcvd = HooksManager::registerHook("pkt_rcvd");
+        pkt_sent = HooksManager::registerHook("pkt_sent");
+    }
+};
+
+// Declare a "MyHooks" object.  As this is outside any function or method, it
+// will be instantiated (and the constructor run) when the module is loaded.
+// As a result, the hook indexes will be defined before any method in this
+// module is called.
+MyHooks my_hooks;
+
+} // Anonymous namespace
+
+void Someclass::someFunction() {
+    :
+    // Check if any callouts are defined on the pkt_rcvd hook.
+    if (HooksManager::calloutPresent(my_hooks.pkt_rcvd)) {
+          :
+    }
+    :
+}
+@endcode
+
+@subsection hooksComponentHookNames Hook Names
+
+Hook names are strings and in principle, any string can be used as the
+name of a hook, even one containing spaces and non-printable characters.
+However, the following guidelines should be observed:
+
+- The names <b>context_create</b> and <b>context_destroy</b> are reserved to
+the hooks system and are automatically registered: an attempt to register
+one of these will lead to a isc::hooks::DuplicateHook exception being thrown.
+
+- The hook name should be a valid "C" function name.  If a user gives a
+callout the same name as one of the hooks, the hooks framework will
+automatically load that callout and attach it to the hook: the user does not
+have to explicitly register it.
+
+- The hook name should not conflict with the name of a function in any of
+the system libraries (e.g. naming a hook "sqrt" could lead to the
+square-root function in the system's maths library being attached to the hook
+as a callout).
+
+- Although hook names can be in any case (including mixed case), the BIND 10
+convention is that they are lower-case.
+
+@section hooksComponentCallingCallouts Calling Callouts on a Hook
+
+@subsection hooksComponentArgument The Callout Handle
+
+Before describing how to call user code at a hook point, we must first consider
+how to pass data to it.
+
+Each user callout has the signature:
+@code
+int callout_name(isc::hooks::CalloutHandle& handle);
+@endcode
+
+The isc::hooks::CalloutHandle object is the object used to pass data to
+and from the callout.  This holds the data as a set of name/value pairs,
+each pair being considered an argument to the callout.  If there are
+multiple callouts attached to a hook, the CalloutHandle is passed to
+each in turn. Should a callout modify an argument, the updated data is
+passed subsequent callouts (each of which could also modify it) before
+being returned to the component.
+
+Two methods are provided to get and set the arguments passed to
+the callout called (naturally enough) getArgument and SetArgument.
+Their usage is illustrated by the following code snippets.
+
+@code
+    int count = 10;
+    boost::shared_ptr<Pkt4> pktptr = ... // Set to appropriate value
+
+    // Assume that "handle_ptr" has been created and is a pointer to a
+    // CalloutHandle.
+    handle_ptr->setArgument("data_count", count);
+    handle_ptr->setArgument("inpacket", pktptr);
+
+    // Call the hook code.  lease_assigned_index is the value returned from
+    // HooksManager::registerHook() when the hook was registered.
+    HooksManager::callCallouts(lease_assigned_index, *handle_ptr);
+
+    // Retrieve the modified values
+    handle_ptr->getArgument("data_count", count);
+    handle_ptr->getArgument("inpacket", pktptr);
+@endcode
+
+As can be seen "getArgument" is used to retrieve data from the
+CalloutHandle, and "setArgument" used to put data into it.  If a callout
+wishes to alter data and pass it back to the component, it should retrieve
+the data with getArgument, modify it, and call setArgument to send
+it back.
+
+There are a couple points to be aware of:
+
+- The data type of the variable in the call to getArgument must
+match the data type of the variable passed to the corresponding
+setArgument <B>exactly</B>: using what would normally be considered
+to be a "compatible" type is not enough.  For example, if the callout
+passed an argument back to the component as an "int" and the component
+attempted to retrieve it as a "long", an exception would be thrown even
+though any value that can be stored in an "int" will fit into a "long".
+This restriction also applies the "const" attribute but only as applied to
+data pointed to by pointers, e.g. if an argument is defined as a "char*",
+an exception will be thrown if an attempt is made to retrieve it into
+a variable of type "const char*".  (However, if an argument is set as a
+"const int", it can be retrieved into an "int".)  The documentation of
+a hook point should detail the exact data type of each argument.
+
+- If a pointer to an object is passed to a callout (either a "raw"
+pointer, or a boost smart pointer (as in the example above), and the
+underlying object is altered through that pointer, the change will be
+reflected in the component even if the callout makes no call to setArgument.
+This can be avoided by passing a pointer to a "const" object.
+
+@subsection hooksComponentSkipFlag The Skip Flag
+
+Although information is passed back to the component from callouts through
+CalloutHandle arguments, a common action for callouts is to inform the component
+that its flow of control should be altered.  For example:
+
+- In the DHCP servers, there is a hook at the point at which a lease is
+  about to be assigned.  Callouts attached to this hooks may handle the
+  lease assignment in special cases, in which case they set the skip flag
+  to indicate that the server should not perform lease assignment in this
+  case.
+- A server may define a hook just after a packet is received.  A callout
+  attached to the hook might inspect the source address and compare it
+  against a blacklist.  If the address is on the list, the callout could set
+  the skip flag to indicate to the server that the packet should be dropped.
+
+For ease of processing, the CalloutHandle contains
+two methods, isc::hooks::CalloutHandle::getSkip() and
+isc::hooks::CalloutHandle::setSkip().  It is only meaningful for the
+component to use the "get" method.  The skip flag is cleared by the hooks
+framework when the component requests that callouts be executed, so any
+value set by the component is lost.  Callouts can both inspect the flag (it
+might have been set by callouts earlier in the callout list for the hook)
+and set it.  Note that the setting of the flag by a callout does not
+prevent callouts later in the list from being called: the skip flag is
+just a boolean flag - the only significance comes from its interpretation
+by the component.
+
+An example of use could be:
+@code
+// Set up arguments for DHCP lease assignment.
+handle->setArgument("query", query);
+handle->setArgument("response", response);
+HooksManager::callCallouts(lease_hook_index, *handle_ptr);
+if (! handle_ptr->getSkip()) {
+    // Skip flag not set, do the address allocation
+    :
+}
+@endcode
+
+
+@subsection hooksComponentGettingHandle Getting the Callout Handle
+
+The CalloutHandle object is linked to the loaded libraries
+for lifetime reasons (described below).  Components
+should retrieve a isc::hooks::CalloutHandle using
+isc::hooks::HooksManager::createCalloutHandle():
+@code
+    CalloutHandlePtr handle_ptr = HooksManager::createCalloutHandle();
+@endcode
+(isc::hooks::CalloutHandlePtr is a typedef for a Boost shared pointer to a
+CalloutHandle.)  The CalloutHandle so retrieved may be used for as
+long as the libraries are loaded.
+
+The handle is deleted by resetting the pointer:
+@code
+    handle_ptr.reset();
+@endcode
+... or by letting the handle pointer go out of scope.  The actual deletion
+occurs when the CallHandle's reference count goes to zero. (The
+current version of the hooks framework does not maintain any other
+pointer to the returned CalloutHandle, so it gets destroyed when the
+shared pointer to it is cleared or destroyed.  However, this may change
+in a future version.)
+
+@subsection hooksComponentCallingCallout Calling the Callout
+
+Calling the callout is a simple matter of executing the
+isc::hooks::HooksManager::callCallouts() method for the hook index in
+question.  For example, with the hook index pkt_sent defined as above,
+the hook can be executed by:
+@code
+    HooksManager::callCallouts(pkt_sent, *handle_ptr);
+@endcode
+... where "*handle_ptr" is a reference (note: not a pointer) to the
+isc::hooks::CalloutHandle object holding the arguments.  No status code
+is returned.  If a component needs to get data returned (other than that
+provided by the "skip" flag), it should define an argument through which
+the callout can do so.
+
+@subsubsection hooksComponentConditionalCallout Conditionally Calling Hook Callouts
+
+Most hooks in a component will not have callouts attached to them. To
+avoid the overhead of setting up arguments in the CalloutHandle, a
+component can check for callouts before doing that processing using
+isc::hooks::HooksManager::calloutsPresent().  Taking the index of a
+hook as its sole argument, the function returns true if there are any
+callouts attached to the hook and false otherwise.
+
+With this check, the code in the component for calling a hook would look
+something like:
+@code
+if (HooksManager::calloutsPresent(lease_hook_index)) {
+    // Set up arguments for lease assignment
+    handle->setArgument("query", query);
+    handle->setArgument("response", response);
+    HooksManager::callCallouts(lease_hook_index, *handle);
+    if (! handle->getSkip()) {
+        // Skip flag not set, do the address allocation
+        :
+    }
+}
+@endcode
+
+@section hooksComponentLoadLibraries Loading the User Libraries
+
+Once hooks are defined, all the hooks code described above will
+work, even if no libraries are loaded (and even if the library
+loading method is not called).  The CalloutHandle returned by
+isc::hooks::HooksManager::createCalloutHandle() will be valid,
+isc::hooks::HooksManager::calloutsPresent() will return false for every
+index, and isc::hooks::HooksManager::callCallouts() will be a no-op.
+
+However, if user libraries are specified in the BIND 10 configuration,
+the component should load them.  (Note the term "libraries": the hooks
+framework allows multiple user libraries to be loaded.) This should take
+place after the component's configuration has been read, and is achieved
+by the isc::hooks::HooksManager::loadLibraries() method.  The method is
+passed a vector of strings, each giving the full file specification of
+a user library:
+@code
+    std::vector<std::string> libraries = ... // Get array of libraries
+    bool success = HooksManager::loadLibraries(libraries);
+@endcode
+loadLibraries() returns a boolean status which is true if all libraries
+loaded successfully or false if one or more failed to load.  Appropriate
+error messages will have been logged in the latter case, the status
+being more to allow the developer to decide whether the execution
+should proceed in such circumstances.
+
+If loadLibraries() is called a second or subsequent time (as a result
+of a reconfiguration), all existing libraries are unloaded and a new
+set loaded.  Libraries can be explicitly unloaded either by calling
+isc::hooks::HooksManager::unloadLibraries() or by calling
+loadLibraries() with an empty vector as an argument.
+
+@subsection hooksComponentUnloadIssues Unload and Reload Issues
+
+Unloading a shared library works by unmapping the part of the process's
+virtual address space in which the library lies.  This may lead to
+problems if there are still references to that address space elsewhere
+in the process.
+
+In many operating systems, heap storage allowed by a shared library will
+lie in the virtual address allocated to the library.  This has implications
+in the hooks framework because:
+
+- Argument information stored in a CalloutHandle by a callout in a library
+may lie in the library's address space.
+- Data modified in objects passed as arguments may lie in the address
+space.  For example, it is common for a DHCP callout to add "options"
+to a packet: the memory allocated for those options will most likely
+lie in library address space.
+
+The problem really arises because of the extensive use by BIND 10 of boost
+smart pointers.  When the pointer is destroyed, the pointed-to memory is
+deallocated.  If the pointer points to address space that is unmapped because
+a library has been unloaded, the deletion causes a segmentation fault.
+
+The hooks framework addresses the issue for CalloutHandles by keeping in
+that object a shared pointer to the object controlling library unloading.
+Although a library can be unloaded at any time, it is only when all
+CalloutHandles that could possibly reference address space in the library
+have been deleted that the library will actually be unloaded and the
+address space unmapped.
+
+The hooks framework cannot solve the second issue as the objects in
+question are under control of the component.  It is up to the component
+developer to ensure that all such objects have been destroyed before
+libraries are reloaded.  In extreme cases this may mean the component
+suspending all processing of incoming requests until all currently
+executing requests have completed and data object destroyed, reloading
+the libraries, then resuming processing.
+
+@section hooksComponentCallouts Component-Defined Callouts
+
+Previous sections have discussed callout registration by user libraries.
+It is possible for a component to register its own functions (i.e. within
+its own address space) as hook callouts.  These functions are called
+in eactly the same way as user callouts, being passed their arguments
+though a CalloutHandle object.  (Guidelines for writing callouts can be
+found in @ref hooksDevelopersGuide.)
+
+A component can associate with a hook callouts that run either before
+user-registered callouts or after them.  Registration is done via a
+isc::hooks::LibraryHandle object, a reference to one being obtained
+through the methods isc::hooks::HooksManager::preCalloutLibraryHandle()
+(for a handle to register callouts to run before the user library
+callouts) or isc::hooks::HooksManager::postCalloutLibraryHandle() (for
+a handle to register callouts to run after the user callouts).  Use of
+the LibraryHandle to register and deregister callouts is described in
+@ref hooksLibraryHandle.
+
+Finally, it should be noted that callouts registered in this way only
+remain registered until the next call to isc::hooks::loadLibraries().
+It is up to the component to re-register the callouts after this
+method has been called.
+
+*/

src/lib/hooks/hooks_manager.cc

@@ -0,0 +1,173 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <hooks/callout_handle.h>
+#include <hooks/callout_manager.h>
+#include <hooks/callout_manager.h>
+#include <hooks/library_handle.h>
+#include <hooks/library_manager_collection.h>
+#include <hooks/hooks_manager.h>
+#include <hooks/server_hooks.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <string>
+#include <vector>
+
+using namespace std;
+
+namespace isc {
+namespace hooks {
+
+// Constructor
+
+HooksManager::HooksManager() {
+}
+
+// Return reference to singleton hooks manager.
+
+HooksManager&
+HooksManager::getHooksManager() {
+    static HooksManager manager;
+    return (manager);
+}
+
+// Are callouts present?
+
+bool
+HooksManager::calloutsPresentInternal(int index) {
+    conditionallyInitialize();
+    return (callout_manager_->calloutsPresent(index));
+}
+
+bool
+HooksManager::calloutsPresent(int index) {
+    return (getHooksManager().calloutsPresentInternal(index));
+}
+
+// Call the callouts
+
+void
+HooksManager::callCalloutsInternal(int index, CalloutHandle& handle) {
+    conditionallyInitialize();
+    return (callout_manager_->callCallouts(index, handle));
+}
+
+void
+HooksManager::callCallouts(int index, CalloutHandle& handle) {
+    return (getHooksManager().callCalloutsInternal(index, handle));
+}
+
+// Load the libraries.  This will delete the previously-loaded libraries
+// (if present) and load new ones.
+
+bool
+HooksManager::loadLibrariesInternal(const std::vector<std::string>& libraries) {
+    // Unload current set of libraries (if any are loaded).
+    unloadLibrariesInternal();
+
+    // Create the library manager and load the libraries.
+    lm_collection_.reset(new LibraryManagerCollection(libraries));
+    bool status = lm_collection_->loadLibraries();
+
+    // ... and obtain the callout manager for them.
+    callout_manager_ = lm_collection_->getCalloutManager();
+
+    return (status);
+}
+
+bool
+HooksManager::loadLibraries(const std::vector<std::string>& libraries) {
+    return (getHooksManager().loadLibrariesInternal(libraries));
+}
+
+// Unload the libraries.  This just deletes all internal objects which will
+// cause the libraries to be unloaded.
+
+void
+HooksManager::unloadLibrariesInternal() {
+    // The order of deletion does not matter here, as each library manager
+    // holds its own pointer to the callout manager.  However, we may as
+    // well delete the library managers first: if there are no other references
+    // to the callout manager, the second statement will delete it, which may
+    // ease debugging.
+    lm_collection_.reset();
+    callout_manager_.reset();
+}
+
+void HooksManager::unloadLibraries() {
+    getHooksManager().unloadLibrariesInternal();
+}
+
+// Create a callout handle
+
+boost::shared_ptr<CalloutHandle>
+HooksManager::createCalloutHandleInternal() {
+    conditionallyInitialize();
+    return (boost::shared_ptr<CalloutHandle>(
+            new CalloutHandle(callout_manager_, lm_collection_)));
+}
+
+boost::shared_ptr<CalloutHandle>
+HooksManager::createCalloutHandle() {
+    return (getHooksManager().createCalloutHandleInternal());
+}
+
+// Perform conditional initialization if nothing is loaded.
+
+void
+HooksManager::performConditionalInitialization() {
+
+    // Nothing present, so create the collection with any empty set of
+    // libraries, and get the CalloutManager.
+    vector<string> libraries;
+    lm_collection_.reset(new LibraryManagerCollection(libraries));
+    lm_collection_->loadLibraries();
+
+    callout_manager_ = lm_collection_->getCalloutManager();
+}
+
+// Shell around ServerHooks::registerHook()
+
+int
+HooksManager::registerHook(const std::string& name) {
+    return (ServerHooks::getServerHooks().registerHook(name));
+}
+
+// Return pre- and post- library handles.
+
+isc::hooks::LibraryHandle&
+HooksManager::preCalloutsLibraryHandleInternal() {
+    conditionallyInitialize();
+    return (callout_manager_->getPreLibraryHandle());
+}
+
+isc::hooks::LibraryHandle&
+HooksManager::preCalloutsLibraryHandle() {
+    return (getHooksManager().preCalloutsLibraryHandleInternal());
+}
+
+isc::hooks::LibraryHandle&
+HooksManager::postCalloutsLibraryHandleInternal() {
+    conditionallyInitialize();
+    return (callout_manager_->getPostLibraryHandle());
+}
+
+isc::hooks::LibraryHandle&
+HooksManager::postCalloutsLibraryHandle() {
+    return (getHooksManager().postCalloutsLibraryHandleInternal());
+}
+
+} // namespace util
+} // namespace isc

src/lib/hooks/hooks_manager.h

@@ -0,0 +1,276 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef HOOKS_MANAGER_H
+#define HOOKS_MANAGER_H
+
+#include <hooks/server_hooks.h>
+
+#include <boost/noncopyable.hpp>
+#include <boost/shared_ptr.hpp>
+
+#include <string>
+#include <vector>
+
+namespace isc {
+namespace hooks {
+
+// Forward declarations
+class CalloutHandle;
+class CalloutManager;
+class LibraryHandle;
+class LibraryManagerCollection;
+
+/// @brief Hooks Manager
+///
+/// This is the overall manager of the hooks framework and is the main class
+/// used by a BIND 10 module when handling hooks.  It is responsible for the
+/// loading and unloading of user libraries, and for calling the callouts on
+/// each hook point.
+///
+/// The class is a singleton, the single instance of the object being accessed
+/// through the static getHooksManager() method.
+
+class HooksManager : boost::noncopyable {
+public:
+    /// @brief Get singleton hooks manager
+    ///
+    /// @return Reference to the singleton hooks manager.
+    static HooksManager& getHooksManager();
+
+    /// @brief Load and reload libraries
+    ///
+    /// Loads the list of libraries into the server address space.  For each
+    /// library, the "standard" functions (ones with the same names as the
+    /// hook points) are configured and the libraries' "load" function
+    /// called.
+    ///
+    /// If libraries are already loaded, they are unloaded and the new
+    /// libraries loaded.
+    ///
+    /// If any library fails to load, an error message will be logged.  The
+    /// remaining libraries will be loaded if possible.
+    ///
+    /// @param libraries List of libraries to be loaded.  The order is
+    ///        important, as it determines the order that callouts on the same
+    ///        hook will be called.
+    ///
+    /// @return true if all libraries loaded without a problem, false if one or
+    ///        more libraries failed to load.  In the latter case, message will
+    ///        be logged that give the reason.
+    static bool loadLibraries(const std::vector<std::string>& libraries);
+
+    /// @brief Unload libraries
+    ///
+    /// Unloads the loaded libraries and leaves the hooks subsystem in the
+    /// state it was after construction but before loadLibraries() is called.
+    ///
+    /// @note: This method should be used with caution - see the notes for
+    ///        the class LibraryManager for pitfalls.  In general, a server
+    ///        should not call this method: library unloading will automatically
+    ///        take place when new libraries are loaded, and when appropriate
+    ///        objects are destroyed.
+    ///
+    /// @return true if all libraries unloaded successfully, false on an error.
+    ///         In the latter case, an error message will have been output.
+    static void unloadLibraries();
+
+    /// @brief Are callouts present?
+    ///
+    /// Checks loaded libraries and returns true if at lease one callout
+    /// has been registered by them for the given hook.
+    ///
+    /// @param index Hooks index for which callouts are checked.
+    ///
+    /// @return true if callouts are present, false if not.
+    /// @throw NoSuchHook Given index does not correspond to a valid hook.
+    static bool calloutsPresent(int index);
+
+    /// @brief Calls the callouts for a given hook
+    ///
+    /// Iterates through the libray handles and calls the callouts associated
+    /// with the given hook index.
+    ///
+    /// @note This method invalidates the current library index set with
+    ///       setLibraryIndex().
+    ///
+    /// @param index Index of the hook to call.
+    /// @param handle Reference to the CalloutHandle object for the current
+    ///        object being processed.
+    static void callCallouts(int index, CalloutHandle& handle);
+
+    /// @brief Return pre-callouts library handle
+    ///
+    /// Returns a library handle that can be used by the server to register
+    /// callouts on a hook that are called _before_ any callouts belonging
+    /// to a library.
+    ///
+    /// @note Both the reference returned and the callouts registered with
+    ///       this handle only remain valid until the next loadLibraries() or
+    ///       unloadLibraries() call.  If the callouts are to remain registered
+    ///       after this time, a new handle will need to be obtained and the
+    ///       callouts re-registered.
+    ///
+    /// @return Reference to library handle associated with pre-library callout
+    ///         registration.
+    static LibraryHandle& preCalloutsLibraryHandle();
+
+    /// @brief Return post-callouts library handle
+    ///
+    /// Returns a library handle that can be used by the server to register
+    /// callouts on a hook that are called _after any callouts belonging
+    /// to a library.
+    ///
+    /// @note Both the reference returned and the callouts registered with
+    ///       this handle only remain valid until the next loadLibraries() or
+    ///       unloadLibraries() call.  If the callouts are to remain registered
+    ///       after this time, a new handle will need to be obtained and the
+    ///       callouts re-registered.
+    ///
+    /// @return Reference to library handle associated with post-library callout
+    ///         registration.
+    static LibraryHandle& postCalloutsLibraryHandle();
+
+    /// @brief Return callout handle
+    ///
+    /// Returns a callout handle to be associated with a request passed round
+    /// the system.
+    ///
+    /// @note This handle is valid only after a loadLibraries() call and then
+    ///       only up to the next loadLibraries() call.
+    ///
+    /// @return Shared pointer to a CalloutHandle object.
+    static boost::shared_ptr<CalloutHandle> createCalloutHandle();
+
+    /// @brief Register Hook
+    ///
+    /// This is just a convenience shell around the ServerHooks::registerHook()
+    /// method.  It - along with the definitions of the two hook indexes for
+    /// the context_create and context_destroy methods - means that server
+    /// authors only need to deal with HooksManager and CalloutHandle, and not
+    /// include any other hooks framework classes.
+    ///
+    /// @param name Name of the hook
+    ///
+    /// @return Index of the hook, to be used in subsequent hook-related calls.
+    ///         This will be greater than or equal to zero (so allowing a
+    ///         negative value to indicate an invalid index).
+    ///
+    /// @throws DuplicateHook A hook with the same name has already been
+    ///         registered.
+    static int registerHook(const std::string& name);
+
+    /// Index numbers for pre-defined hooks.
+    static const int CONTEXT_CREATE = ServerHooks::CONTEXT_CREATE;
+    static const int CONTEXT_DESTROY = ServerHooks::CONTEXT_DESTROY;
+
+private:
+
+    /// @brief Constructor
+    ///
+    /// This is private as the object is a singleton and can only be addessed
+    /// through the getHooksManager() static method.
+    HooksManager();
+
+    //@{
+    /// The following methods correspond to similarly-named static methods,
+    /// but actually do the work on the singleton instance of the HooksManager.
+    /// See the descriptions of the static methods for more details.
+
+    /// @brief Load and reload libraries
+    ///
+    /// @param libraries List of libraries to be loaded.  The order is
+    ///        important, as it determines the order that callouts on the same
+    ///        hook will be called.
+    ///
+    /// @return true if all libraries loaded without a problem, false if one or
+    ///        more libraries failed to load.  In the latter case, message will
+    ///        be logged that give the reason.
+    bool loadLibrariesInternal(const std::vector<std::string>& libraries);
+
+    /// @brief Unload libraries
+    void unloadLibrariesInternal();
+
+    /// @brief Are callouts present?
+    ///
+    /// @param index Hooks index for which callouts are checked.
+    ///
+    /// @return true if callouts are present, false if not.
+    /// @throw NoSuchHook Given index does not correspond to a valid hook.
+    bool calloutsPresentInternal(int index);
+
+    /// @brief Calls the callouts for a given hook
+    ///
+    /// @param index Index of the hook to call.
+    /// @param handle Reference to the CalloutHandle object for the current
+    ///        object being processed.
+    void callCalloutsInternal(int index, CalloutHandle& handle);
+
+    /// @brief Return callout handle
+    ///
+    /// @return Shared pointer to a CalloutHandle object.
+    boost::shared_ptr<CalloutHandle> createCalloutHandleInternal();
+
+    /// @brief Return pre-callouts library handle
+    ///
+    /// @return Reference to library handle associated with pre-library callout
+    ///         registration.
+    LibraryHandle& preCalloutsLibraryHandleInternal();
+
+    /// @brief Return post-callouts library handle
+    ///
+    /// @return Reference to library handle associated with post-library callout
+    ///         registration.
+    LibraryHandle& postCalloutsLibraryHandleInternal();
+
+    //@}
+
+    /// @brief Initialization to No Libraries
+    ///
+    /// Initializes the hooks manager with an "empty set" of libraries.  This
+    /// method is called if conditionallyInitialize() determines that such
+    /// initialization is needed.
+    void performConditionalInitialization();
+
+    /// @brief Conditional initialization of the  hooks manager
+    ///
+    /// loadLibraries() performs the initialization of the HooksManager,
+    /// setting up the internal structures and loading libraries.  However,
+    /// in some cases, server authors may not do that.  This method is called
+    /// whenever any hooks execution function is invoked (checking callouts,
+    /// calling callouts or returning a callout handle).  If the HooksManager
+    /// is unitialised, it will initialize it with an "empty set" of libraries.
+    ///
+    /// For speed, the test of whether initialization is required is done
+    /// in-line here.  The actual initialization is performed in
+    /// performConditionalInitialization().
+    void conditionallyInitialize() {
+        if (!lm_collection_) {
+            performConditionalInitialization();
+        }
+    }
+
+    // Members
+
+    /// Set of library managers.
+    boost::shared_ptr<LibraryManagerCollection> lm_collection_;
+
+    /// Callout manager for the set of library managers.
+    boost::shared_ptr<CalloutManager> callout_manager_;
+};
+
+} // namespace util
+} // namespace hooks
+
+#endif // HOOKS_MANAGER_H

src/lib/hooks/hooks_messages.mes

@@ -14,22 +14,24 @@
 
 $NAMESPACE isc::hooks
 
-% HOOKS_CALLOUT hooks library with index %1 has called a callout on hook %2 that has address %3
+% HOOKS_CALLOUT_CALLED hooks library with index %1 has called a callout on hook %2 that has address %3
 Only output at a high debugging level, this message indicates that
 a callout on the named hook registered by the library with the given
 index (in the list of loaded libraries) has been called and returned a
 success state.  The address of the callout is given in the message
 
 % HOOKS_CALLOUT_ERROR error returned by callout on hook %1 registered by library with index %2 (callout address %3)
-If a callout returns an error status when called, this warning message
+If a callout returns an error status when called, this error message
-is issues.  It identifies the hook to which the callout is attached,
+is issued.  It identifies the hook to which the callout is attached, the
-and the index of the library (in the list of loaded libraries) that
+index of the library (in the list of loaded libraries) that registered
-registered it.  The address of the callout is also given.
+it and the address of the callout.  The error is otherwise ignored.
 
-% HOOKS_CALLOUT_REMOVED callout removed from hook %1 for library %2
+% HOOKS_CALLOUTS_REMOVED callouts removed from hook %1 for library %2
 This is a debug message issued during library unloading.  It notes that
 one of more callouts registered by that library have been removed from
-the specified hook.
+the specified hook.  This is similar to the HOOKS_DEREGISTER_ALL_CALLOUTS
+message (and the two are likely to be seen together), but is issued at a
+higher-level in the hook framework.
 
 % HOOKS_CLOSE_ERROR failed to close hook library %1: %2
 BIND 10 has failed to close the named hook library for the stated reason.
@@ -37,14 +39,22 @@ Although this is an error, this should not affect the running system
 other than as a loss of resources.  If this error persists, you should
 restart BIND 10.
 
-% HOOKS_DEREGISTER_ALL_CALLOUTS hook library at index %1 deregistered all callouts on hook %2
+% HOOKS_CALLOUT_EXCEPTION exception thrown by callout on hook %1 registered by library with index %2 (callout address %3)
-A debug message issued when all callouts on the specified hook registered
+If a callout throws an exception when called, this error message is
-by the library with the given index were removed.
+issued.  It identifies the hook to which the callout is attached, the
+index of the library (in the list of loaded libraries) that registered
+it and the address of the callout.  The error is otherwise ignored.
 
-% HOOKS_DEREGISTER_CALLOUT hook library at index %1 deregistered a callout on hook %2
+% HOOKS_ALL_CALLOUTS_DEREGISTERED hook library at index %1 removed all callouts on hook %2
+A debug message issued when all callouts on the specified hook registered
+by the library with the given index were removed.  This is similar to
+the HOOKS_CALLOUTS_REMOVED message (and the two are likely to be seen
+together), but is issued at a lower-level in the hook framework.
+
+% HOOKS_CALLOUT_DEREGISTERED hook library at index %1 deregistered a callout on hook %2
 A debug message issued when all instances of a particular callouts on
 the hook identified in the message that were registered by the library
-with the given index were removed.
+with the given index have been removed.
 
 % HOOKS_INCORRECT_VERSION hook library %1 is at version %2, require version %3
 BIND 10 has detected that the named hook library has been built against
@@ -67,7 +77,7 @@ has been successfully unloaded.
 A debug  message issued when the version check on the hooks library
 has succeeded.
 
-% HOOKS_LOAD 'load' function in hook library %1 returned success
+% HOOKS_LOAD_SUCCESS 'load' function in hook library %1 returned success
 This is a debug message issued when the "load" function has been found
 in a hook library and has been successfully called.
 
@@ -77,8 +87,16 @@ was called.  The function returned a non-zero status (also given in
 the message) which was interpreted as an error.  The library has been
 unloaded and no callouts from it will be installed.
 
-% HOOKS_LOAD_LIBRARY loading hooks library %1
+% HOOKS_LOAD_EXCEPTION 'load' function in hook library %1 threw an exception
-This is a debug message called when the specified library is being loaded.
+A "load" function was found in the library named in the message and
+was called.  The function threw an exception (an error indication)
+during execution, which is an error condition.  The library has been
+unloaded and no callouts from it will be installed.
+
+% HOOKS_LIBRARY_LOADING loading hooks library %1
+This is a debug message output just before the specified library is loaded.
+If the action is successfully, it will be followed by the
+HOOKS_LIBRARY_LOADED informational message.
 
 % HOOKS_NO_LOAD no 'load' function found in hook library %1
 This is a debug message saying that the specified library was loaded
@@ -102,27 +120,27 @@ BIND 10 failed to open the specified hook library for the stated
 reason. The library has not been loaded.  BIND 10 will continue to
 function, but without the services offered by the library.
 
-% HOOKS_REGISTER_CALLOUT hooks library with index %1 registered callout for hook '%2'
+% HOOKS_CALLOUT_REGISTRATION hooks library with index %1 registering callout for hook '%2'
 This is a debug message, output when a library (whose index in the list
 of libraries (being) loaded is given) registers a callout.
 
-% HOOKS_REGISTER_HOOK hook %1 was registered
+% HOOKS_HOOK_REGISTERED hook %1 was registered
 This is a debug message indicating that a hook of the specified name
 was registered by a BIND 10 server.  The server doing the logging is
 indicated by the full name of the logger used to log this message.
 
-% HOOKS_REGISTER_STD_CALLOUT hooks library %1 registered standard callout for hook %2
+% HOOKS_STD_CALLOUT_REGISTERED hooks library %1 registered standard callout for hook %2 at address %3
 This is a debug message, output when the library loading function has
 located a standard callout (a callout with the same name as a hook point)
-and registered it.
+and registered it.  The address of the callout is indicated.
 
-% HOOKS_RESET_HOOK_LIST the list of hooks has been reset
+% HOOKS_HOOK_LIST_RESET the list of hooks has been reset
 This is a message indicating that the list of hooks has been reset.
 While this is usual when running the BIND 10 test suite, it should not be
 seen when running BIND 10 in a producion environment.  If this appears,
 please report a bug through the usual channels.
 
-% HOOKS_UNLOAD 'unload' function in hook library %1 returned success
+% HOOKS_UNLOAD_SUCCESS 'unload' function in hook library %1 returned success
 This is a debug message issued when an "unload" function has been found
 in a hook library during the unload process, called, and returned success.
 
@@ -132,6 +150,18 @@ It was called, but returned an error (non-zero) status, resulting in
 the issuing of this message.  The unload process continued after this
 message and the library has been unloaded.
 
-% HOOKS_UNLOAD_LIBRARY unloading library %1
+% HOOKS_UNLOAD_EXCEPTION 'unload' function in hook library %1 threw an exception
-This is a debug message called when the specified library is being
+During the unloading of a library, an "unload" function was found.  It was
-unloaded.
+called, but in the process generated an exception (an error indication).
+The unload process continued after this message and the library has
+been unloaded.
+
+% 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
+HOOKS_LIBRARY_UNLOADED informational message.
+
+% HOOKS_VERSION_EXCEPTION 'version' function in hook library %1 threw an exception
+This error message is issued if the version() function in the specified
+hooks library was called and generated an exception.  The library is
+considered unusable and will not be loaded.

src/lib/hooks/library_handle.cc

@@ -22,17 +22,54 @@ namespace hooks {
 
 void
 LibraryHandle::registerCallout(const std::string& name, CalloutPtr callout) {
+    // Reset library index if required, saving the current value.
+    int saved_index = callout_manager_->getLibraryIndex();
+    if (index_ >= 0) {
+        callout_manager_->setLibraryIndex(index_);
+    }
+
+    // Register the callout.
     callout_manager_->registerCallout(name, callout);
+
+    // Restore the library index if required.  We know that the saved index
+    // is valid for the number of libraries (or is -1, which is an internal
+    // state indicating there is no current library index) as we obtained it
+    // from the callout manager.
+    if (index_ >= 0) {
+        callout_manager_->setLibraryIndex(saved_index);
+    }
 }
 
 bool
 LibraryHandle::deregisterCallout(const std::string& name, CalloutPtr callout) {
-    return (callout_manager_->deregisterCallout(name, callout));
+    int saved_index = callout_manager_->getLibraryIndex();
+    if (index_ >= 0) {
+        callout_manager_->setLibraryIndex(index_);
+    }
+
+    bool status = callout_manager_->deregisterCallout(name, callout);
+
+    if (index_ >= 0) {
+        callout_manager_->setLibraryIndex(saved_index);
+    }
+
+    return (status);
 }
 
 bool
 LibraryHandle::deregisterAllCallouts(const std::string& name) {
-    return (callout_manager_->deregisterAllCallouts(name));
+    int saved_index = callout_manager_->getLibraryIndex();
+    if (index_ >= 0) {
+        callout_manager_->setLibraryIndex(index_);
+    }
+
+    bool status = callout_manager_->deregisterAllCallouts(name);
+
+    if (index_ >= 0) {
+        callout_manager_->setLibraryIndex(saved_index);
+    }
+
+    return (status);
 }
 
 } // namespace util

src/lib/hooks/library_handle.h

@@ -58,7 +58,18 @@ public:
     ///        object.  Note that the "raw" pointer is safe - the only
     ///        instance of the LibraryHandle in the system is as a member of
     ///        the CalloutManager to which it points.
-    LibraryHandle(CalloutManager* manager) : callout_manager_(manager)
+    ///
+    /// @param index Index of the library to which the LibraryHandle applies.
+    ///        If negative, the library index as set in the CalloutManager is
+    ///        used.  Otherwise the current library index is saved, this value
+    ///        is set as the current index, the registration call is made, and
+    ///        the CalloutManager's library index is reset.  Note: although -1
+    ///        is a valid argument value for
+    ///        isc::hooks::CalloutManager::setLibraryIndex(), in this class is
+    ///        is used as a sentinel to indicate that the library index in
+    ///        isc::util::CalloutManager should not be set or reset.
+    LibraryHandle(CalloutManager* manager, int index = -1)
+        : callout_manager_(manager), index_(index)
     {}
 
     /// @brief Register a callout on a hook
@@ -105,8 +116,31 @@ public:
     bool deregisterAllCallouts(const std::string& name);
 
 private:
+    /// @brief Copy constructor
+    ///
+    /// Private (with no implementation) as it makes no sense to copy an object
+    /// of this type.  All code receives a reference to an existing handle which
+    /// is tied to a particular CalloutManager.  Creating a copy of that handle
+    /// runs the risk of a "dangling pointer" to the original handle's callout
+    /// manager.
+    ///
+    /// @param Unused - should be the object to copy.
+    LibraryHandle(const LibraryHandle&);
+
+    /// @brief Assignment operator
+    ///
+    /// Declared private like the copy constructor for the same reasons. It too
+    /// has no implementation.
+    ///
+    /// @param Unused - should be the object to copy.
+    LibraryHandle& operator=(const LibraryHandle&);
+
     /// Back pointer to the collection object for the library
     CalloutManager* callout_manager_;
+
+    /// Library index to which this handle applies.  -1 indicates that it
+    /// applies to whatever index is current in the CalloutManager.
+    int index_;
 };
 
 } // namespace util

src/lib/hooks/library_manager.cc

@@ -24,20 +24,84 @@
 
 #include <dlfcn.h>
 
-namespace {
-
-// String constants
-
-const char* LOAD_FUNCTION_NAME = "load";
-const char* UNLOAD_FUNCTION_NAME = "unload";
-const char* VERSION_FUNCTION_NAME = "version";
-}
-
 using namespace std;
 
 namespace isc {
 namespace hooks {
 
+/// @brief Local class for conversion of void pointers to function pointers
+///
+/// Converting between void* and function pointers in C++ is fraught with
+/// difficulty and pitfalls, e.g. see
+/// https://groups.google.com/forum/?hl=en&fromgroups#!topic/comp.lang.c++/37o0l8rtEE0
+///
+/// The method given in that article - convert using a union is used here.  A
+/// union is declared (and zeroed) and the appropriate member extracted when
+/// needed.
+
+class PointerConverter {
+public:
+    /// @brief Constructor
+    ///
+    /// Zeroes the union and stores the void* pointer we wish to convert (the
+    /// one returned by dlsym).
+    ///
+    /// @param dlsym_ptr void* pointer returned by call to dlsym()
+    PointerConverter(void* dlsym_ptr) {
+        memset(&pointers_, 0, sizeof(pointers_));
+        pointers_.dlsym_ptr = dlsym_ptr;
+    }
+
+    /// @name Pointer accessor functions
+    ///
+    /// It is up to the caller to ensure that the correct member is called so
+    /// that the correct trype of pointer is returned.
+    ///
+    ///@{
+
+    /// @brief Return pointer to callout function
+    ///
+    /// @return Pointer to the callout function
+    CalloutPtr calloutPtr() const {
+        return (pointers_.callout_ptr);
+    }
+
+    /// @brief Return pointer to load function
+    ///
+    /// @return Pointer to the load function
+    load_function_ptr loadPtr() const {
+        return (pointers_.load_ptr);
+    }
+
+    /// @brief Return pointer to unload function
+    ///
+    /// @return Pointer to the unload function
+    unload_function_ptr unloadPtr() const {
+        return (pointers_.unload_ptr);
+    }
+
+    /// @brief Return pointer to version function
+    ///
+    /// @return Pointer to the version function
+    version_function_ptr versionPtr() const {
+        return (pointers_.version_ptr);
+    }
+
+    ///@}
+
+private:
+
+    /// @brief Union linking void* and pointers to functions.
+    union {
+        void*                   dlsym_ptr;      // void* returned by dlsym
+        CalloutPtr              callout_ptr;    // Pointer to callout
+        load_function_ptr       load_ptr;       // Pointer to load function
+        unload_function_ptr     unload_ptr;     // Pointer to unload function
+        version_function_ptr    version_ptr;    // Pointer to version function
+    } pointers_;
+};
+
+
 // Open the library
 
 bool
@@ -45,7 +109,7 @@ LibraryManager::openLibrary() {
 
     // Open the library.  We'll resolve names now, so that if there are any
     // issues we don't bugcheck in the middle of apparently unrelated code.
-    dl_handle_ = dlopen(library_name_.c_str(), RTLD_NOW | RTLD_DEEPBIND);
+    dl_handle_ = dlopen(library_name_.c_str(), RTLD_NOW | RTLD_LOCAL);
     if (dl_handle_ == NULL) {
         LOG_ERROR(hooks_logger, HOOKS_OPEN_ERROR).arg(library_name_)
                   .arg(dlerror());
@@ -78,28 +142,17 @@ LibraryManager::closeLibrary() {
 bool
 LibraryManager::checkVersion() const {
 
-    // Look up the "version" string in the library.  This is returned as
-    // "void*": without any other information, we must assume that it is of
-    // the correct type of version_function_ptr.
-    //
-    // Note that converting between void* and function pointers in C++ is
-    // fraught with difficulty and pitfalls (e.g. see
-    // https://groups.google.com/forum/?hl=en&fromgroups#!topic/
-    // comp.lang.c++/37o0l8rtEE0)
-    // The method given in that article - convert using a union is used here.
-    union {
-        version_function_ptr    ver_ptr;
-        void*                   dlsym_ptr;
-    } pointers;
-
-    // Zero the union, whatever the size of the pointers.
-    pointers.ver_ptr = NULL;
-    pointers.dlsym_ptr = NULL;
-
     // Get the pointer to the "version" function.
-    pointers.dlsym_ptr = dlsym(dl_handle_, VERSION_FUNCTION_NAME);
+    PointerConverter pc(dlsym(dl_handle_, VERSION_FUNCTION_NAME));
-    if (pointers.ver_ptr != NULL) {
+    if (pc.versionPtr() != NULL) {
-        int version = (*pointers.ver_ptr)();
+        int version = BIND10_HOOKS_VERSION - 1; // This is an invalid value
+        try {
+            version = (*pc.versionPtr())();
+        } catch (...) {
+            LOG_ERROR(hooks_logger, HOOKS_VERSION_EXCEPTION).arg(library_name_);
+            return (false);
+        }
+
         if (version == BIND10_HOOKS_VERSION) {
             // All OK, version checks out
             LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_LIBRARY_VERSION)
@@ -121,31 +174,23 @@ LibraryManager::checkVersion() const {
 
 void
 LibraryManager::registerStandardCallouts() {
-    // Create a library handle for doing the registration.  We also need to
+    // Set the library index for doing the registration.  This is picked up
-    // set the current library index to indicate the current library.
+    // when the library handle is created.
     manager_->setLibraryIndex(index_);
-    LibraryHandle library_handle(manager_.get());
 
-    // Iterate through the list of known hooksv
+    // Iterate through the list of known hooks
     vector<string> hook_names = ServerHooks::getServerHooks().getHookNames();
     for (int i = 0; i < hook_names.size(); ++i) {
 
-        // Convert void* to function pointers using the same tricks as
-        // described above.
-        union {
-            CalloutPtr  callout_ptr;
-            void*       dlsym_ptr;
-        } pointers;
-        pointers.callout_ptr = NULL;
-        pointers.dlsym_ptr = NULL;
-
         // Look up the symbol
-        pointers.dlsym_ptr = dlsym(dl_handle_, hook_names[i].c_str());
+        void* dlsym_ptr = dlsym(dl_handle_, hook_names[i].c_str());
-        if (pointers.callout_ptr != NULL) {
+        PointerConverter pc(dlsym_ptr);
+        if (pc.calloutPtr() != NULL) {
             // Found a symbol, so register it.
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_REGISTER_STD_CALLOUT)
+            manager_->getLibraryHandle().registerCallout(hook_names[i],
-                .arg(library_name_).arg(hook_names[i]);
+                                                         pc.calloutPtr());
-            library_handle.registerCallout(hook_names[i], pointers.callout_ptr);
+            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_STD_CALLOUT_REGISTERED)
+                .arg(library_name_).arg(hook_names[i]).arg(dlsym_ptr);
 
         }
     }
@@ -156,35 +201,32 @@ LibraryManager::registerStandardCallouts() {
 bool
 LibraryManager::runLoad() {
 
-    // Look up the "load" function in the library.  The code here is similar
-    // to that in "checkVersion".
-    union {
-        load_function_ptr   load_ptr;
-        void*               dlsym_ptr;
-    } pointers;
-
-    // Zero the union, whatever the size of the pointers.
-    pointers.load_ptr = NULL;
-    pointers.dlsym_ptr = NULL;
-
     // Get the pointer to the "load" function.
-    pointers.dlsym_ptr = dlsym(dl_handle_, LOAD_FUNCTION_NAME);
+    PointerConverter pc(dlsym(dl_handle_, LOAD_FUNCTION_NAME));
-    if (pointers.load_ptr != NULL) {
+    if (pc.loadPtr() != NULL) {
 
         // Call the load() function with the library handle.  We need to set
         // the CalloutManager's index appropriately.  We'll invalidate it
         // afterwards.
-        manager_->setLibraryIndex(index_);
+
-        int status = (*pointers.load_ptr)(manager_->getLibraryHandle());
+        int status = -1;
-        manager_->setLibraryIndex(index_);
+        try {
+            manager_->setLibraryIndex(index_);
+            status = (*pc.loadPtr())(manager_->getLibraryHandle());
+        } catch (...) {
+            LOG_ERROR(hooks_logger, HOOKS_LOAD_EXCEPTION).arg(library_name_);
+            return (false);
+        }
+
         if (status != 0) {
             LOG_ERROR(hooks_logger, HOOKS_LOAD_ERROR).arg(library_name_)
                       .arg(status);
             return (false);
         } else {
-        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LOAD)
+        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LOAD_SUCCESS)
             .arg(library_name_);
         }
+
     } else {
         LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_NO_LOAD)
             .arg(library_name_);
@@ -199,31 +241,29 @@ LibraryManager::runLoad() {
 bool
 LibraryManager::runUnload() {
 
-    // Look up the "load" function in the library.  The code here is similar
-    // to that in "checkVersion".
-    union {
-        unload_function_ptr unload_ptr;
-        void*               dlsym_ptr;
-    } pointers;
-
-    // Zero the union, whatever the relative size of the pointers.
-    pointers.unload_ptr = NULL;
-    pointers.dlsym_ptr = NULL;
-
     // Get the pointer to the "load" function.
-    pointers.dlsym_ptr = dlsym(dl_handle_, UNLOAD_FUNCTION_NAME);
+    PointerConverter pc(dlsym(dl_handle_, UNLOAD_FUNCTION_NAME));
-    if (pointers.unload_ptr != NULL) {
+    if (pc.unloadPtr() != NULL) {
 
         // Call the load() function with the library handle.  We need to set
         // the CalloutManager's index appropriately.  We'll invalidate it
         // afterwards.
-        int status = (*pointers.unload_ptr)();
+        int status = -1;
+        try {
+            status = (*pc.unloadPtr())();
+        } catch (...) {
+            // Exception generated.  Note a warning as the unload will occur
+            // anyway.
+            LOG_WARN(hooks_logger, HOOKS_UNLOAD_EXCEPTION).arg(library_name_);
+            return (false);
+        }
+
         if (status != 0) {
             LOG_ERROR(hooks_logger, HOOKS_UNLOAD_ERROR).arg(library_name_)
                       .arg(status);
             return (false);
         } else {
-        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_UNLOAD)
+        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_UNLOAD_SUCCESS)
             .arg(library_name_);
         }
     } else {
@@ -238,43 +278,44 @@ LibraryManager::runUnload() {
 
 bool
 LibraryManager::loadLibrary() {
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LOAD_LIBRARY)
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_LOADING)
         .arg(library_name_);
 
     // In the following, if a method such as openLibrary() fails, it will
     // have issued an error message so there is no need to issue another one
     // here.
 
+    // Open the library (which is a check that it exists and is accessible).
     if (openLibrary()) {
 
         // Library opened OK, see if a version function is present and if so,
-        // check it.
+        // check what value it returns.
         if (checkVersion()) {
 
             // Version OK, so now register the standard callouts and call the
-            // librarie's load() function if present.
+            // library's load() function if present.
             registerStandardCallouts();
             if (runLoad()) {
 
                 // Success - the library has been successfully loaded.
                 LOG_INFO(hooks_logger, HOOKS_LIBRARY_LOADED).arg(library_name_);
                 return (true);
+
             } else {
 
                 // The load function failed, so back out.  We can't just close
                 // the library as (a) we need to call the library's "unload"
                 // function (if present) in case "load" allocated resources that
-                // need to be freed and (b) - we need to remove any callouts
+                // need to be freed and (b) we need to remove any callouts that
-                // that have been installed.
+                // have been installed.
                 static_cast<void>(unloadLibrary());
             }
-        } else {
-
-            // Version check failed so close the library and free up resources.
-            // Ignore the status return here - we already have an error
-            // consition.
-            static_cast<void>(closeLibrary());
         }
+
+        // Either the version check or call to load() failed, so close the
+        // library and free up resources.  Ignore the status return here - we
+        // already know there's an error and will have output a message.
+        static_cast<void>(closeLibrary());
     }
 
     return (false);
@@ -285,7 +326,7 @@ LibraryManager::loadLibrary() {
 
 bool
 LibraryManager::unloadLibrary() {
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_UNLOAD_LIBRARY)
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_UNLOADING)
         .arg(library_name_);
 
     // Call the unload() function if present.  Note that this is done first -
@@ -300,13 +341,13 @@ LibraryManager::unloadLibrary() {
     for (int i = 0; i < hooks.size(); ++i) {
         bool removed = manager_->deregisterAllCallouts(hooks[i]);
         if (removed) {
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUT_REMOVED)
+            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUTS_REMOVED)
                 .arg(hooks[i]).arg(library_name_);
         }
     }
 
     // ... and close the library.
-    result = result && closeLibrary();
+    result = closeLibrary() && result;
     if (result) {
 
         // Issue the informational message only if the library was unloaded

src/lib/hooks/library_manager.h

@@ -23,6 +23,7 @@ namespace isc {
 namespace hooks {
 
 class CalloutManager;
+class LibraryHandle;
 class LibraryManager;
 
 /// @brief Library manager
@@ -31,27 +32,26 @@ class LibraryManager;
 ///
 /// On loading, it opens the library using dlopen and checks the version (set
 /// with the "version" method.  If all is OK, it iterates through the list of
-/// known hooks and locates their symbols, registering each callout as it
+/// known hooks and locates their symbols, registering each callout as it does
-/// does so.  Finally it locates the "load" and "unload" functions (if present),
+/// so.  Finally it locates the "load" function (if present) and calls it.
-/// calling the "load" callout if present.
 ///
-/// On unload, it calls the "unload" method if one was located, clears the
+/// On unload, it calls the "unload" method if present, clears the callouts on
-/// callouts from all hooks and closes the library.
+/// all hooks, and closes the library.
 ///
-/// @note Caution needs to be exercised whtn using the unload method. During
+/// @note Caution needs to be exercised when using the unload method. During
-///       use, data will pass between the server and the library.  In this
+///       normal use, data will pass between the server and the library.  In
-///       process, the library may allocate memory and pass it back to the
+///       this process, the library may allocate memory and pass it back to the
 ///       server.  This could happen by the server setting arguments or context
 ///       in the CalloutHandle object, or by the library modifying the content
 ///       of pointed-to data. If the library is unloaded, this memory may lie
 ///       in the virtual address space deleted in that process. (The word "may"
-///       is used, as this could be operating-system specific.) If this happens,
+///       is used, as this could be operating-system specific.) Should this
-///       any reference to the memory will cause a segmentation fault.  This can
+///       happen, any reference to the memory will cause a segmentation fault.
-///       occur in a quite obscure place, for example in the middle of a
+///       This can occur in a quite obscure place, for example in the middle of
-///       destructor of an STL class when it is deleting memory allocated
+///       a destructor of an STL class when it is deleting memory allocated
-///       when the data structure was extended.
+///       when the data structure was extended by a function in the library.
 ///
-/// @par  The only safe way to run the "unload" function is to ensure that all
+/// @note The only safe way to run the "unload" function is to ensure that all
 ///       possible references to it are removed first.  This means that all
 ///       CalloutHandles must be destroyed, as must any data items that were
 ///       passed to the callouts.  In practice, it could mean that a server
@@ -60,12 +60,6 @@ class LibraryManager;
 ///       reloading the libraries.
 
 class LibraryManager {
-private:
-    /// Useful typedefs for the framework functions
-    typedef int (*version_function_ptr)();            ///< version() signature
-    typedef int (*load_function_ptr)(LibraryHandle&); ///< load() signature
-    typedef int (*unload_function_ptr)();             ///< unload() signature
-
 public:
     /// @brief Constructor
     ///
@@ -84,22 +78,20 @@ public:
     /// @brief Destructor
     ///
     /// If the library is open, closes it.  This is principally a safety
-    /// feature to ensure closure in the case of an exception destroying
+    /// feature to ensure closure in the case of an exception destroying this
-    /// this object.
+    /// object.  However, see the caveat in the class header about when it is
-    ///
+    /// safe to unload libraries.
-    /// However, see the caveat in the class header about when it is safe
-    /// to unload libraries.
     ~LibraryManager() {
         static_cast<void>(unloadLibrary());
     }
 
     /// @brief Loads a library
     ///
-    /// Open the library and check the version.  If all is OK, load all
+    /// Open the library and check the version.  If all is OK, load all standard
-    /// standard symbols then call "load" if present.
+    /// symbols then call "load" if present.
     ///
-    /// @return true if the library loaded successfully, false otherwise.
+    /// @return true if the library loaded successfully, false otherwise. In the
-    ///         In the latter case, the library will be unloaded if possible.
+    ///         latter case, the library will be unloaded if possible.
     bool loadLibrary();
 
     /// @brief Unloads a library
@@ -107,8 +99,8 @@ public:
     /// Calls the libraries "unload" function if present, the closes the
     /// library.
     ///
-    /// However, see the caveat in the class header about when it is safe
+    /// However, see the caveat in the class header about when it is safe to
-    /// to unload libraries.
+    /// unload libraries.
     ///
     /// @return true if the library unloaded successfully, false if an error
     ///         occurred in the process (most likely the unload() function
@@ -139,16 +131,18 @@ protected:
     /// Closes the library associated with this LibraryManager.  A message is
     /// logged on an error.
     ///
-    /// @return true if the library closed successfully, false otherwise.
+    /// @return true if the library closed successfully, false otherwise. "true"
-    ///         "true" is also returned if the library were already closed
+    ///         is also returned if the library were already closed when this
-    ///         when this method was called.
+    ///         method was called.
     bool closeLibrary();
 
     /// @brief Check library version
     ///
     /// With the library open, accesses the "version()" function and, if
     /// present, checks the returned value against the hooks version symbol
-    /// for the currently running BIND 10.
+    /// for the currently running BIND 10.  The "version()" function is
+    /// mandatory and must be present (and return the correct value) for the
+    /// library to load.
     ///
     /// If there is no version() function, or if there is a mismatch in
     /// version number, a message logged.

src/lib/hooks/library_manager_collection.cc

@@ -0,0 +1,114 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <hooks/callout_manager.h>
+#include <hooks/library_manager.h>
+#include <hooks/library_manager_collection.h>
+
+namespace isc {
+namespace hooks {
+
+// Return callout manager for the loaded libraries.  This call is only valid
+// after one has been created for the loaded libraries (which includes the
+// case of no loaded libraries).
+//
+// Note that there is no real connection between the callout manager and the
+// libraries, other than it knows the number of libraries so can do sanity
+// checks on values passed to it.  However, this may change in the future,
+// so the hooks framework is written such that a callout manager is used only
+// with the LibraryManagerCollection that created it.  It is also the reason
+// why each LibraryManager contains a pointer to this CalloutManager.
+
+boost::shared_ptr<CalloutManager>
+LibraryManagerCollection::getCalloutManager() const {
+
+    // Only return a pointer if we have a CalloutManager created.
+    if (! callout_manager_) {
+        isc_throw(LoadLibrariesNotCalled, "must load hooks libraries before "
+                  "attempting to retrieve a CalloutManager for them");
+    }
+
+    return (callout_manager_);
+}
+
+// Load a set of libraries
+
+bool
+LibraryManagerCollection::loadLibraries() {
+
+    // Unload libraries if any are loaded.
+    static_cast<void>(unloadLibraries());
+
+    // Create the callout manager.  A pointer to this is maintained by
+    // each library.  Note that the callout manager does not hold any memory
+    // allocated by a library: although a library registers a callout (and so
+    // causes the creation of an entry in the CalloutManager's callout list),
+    // that creation is done by the CalloutManager itself.  The CalloutManager
+    // is created within the server.
+    //
+    // The upshot of this is that it is therefore safe for the CalloutManager
+    // to be deleted after all associated libraries are deleted, hence this
+    // link (LibraryManager -> CalloutManager) is safe.
+    callout_manager_.reset(new CalloutManager(library_names_.size()));
+
+    // Now iterate through the libraries are load them one by one.  We'll
+    for (int i = 0; i < library_names_.size(); ++i) {
+        // Create a pointer to the new library manager.  The index of this
+        // library is determined by the number of library managers currently
+        // loaded: note that the library indexes run from 1 to (number of loaded
+        // libraries).
+        boost::shared_ptr<LibraryManager> manager(
+                new LibraryManager(library_names_[i], lib_managers_.size() + 1,
+                                   callout_manager_));
+
+        // Load the library.  On success, add it to the list of loaded
+        // libraries.  On failure, an error will have been logged and the
+        // library closed.
+        if (manager->loadLibrary()) {
+            lib_managers_.push_back(manager);
+        }
+    }
+
+    // Update the CalloutManager's idea of the number of libraries it is
+    // handling.
+    callout_manager_->setNumLibraries(lib_managers_.size());
+
+    // Get an indication of whether all libraries loaded successfully.
+    bool status = (library_names_.size() == lib_managers_.size());
+
+    // Don't need the library names any more, so free up the space.
+    library_names_.clear();
+
+    return (status);
+}
+
+// Unload the libraries.
+
+void
+LibraryManagerCollection::unloadLibraries() {
+
+    // Delete the library managers in the reverse order to which they were
+    // created, then clear the library manager vector.
+    for (int i = lib_managers_.size() - 1; i >= 0; --i) {
+        lib_managers_[i].reset();
+    }
+    lib_managers_.clear();
+
+    // Get rid of the callout manager. (The other member, the list of library
+    // names, was cleared when the libraries were loaded.)
+    callout_manager_.reset();
+}
+
+} // namespace hooks
+} // namespace isc

src/lib/hooks/library_manager_collection.h

@@ -0,0 +1,133 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef LIBRARY_MANAGER_COLLECTION_H
+#define LIBRARY_MANAGER_COLLECTION_H
+
+#include <exceptions/exceptions.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <vector>
+
+namespace isc {
+namespace hooks {
+
+/// @brief LoadLibraries not called
+///
+/// Thrown if an attempt is made get a CalloutManager before the libraries
+/// have been loaded.
+class LoadLibrariesNotCalled : public Exception {
+public:
+    LoadLibrariesNotCalled(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+
+// Forward declarations
+class CalloutManager;
+class LibraryManager;
+
+/// @brief Library manager collection
+///
+/// The LibraryManagerCollection class, as the name implies, is responsible for
+/// managing the collection of LibraryManager objects that describe the loaded
+/// libraries.  As such, it converts a single operation (e.g load libraries)
+/// into multiple operations, one per library.  However, the class does more
+/// than that - it provides a single object with which to manage lifetimes.
+///
+/// As described in the LibraryManager documentation, a CalloutHandle may end
+/// up with pointers to memory within the address space of a loaded library.
+/// If the library is unloaded before this address space is deleted, the
+/// deletion of the CalloutHandle may attempt to free memory into the newly-
+/// unmapped address space and cause a segmentation fault.
+///
+/// To prevent this, each CalloutHandle maintains a shared pointer to the
+/// LibraryManagerCollection current when it was created.  In addition, the
+/// containing HooksManager object also maintains a shared pointer to it.  A
+/// a LibraryManagerCollection is never explicitly deleted: when a new set
+/// of libraries is loaded, the HooksManager clears its pointer to the
+/// collection.  The LibraryManagerCollection is only destroyed when all
+/// CallHandle objects referencing it are destroyed.
+///
+/// Note that this does not completely solve the problem - a hook function may
+/// have modified a packet being processed by the server and that packet may
+/// hold a pointer to memory in the library's virtual address space. To avoid
+/// a segmentation fault, that packet needs to free the memory before the
+/// LibraryManagerCollection is destroyed and this places demands on the server
+/// code.  However, the link with the CalloutHandle does at least mean that
+/// authors of server code do not need to be so careful about when they destroy
+/// CalloutHandles.
+
+class LibraryManagerCollection {
+public:
+    /// @brief Constructor
+    ///
+    /// @param List of libraries that this collection will manage.  The order
+    ///        of the libraries is important.
+    LibraryManagerCollection(const std::vector<std::string>& libraries)
+        : library_names_(libraries)
+    {}
+
+    /// @brief Destructor
+    ///
+    /// Unloads all loaded libraries.
+    ~LibraryManagerCollection() {
+        static_cast<void>(unloadLibraries());
+    }
+
+    /// @brief Load libraries
+    ///
+    /// Loads the libraries.  This creates the LibraryManager associated with
+    /// each library and calls its loadLibrary() method.  If a library fails
+    /// to load, the fact is noted but attempts are made to load the remaining
+    /// libraries.
+    bool loadLibraries();
+
+    /// @brief Get callout manager
+    ///
+    /// Returns a callout manager that can be used with this set of loaded
+    /// libraries (even if the number of loaded libraries is zero).  This
+    /// method may only be caslled after loadLibraries() has been called.
+    ///
+    /// @return Pointer to a callout manager for this set of libraries.
+    ///
+    /// @throw LoadLibrariesNotCalled Thrown if this method is called between
+    ///        construction and the time loadLibraries() is called.
+    boost::shared_ptr<CalloutManager> getCalloutManager() const;
+
+protected:
+    /// @brief Unload libraries
+    ///
+    /// Unloads and closes all loaded libraries.  They are unloaded in the
+    /// reverse order to the order in which they were loaded.
+    void unloadLibraries();
+
+private:
+
+    /// Vector of library names
+    std::vector<std::string>                        library_names_;
+
+    /// Vector of library managers
+    std::vector<boost::shared_ptr<LibraryManager> > lib_managers_;
+
+    /// Callout manager to be associated with the libraries
+    boost::shared_ptr<CalloutManager>               callout_manager_;
+};
+
+} // namespace hooks
+} // namespace isc
+
+
+#endif // LIBRARY_MANAGER_COLLECTION_H

src/lib/hooks/server_hooks.cc

@@ -55,7 +55,7 @@ ServerHooks::registerHook(const string& name) {
     inverse_hooks_[index] = name;
 
     // Log it if debug is enabled
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_REGISTER_HOOK).arg(name);
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_HOOK_REGISTERED).arg(name);
 
     // ... and return numeric index.
     return (index);
@@ -65,9 +65,6 @@ ServerHooks::registerHook(const string& name) {
 
 void
 ServerHooks::reset() {
-    // Log a wanring - although this is done during testing, it should never be
-    // seen in a production system.
-    LOG_WARN(hooks_logger, HOOKS_RESET_HOOK_LIST);
 
     // Clear out the name->index and index->name maps.
     hooks_.clear();
@@ -85,6 +82,10 @@ ServerHooks::reset() {
                   ". context_destroy: expected = " << CONTEXT_DESTROY <<
                   ", actual = " << destroy);
     }
+
+    // Log a warning - although this is done during testing, it should never be
+    // seen in a production system.
+    LOG_WARN(hooks_logger, HOOKS_HOOK_LIST_RESET);
 }
 
 // Find the name associated with a hook index.
@@ -130,33 +131,6 @@ ServerHooks::getHookNames() const {
     return (names);
 }
 
-// Hook registration function methods
-
-// Access the hook registration function vector itself
-
-std::vector<HookRegistrationFunction::RegistrationFunctionPtr>&
-HookRegistrationFunction::getFunctionVector() {
-    static std::vector<RegistrationFunctionPtr> reg_functions;
-    return (reg_functions);
-}
-
-// Constructor - add a registration function to the function vector
-
-HookRegistrationFunction::HookRegistrationFunction(
-    HookRegistrationFunction::RegistrationFunctionPtr reg_func) {
-    getFunctionVector().push_back(reg_func);
-}
-
-// Execute all registered registration functions
-
-void
-HookRegistrationFunction::execute(ServerHooks& hooks) {
-    std::vector<RegistrationFunctionPtr>& reg_functions = getFunctionVector();
-    for (int i = 0; i < reg_functions.size(); ++i) {
-        (*reg_functions[i])(hooks);
-    }
-}
-
 // Return global ServerHooks object
 
 ServerHooks&

src/lib/hooks/server_hooks.h

@@ -61,7 +61,7 @@ public:
 /// difference in a frequently-executed piece of code.)
 ///
 /// ServerHooks is a singleton object and is only accessible by the static
-/// method getserverHooks().
+/// method getServerHooks().
 
 class ServerHooks : public boost::noncopyable {
 public:
@@ -134,7 +134,7 @@ public:
     /// @return Vector of strings holding hook names.
     std::vector<std::string> getHookNames() const;
 
-    /// @brief Return ServerHookms object
+    /// @brief Return ServerHooks object
     ///
     /// Returns the global ServerHooks object.
     ///
@@ -151,7 +151,7 @@ private:
     ///
     /// Constructor is declared private to enforce the singleton nature of
     /// the object.  A reference to the singleton is obtainable through the
-    /// ggetServerHooks() static method.
+    /// getServerHooks() static method.
     ///
     /// @throws isc::Unexpected if the registration of the pre-defined hooks
     ///         fails in some way.
@@ -167,84 +167,6 @@ private:
     InverseHookCollection inverse_hooks_;   ///< Hook index/name collection
 };
 
-
-/// @brief Hooks Registration
-///
-/// All hooks must be registered before libraries are loaded and callouts
-/// assigned to them.  One way of doing this is to have a global list of hooks:
-/// the addition of any hook anywhere would require updating the list. This
-/// is possible and, if desired, the author of a server can do it.
-///
-/// An alternative is the option provided here, where each component of BIND 10
-/// registers the hooks they are using.  To do this, the component should
-/// create a hook registration function of the form:
-///
-/// @code
-/// static int hook1_num = -1;  // Initialize number for hook 1
-/// static int hook2_num = -1;  // Initialize number for hook 2
-///
-/// void myModuleRegisterHooks(ServerHooks& hooks) {
-///     hook1_num = hooks.registerHook("hook1");
-///     hook2_num = hooks.registerHook("hook2");
-/// }
-/// @endcode
-///
-/// ... which registers the hooks and stores the associated hook index. To
-/// avoid the need to add an explicit call to each of the hook registration
-/// functions to the server initialization code, the component should declare
-/// an object of this class in the same file as the registration function,
-/// but outside of any function.  The declaration should include the name
-/// of the registration function, i.e.
-///
-/// @code
-/// HookRegistrationFunction f(myModuleRegisterHooks);
-/// @code
-///
-/// The constructor of this object will run prior to main() getting called and
-/// will add the registration function to a list of such functions.  The server
-/// then calls the static class method "execute()" to run all the declared
-/// registration functions.
-
-class HookRegistrationFunction {
-public:
-    /// @brief Pointer to a hook registration function
-    typedef void (*RegistrationFunctionPtr)(ServerHooks&);
-
-    /// @brief Constructor
-    ///
-    /// For variables declared outside functions or methods, the constructors
-    /// are run after the program is loaded and before main() is called. This
-    /// constructor adds the passed pointer to a vector of such pointers.
-    HookRegistrationFunction(RegistrationFunctionPtr reg_func);
-
-    /// @brief Access registration function vector
-    ///
-    /// One of the problems with functions run prior to starting main() is the
-    /// "static initialization fiasco".  This occurs because the order in which
-    /// objects outside functions are constructed is not defined.  So if this
-    /// constructor were to depend on a vector declared externally, we would
-    /// not be able to guarantee that the vector had been initialised properly
-    /// before we used it.
-    ///
-    /// To get round this situation, the vector is declared statically within
-    /// a static function.  The first time the function is called, the vector
-    /// is initialized before it is used.
-    ///
-    /// This function returns a reference to the vector used to hold the
-    /// pointers.
-    ///
-    /// @return Reference to the (static) list of registration functions
-    static std::vector<RegistrationFunctionPtr>& getFunctionVector();
-
-    /// @brief Execute registration functions
-    ///
-    /// Called by the server initialization code, this function executes all
-    /// registered hook registration functions.
-    ///
-    /// @param hooks ServerHooks object to which hook information will be added.
-    static void execute(ServerHooks& hooks);
-};
-
 } // namespace util
 } // namespace isc
 

src/lib/hooks/tests/Makefile.am

@@ -27,18 +27,23 @@ TESTS_ENVIRONMENT = \
 TESTS =
 if HAVE_GTEST
 # Build shared libraries for testing.
-lib_LTLIBRARIES = libnvl.la libivl.la libbcl.la liblcl.la liblecl.la \
+lib_LTLIBRARIES = libnvl.la libivl.la libfxl.la libbcl.la liblcl.la liblecl.la \
                   libucl.la libfcl.la
- 
+
 # No version function
 libnvl_la_SOURCES  = no_version_library.cc
 libnvl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libnvl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
- 
+
 # Incorrect version function
 libivl_la_SOURCES  = incorrect_version_library.cc
 libivl_la_CXXFLAGS = $(AM_CXXFLAGS)
-libilv_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
+libivl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
+
+# All framework functions throw an exception
+libfxl_la_SOURCES = framework_exception_library.cc
+libfxl_la_CXXFLAGS = $(AM_CXXFLAGS)
+libfxl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
 
 # The basic callout library - contains standard callouts
 libbcl_la_SOURCES  = basic_callout_library.cc
@@ -71,10 +76,16 @@ TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += callout_handle_unittest.cc
 run_unittests_SOURCES += callout_manager_unittest.cc
+run_unittests_SOURCES += common_test_class.h
 run_unittests_SOURCES += handles_unittest.cc
+run_unittests_SOURCES += hooks_manager_unittest.cc
+run_unittests_SOURCES += library_manager_collection_unittest.cc
 run_unittests_SOURCES += library_manager_unittest.cc
 run_unittests_SOURCES += server_hooks_unittest.cc
 
+nodist_run_unittests_SOURCES  = marker_file.h
+nodist_run_unittests_SOURCES += test_libraries.h
+
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 run_unittests_LDADD    = $(AM_LDADD)    $(GTEST_LDADD)
@@ -88,4 +99,4 @@ endif
 
 noinst_PROGRAMS = $(TESTS)
 
-EXTRA_DIST = library_manager_unittest.cc.in marker_file.h.in
+EXTRA_DIST = marker_file.h.in test_libraries.h.in

src/lib/hooks/tests/basic_callout_library.cc

@@ -24,16 +24,16 @@
 /// - A context_create callout is supplied.
 ///
 /// - Three "standard" callouts are supplied corresponding to the hooks
-///   "lm_one", "lm_two", "lm_three".  All do some trivial calculations
+///   "hookpt_one", "hookpt_two", "hookpt_three".  All do some trivial calculations
 ///   on the arguments supplied to it and the context variables, returning
 ///   intermediate results through the "result" argument. The result of
-///   the calculation is:
+///   executing all four callouts in order is:
 ///
 ///   @f[ (10 + data_1) * data_2 - data_3 @f]
 ///
 ///   ...where data_1, data_2 and data_3 are the values passed in arguments of
-///   the same name to the three callouts (data_1 passed to lm_one, data_2 to
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
-///   lm_two etc.) and the result is returned in the argument "result".
+///   hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 #include <fstream>
@@ -58,7 +58,7 @@ context_create(CalloutHandle& handle) {
 // between callouts in the same library.)
 
 int
-lm_one(CalloutHandle& handle) {
+hookpt_one(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_1", data);
 
@@ -75,7 +75,7 @@ lm_one(CalloutHandle& handle) {
 // argument.
 
 int
-lm_two(CalloutHandle& handle) {
+hookpt_two(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_2", data);
 
@@ -91,7 +91,7 @@ lm_two(CalloutHandle& handle) {
 // Final callout subtracts the result in "data_3".
 
 int
-lm_three(CalloutHandle& handle) {
+hookpt_three(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_3", data);
 

src/lib/hooks/tests/callout_handle_unittest.cc

@@ -83,7 +83,7 @@ TEST_F(CalloutHandleTest, ArgumentDistinctSimpleType) {
     EXPECT_EQ(142, d);
 
     // Add a short (random value).
-    short e = -81; 
+    short e = -81;
     handle.setArgument("short", e);
     EXPECT_EQ(-81, e);
 

src/lib/hooks/tests/callout_manager_unittest.cc

@@ -22,6 +22,7 @@
 #include <gtest/gtest.h>
 
 #include <algorithm>
+#include <climits>
 #include <string>
 #include <vector>
 
@@ -177,35 +178,55 @@ TEST_F(CalloutManagerTest, BadConstructorParameters) {
     boost::scoped_ptr<CalloutManager> cm;
 
     // Invalid number of libraries
-    EXPECT_THROW(cm.reset(new CalloutManager(0)), BadValue);
     EXPECT_THROW(cm.reset(new CalloutManager(-1)), BadValue);
 }
 
 // Check the number of libraries is reported successfully.
 
-TEST_F(CalloutManagerTest, GetNumLibraries) {
+TEST_F(CalloutManagerTest, NumberOfLibraries) {
     boost::scoped_ptr<CalloutManager> cm;
 
     // Check two valid values of number of libraries to ensure that the
     // GetNumLibraries() returns the value set.
+    EXPECT_NO_THROW(cm.reset(new CalloutManager()));
+    EXPECT_EQ(0, cm->getNumLibraries());
+
+    EXPECT_NO_THROW(cm.reset(new CalloutManager(0)));
+    EXPECT_EQ(0, cm->getNumLibraries());
+
     EXPECT_NO_THROW(cm.reset(new CalloutManager(4)));
     EXPECT_EQ(4, cm->getNumLibraries());
 
     EXPECT_NO_THROW(cm.reset(new CalloutManager(42)));
     EXPECT_EQ(42, cm->getNumLibraries());
+
+    // Check that setting the number of libraries alterns the number reported.
+    EXPECT_NO_THROW(cm->setNumLibraries(27));
+    EXPECT_EQ(27, cm->getNumLibraries());
 }
 
 // Check that we can only set the current library index to the correct values.
 
 TEST_F(CalloutManagerTest, CheckLibraryIndex) {
-    // Check valid indexes
+    // Check valid indexes.  As the callout manager is sized for 10 libraries,
-    for (int i = 0; i < 4; ++i) {
+    // we expect:
+    //
+    // -1 to be valid as it is the standard "invalid" value.
+    // 0 to be valid for the pre-user library callouts
+    // 1-10 to be valid for the user-library callouts
+    // INT_MAX to be valid for the post-user library callouts
+    //
+    // All other values to be invalid.
+    for (int i = -1; i < 11; ++i) {
         EXPECT_NO_THROW(getCalloutManager()->setLibraryIndex(i));
+        EXPECT_EQ(i, getCalloutManager()->getLibraryIndex());
     }
+    EXPECT_NO_THROW(getCalloutManager()->setLibraryIndex(INT_MAX));
+    EXPECT_EQ(INT_MAX, getCalloutManager()->getLibraryIndex());
 
     // Check invalid ones
-    EXPECT_THROW(getCalloutManager()->setLibraryIndex(-1), NoSuchLibrary);
+    EXPECT_THROW(getCalloutManager()->setLibraryIndex(-2), NoSuchLibrary);
-    EXPECT_THROW(getCalloutManager()->setLibraryIndex(15), NoSuchLibrary);
+    EXPECT_THROW(getCalloutManager()->setLibraryIndex(11), NoSuchLibrary);
 }
 
 // Check that we can only register callouts on valid hook names.
@@ -237,7 +258,7 @@ TEST_F(CalloutManagerTest, RegisterCallout) {
     EXPECT_TRUE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Check that calling the callouts returns as expected. (This is also a
     // test of the callCallouts method.)
     callout_value_ = 0;
@@ -291,7 +312,7 @@ TEST_F(CalloutManagerTest, CalloutsPresent) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Set up so that hooks "alpha", "beta" and "delta" have callouts attached
     // to them, and callout  "gamma" does not. (In the statements below, the
     // exact callouts attached to a hook are not relevant - only the fact
@@ -327,7 +348,7 @@ TEST_F(CalloutManagerTest, CallNoCallouts) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Call the callouts on an arbitrary hook and ensure that nothing happens.
     callout_value_ = 475;
     getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
@@ -344,7 +365,7 @@ TEST_F(CalloutManagerTest, CallCalloutsSuccess) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributes one callout on hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(1);
@@ -388,7 +409,7 @@ TEST_F(CalloutManagerTest, CallCalloutsError) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributing one callout on hook "alpha". The first callout
     // returns an error (after adding its value to the result).
     callout_value_ = 0;
@@ -460,7 +481,7 @@ TEST_F(CalloutManagerTest, DeregisterSingleCallout) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Add a callout to hook "alpha" and check it is added correctly.
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -486,7 +507,7 @@ TEST_F(CalloutManagerTest, DeregisterSingleCalloutSameLibrary) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Add multiple callouts to hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -522,7 +543,7 @@ TEST_F(CalloutManagerTest, DeregisterMultipleCalloutsSameLibrary) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributes one callout on hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -578,7 +599,7 @@ TEST_F(CalloutManagerTest, DeregisterMultipleCalloutsMultipleLibraries) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributes two callouts to hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -607,7 +628,7 @@ TEST_F(CalloutManagerTest, DeregisterMultipleCalloutsMultipleLibraries) {
 TEST_F(CalloutManagerTest, DeregisterAllCallouts) {
     // Ensure that no callouts are attached to hook one.
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(alpha_index_));
-                 
+
     // Each library contributes two callouts to hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -647,7 +668,7 @@ TEST_F(CalloutManagerTest, MultipleCalloutsLibrariesHooks) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Register callouts on the alpha hook.
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -723,7 +744,7 @@ TEST_F(CalloutManagerTest, LibraryHandleRegistration) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Check that calling the callouts returns as expected. (This is also a
     // test of the callCallouts method.)
     callout_value_ = 0;
@@ -752,6 +773,112 @@ TEST_F(CalloutManagerTest, LibraryHandleRegistration) {
     EXPECT_EQ(1, callout_value_);
 }
 
+// A repeat of the test above, but using the alternate constructor for the
+// LibraryHandle.
+TEST_F(CalloutManagerTest, LibraryHandleAlternateConstructor) {
+    // Ensure that no callouts are attached to any of the hooks.
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(alpha_index_));
+
+    // Set up so that hooks "alpha" and "beta" have callouts attached from a
+    // different libraries.
+    LibraryHandle lh0(getCalloutManager().get(), 0);
+    lh0.registerCallout("alpha", callout_one);
+    lh0.registerCallout("alpha", callout_two);
+
+    LibraryHandle lh1(getCalloutManager().get(), 1);
+    lh1.registerCallout("alpha", callout_three);
+    lh1.registerCallout("alpha", callout_four);
+
+    // Check all is as expected.
+    EXPECT_TRUE(getCalloutManager()->calloutsPresent(alpha_index_));
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
+
+    // Check that calling the callouts returns as expected. (This is also a
+    // test of the callCallouts method.)
+    callout_value_ = 0;
+    getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
+    EXPECT_EQ(1234, callout_value_);
+
+    // Deregister a callout on library index 0 (after we check we can't
+    // deregister it through library index 1).
+    EXPECT_FALSE(lh1.deregisterCallout("alpha", callout_two));
+    callout_value_ = 0;
+    getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
+    EXPECT_EQ(1234, callout_value_);
+
+    EXPECT_TRUE(lh0.deregisterCallout("alpha", callout_two));
+    callout_value_ = 0;
+    getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
+    EXPECT_EQ(134, callout_value_);
+
+    // Deregister all callouts on library index 1.
+    EXPECT_TRUE(lh1.deregisterAllCallouts("alpha"));
+    callout_value_ = 0;
+    getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
+    EXPECT_EQ(1, callout_value_);
+}
+
+// Check that the pre- and post- user callout library handles work
+// appropriately with no user libraries.
+
+TEST_F(CalloutManagerTest, LibraryHandlePrePostNoLibraries) {
+    // Create a local callout manager and callout handle to reflect no libraries
+    // being loaded.
+    boost::shared_ptr<CalloutManager> manager(new CalloutManager(0));
+    CalloutHandle handle(manager);
+
+    // Ensure that no callouts are attached to any of the hooks.
+    EXPECT_FALSE(manager->calloutsPresent(alpha_index_));
+
+    // Setup the pre-and post callouts.
+    manager->getPostLibraryHandle().registerCallout("alpha", callout_four);
+    manager->getPreLibraryHandle().registerCallout("alpha", callout_one);
+    // Check all is as expected.
+    EXPECT_TRUE(manager->calloutsPresent(alpha_index_));
+    EXPECT_FALSE(manager->calloutsPresent(beta_index_));
+    EXPECT_FALSE(manager->calloutsPresent(gamma_index_));
+    EXPECT_FALSE(manager->calloutsPresent(delta_index_));
+
+    // Check that calling the callouts returns as expected.
+    callout_value_ = 0;
+    manager->callCallouts(alpha_index_, handle);
+    EXPECT_EQ(14, callout_value_);
+
+    // Deregister the pre- library callout.
+    EXPECT_TRUE(manager->getPreLibraryHandle().deregisterAllCallouts("alpha"));
+    callout_value_ = 0;
+    manager->callCallouts(alpha_index_, handle);
+    EXPECT_EQ(4, callout_value_);
+}
+
+// Repeat the tests with one user library.
+
+TEST_F(CalloutManagerTest, LibraryHandlePrePostUserLibrary) {
+
+    // Setup the pre-, library and post callouts.
+    getCalloutManager()->getPostLibraryHandle().registerCallout("alpha",
+                                                                callout_four);
+    getCalloutManager()->getPreLibraryHandle().registerCallout("alpha",
+                                                                callout_one);
+
+    // ... and set up a callout in between, on library number 2.
+    LibraryHandle lh1(getCalloutManager().get(), 2);
+    lh1.registerCallout("alpha", callout_five);
+
+    // Check all is as expected.
+    EXPECT_TRUE(getCalloutManager()->calloutsPresent(alpha_index_));
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
+    EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
+
+    // Check that calling the callouts returns as expected.
+    callout_value_ = 0;
+    getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
+    EXPECT_EQ(154, callout_value_);
+}
+
 // The setting of the hook index is checked in the handles_unittest
 // set of tests, as access restrictions mean it is not easily tested
 // on its own.

src/lib/hooks/tests/common_test_class.h

@@ -0,0 +1,142 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef COMMON_HOOKS_TEST_CLASS_H
+#define COMMON_HOOKS_TEST_CLASS_H
+
+#include <hooks/callout_handle.h>
+#include <hooks/callout_manager.h>
+#include <hooks/server_hooks.h>
+#include <hooks/tests/marker_file.h>
+
+#include <boost/shared_ptr.hpp>
+#include <gtest/gtest.h>
+
+/// @brief Common hooks test class
+///
+/// This class is a shared parent of the test fixture class in the tests of the
+/// higher-level hooks classes (LibraryManager, LibraryManagerCollection and
+/// HooksManager).  It
+///
+/// - sets the the ServerHooks object with three hooks and stores their
+///   indexes.
+/// - executes the callouts (which are assumed to perform a calculation)
+///   and checks the results.
+
+class HooksCommonTestClass {
+public:
+    /// @brief Constructor
+    HooksCommonTestClass() {
+
+        // Set up the server hooks.  ServerHooks is a singleton, so we reset it
+        // between each test.
+        isc::hooks::ServerHooks& hooks =
+            isc::hooks::ServerHooks::getServerHooks();
+        hooks.reset();
+        hookpt_one_index_ = hooks.registerHook("hookpt_one");
+        hookpt_two_index_ = hooks.registerHook("hookpt_two");
+        hookpt_three_index_ = hooks.registerHook("hookpt_three");
+    }
+
+    /// @brief Call callouts test
+    ///
+    /// All of the loaded libraries for which callouts are called register four
+    /// callouts: a context_create callout and three callouts that are attached
+    /// to hooks hookpt_one, hookpt_two and hookpt_three.  These four callouts,
+    /// executed in sequence, perform a series of calculations. Data is passed
+    /// between callouts in the argument list, in a variable named "result".
+    ///
+    /// context_create initializes the calculation by setting a seed
+    /// value, called r0 here.  This value is dependent on the library being
+    /// loaded.  Prior to that, the argument "result" is initialized to -1,
+    /// the purpose being to avoid exceptions when running this test with no
+    /// libraries loaded.
+    ///
+    /// Callout hookpt_one is passed a value d1 and performs a simple arithmetic
+    /// operation on it and r0 yielding a result r1.  Hence we can say that
+    /// @f[ r1 = hookpt_one(r0, d1) @f]
+    ///
+    /// Callout hookpt_two is passed a value d2 and peforms another simple
+    /// arithmetic operation on it and d2, yielding r2, i.e.
+    /// @f[ r2 = hookpt_two(d1, d2) @f]
+    ///
+    /// hookpt_three does a similar operation giving
+    /// @f[ r3 = hookpt_three(r2, d3) @f].
+    ///
+    /// The details of the operations hookpt_one, hookpt_two and hookpt_three
+    /// depend on the library, so the results obtained not only depend on
+    /// the data, but also on the library loaded.  This method is passed both
+    /// data and expected results.  It executes the three callouts in sequence,
+    /// checking the intermediate and final results.  Only if the expected
+    /// library has been loaded correctly and the callouts in it registered
+    /// correctly will be the results be as expected.
+    ///
+    /// It is assumed that callout_manager_ has been set up appropriately.
+    ///
+    /// @note The CalloutHandle used in the calls is declared locally here.
+    ///       The advantage of this (apart from scope reduction) is that on
+    ///       exit, it is destroyed.  This removes any references to memory
+    ///       allocated by loaded libraries while they are still loaded.
+    ///
+    /// @param manager CalloutManager to use for the test
+    /// @param r0...r3, d1..d3 Data (dN) and expected results (rN) - both
+    ///        intermediate and final.  The arguments are ordered so that they
+    ///        appear in the argument list in the order they are used.
+    void executeCallCallouts(
+            const boost::shared_ptr<isc::hooks::CalloutManager>& manager,
+            int r0, int d1, int r1, int d2, int r2, int d3, int r3) {
+        static const char* COMMON_TEXT = " callout returned the wong value";
+        static const char* RESULT = "result";
+
+        int result;
+
+        // Set up a callout handle for the calls.
+        isc::hooks::CalloutHandle handle(manager);
+
+        // Initialize the argument RESULT.  This simplifies testing by
+        // eliminating the generation of an exception when we try the unload
+        // test.  In that case, RESULT is unchanged.
+        handle.setArgument(RESULT, -1);
+
+        // Seed the calculation.
+        manager->callCallouts(isc::hooks::ServerHooks::CONTEXT_CREATE, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
+
+        // Perform the first calculation.
+        handle.setArgument("data_1", d1);
+        manager->callCallouts(hookpt_one_index_, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r1, result) << "hookpt_one" << COMMON_TEXT;
+
+        // ... the second ...
+        handle.setArgument("data_2", d2);
+        manager->callCallouts(hookpt_two_index_, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r2, result) << "hookpt_two" << COMMON_TEXT;
+
+        // ... and the third.
+        handle.setArgument("data_3", d3);
+        manager->callCallouts(hookpt_three_index_, handle);
+        handle.getArgument(RESULT, result);
+        EXPECT_EQ(r3, result) << "hookpt_three" << COMMON_TEXT;
+    }
+
+    /// Hook indexes.  These are are made public for ease of reference.
+    int hookpt_one_index_;
+    int hookpt_two_index_;
+    int hookpt_three_index_;
+};
+
+#endif // COMMON_HOOKS_TEST_CLASS_H

src/lib/hooks/tests/framework_exception_library.cc

@@ -0,0 +1,47 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+/// @file
+/// @brief Framework exception library
+///
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
+///
+/// - All three framework functions are supplied (version(), load() and
+///   unload()) and all generate an exception.
+
+#include <hooks/hooks.h>
+
+#include <exception>
+
+extern "C" {
+
+int
+version() {
+    throw std::exception();
+}
+
+int
+load(isc::hooks::LibraryHandle& handle) {
+    throw std::exception();
+}
+
+int
+unload() {
+    throw std::exception();
+}
+
+};
+

src/lib/hooks/tests/full_callout_library.cc

@@ -34,8 +34,8 @@
 ///   @f[ ((7 * data_1) - data_2) * data_3 @f]
 ///
 ///   ...where data_1, data_2 and data_3 are the values passed in arguments of
-///   the same name to the three callouts (data_1 passed to lm_one, data_2 to
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
-///   lm_two etc.) and the result is returned in the argument "result".
+///   hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 #include <hooks/tests/marker_file.h>
@@ -61,7 +61,7 @@ context_create(CalloutHandle& handle) {
 // between callouts in the same library.)
 
 int
-lm_one(CalloutHandle& handle) {
+hookpt_one(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_1", data);
 
@@ -78,7 +78,7 @@ lm_one(CalloutHandle& handle) {
 // running total.
 
 static int
-lm_nonstandard_two(CalloutHandle& handle) {
+hook_nonstandard_two(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_2", data);
 
@@ -94,7 +94,7 @@ lm_nonstandard_two(CalloutHandle& handle) {
 // Final callout multplies the current running total by data_3.
 
 static int
-lm_nonstandard_three(CalloutHandle& handle) {
+hook_nonstandard_three(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_3", data);
 
@@ -117,8 +117,8 @@ version() {
 int
 load(LibraryHandle& handle) {
     // Register the non-standard functions
-    handle.registerCallout("lm_two", lm_nonstandard_two);
+    handle.registerCallout("hookpt_two", hook_nonstandard_two);
-    handle.registerCallout("lm_three", lm_nonstandard_three);
+    handle.registerCallout("hookpt_three", hook_nonstandard_three);
 
     return (0);
 }

src/lib/hooks/tests/handles_unittest.cc

@@ -301,10 +301,10 @@ TEST_F(HandlesTest, ContextAccessCheck) {
     // Create the callout handles and distinguish them by setting the
     // "handle_num" argument.
     CalloutHandle callout_handle_1(getCalloutManager());
-    callout_handle_1.setArgument("handle_num", static_cast<int>(1)); 
+    callout_handle_1.setArgument("handle_num", static_cast<int>(1));
 
     CalloutHandle callout_handle_2(getCalloutManager());
-    callout_handle_2.setArgument("handle_num", static_cast<int>(2)); 
+    callout_handle_2.setArgument("handle_num", static_cast<int>(2));
 
     // Now call the callouts attached to the first three hooks.  Each hook is
     // called twice (once for each callout handle) before the next hook is
@@ -606,7 +606,7 @@ TEST_F(HandlesTest, DynamicRegistrationAnotherHook) {
 
     // See what we get for calling the callouts on alpha first.
     CalloutHandle callout_handle_1(getCalloutManager());
-    callout_handle_1.setArgument("handle_num", static_cast<int>(1)); 
+    callout_handle_1.setArgument("handle_num", static_cast<int>(1));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_1);
 
     zero_results();
@@ -622,7 +622,7 @@ TEST_F(HandlesTest, DynamicRegistrationAnotherHook) {
 
     // Use a new callout handle so as to get fresh callout context.
     CalloutHandle callout_handle_2(getCalloutManager());
-    callout_handle_2.setArgument("handle_num", static_cast<int>(2)); 
+    callout_handle_2.setArgument("handle_num", static_cast<int>(2));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_2);
 
     zero_results();
@@ -654,7 +654,7 @@ TEST_F(HandlesTest, DynamicRegistrationSameHook) {
 
     // See what we get for calling the callouts on alpha first.
     CalloutHandle callout_handle_1(getCalloutManager());
-    callout_handle_1.setArgument("handle_num", static_cast<int>(1)); 
+    callout_handle_1.setArgument("handle_num", static_cast<int>(1));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_1);
     zero_results();
     getCalloutManager()->callCallouts(delta_index_, callout_handle_1);
@@ -662,7 +662,7 @@ TEST_F(HandlesTest, DynamicRegistrationSameHook) {
 
     // Run it again - we should have added something to this hook.
     CalloutHandle callout_handle_2(getCalloutManager());
-    callout_handle_2.setArgument("handle_num", static_cast<int>(2)); 
+    callout_handle_2.setArgument("handle_num", static_cast<int>(2));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_2);
     zero_results();
     getCalloutManager()->callCallouts(delta_index_, callout_handle_2);
@@ -670,7 +670,7 @@ TEST_F(HandlesTest, DynamicRegistrationSameHook) {
 
     // And a third time...
     CalloutHandle callout_handle_3(getCalloutManager());
-    callout_handle_3.setArgument("handle_num", static_cast<int>(3)); 
+    callout_handle_3.setArgument("handle_num", static_cast<int>(3));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_3);
     zero_results();
     getCalloutManager()->callCallouts(delta_index_, callout_handle_3);
@@ -694,7 +694,7 @@ TEST_F(HandlesTest, DynamicDeregistrationDifferentHook) {
 
     // Call the callouts on alpha
     CalloutHandle callout_handle_1(getCalloutManager());
-    callout_handle_1.setArgument("handle_num", static_cast<int>(1)); 
+    callout_handle_1.setArgument("handle_num", static_cast<int>(1));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_1);
 
     zero_results();
@@ -707,7 +707,7 @@ TEST_F(HandlesTest, DynamicDeregistrationDifferentHook) {
     // The run of the callouts should have altered the callout list on the
     // first library for hook alpha, so call again to make sure.
     CalloutHandle callout_handle_2(getCalloutManager());
-    callout_handle_2.setArgument("handle_num", static_cast<int>(2)); 
+    callout_handle_2.setArgument("handle_num", static_cast<int>(2));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_2);
 
     zero_results();
@@ -734,7 +734,7 @@ TEST_F(HandlesTest, DynamicDeregistrationSameHook) {
 
     // Call the callouts on alpha
     CalloutHandle callout_handle_1(getCalloutManager());
-    callout_handle_1.setArgument("handle_num", static_cast<int>(1)); 
+    callout_handle_1.setArgument("handle_num", static_cast<int>(1));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_1);
 
     zero_results();
@@ -745,7 +745,7 @@ TEST_F(HandlesTest, DynamicDeregistrationSameHook) {
     // The run of the callouts should have altered the callout list on the
     // first library for hook alpha, so call again to make sure.
     CalloutHandle callout_handle_2(getCalloutManager());
-    callout_handle_2.setArgument("handle_num", static_cast<int>(2)); 
+    callout_handle_2.setArgument("handle_num", static_cast<int>(2));
     getCalloutManager()->callCallouts(alpha_index_, callout_handle_2);
 
     zero_results();
@@ -934,7 +934,7 @@ TEST_F(HandlesTest, HookName) {
     getCalloutManager()->registerCallout("alpha", callout_hook_name);
     getCalloutManager()->registerCallout("beta", callout_hook_name);
 
-    // Call alpha abd beta callouts and check the hook to which they belong.
+    // Call alpha and beta callouts and check the hook to which they belong.
     CalloutHandle callout_handle(getCalloutManager());
 
     EXPECT_EQ(std::string(""), HandlesTest::common_string_);
@@ -949,8 +949,8 @@ TEST_F(HandlesTest, HookName) {
     // only callout in the list.
     getCalloutManager()->setLibraryIndex(1);
     getCalloutManager()->registerCallout("gamma", callout_hook_dummy);
-    getCalloutManager()->registerCallout("gamma", callout_hook_dummy);
     getCalloutManager()->registerCallout("gamma", callout_hook_name);
+    getCalloutManager()->registerCallout("gamma", callout_hook_dummy);
 
     EXPECT_EQ(std::string("beta"), HandlesTest::common_string_);
     getCalloutManager()->callCallouts(gamma_index_, callout_handle);

src/lib/hooks/tests/hooks_manager_unittest.cc

@@ -0,0 +1,454 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <hooks/callout_handle.h>
+#include <hooks/hooks_manager.h>
+#include <hooks/server_hooks.h>
+
+#include <hooks/tests/common_test_class.h>
+#include <hooks/tests/test_libraries.h>
+
+#include <boost/shared_ptr.hpp>
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <string>
+
+
+using namespace isc;
+using namespace isc::hooks;
+using namespace std;
+
+namespace {
+
+/// @brief Hooks manager collection test class
+
+class HooksManagerTest : public ::testing::Test,
+                         public HooksCommonTestClass {
+public:
+    /// @brief Constructor
+    ///
+    /// Reset the hooks manager.  The hooks manager is a singleton, so needs
+    /// to be reset for each test.
+    HooksManagerTest() {
+        HooksManager::unloadLibraries();
+    }
+
+    /// @brief Destructor
+    ///
+    /// Unload all libraries.
+    ~HooksManagerTest() {
+        HooksManager::unloadLibraries();
+    }
+
+
+    /// @brief Call callouts test
+    ///
+    /// See the header for HooksCommonTestClass::execute for details.
+    ///
+    /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
+    ///        are ordered so that the variables appear in the argument list in
+    ///        the order they are used.
+    void executeCallCallouts(int r0, int d1, int r1, int d2, int r2, int d3,
+                             int r3) {
+        static const char* COMMON_TEXT = " callout returned the wong value";
+        static const char* RESULT = "result";
+
+        // Get a CalloutHandle for the calculation.
+        CalloutHandlePtr handle = HooksManager::createCalloutHandle();
+
+        // Initialize the argument RESULT.  This simplifies testing by
+        // eliminating the generation of an exception when we try the unload
+        // test.  In that case, RESULT is unchanged.
+        int result = -1;
+        handle->setArgument(RESULT, result);
+
+        // Seed the calculation.
+        HooksManager::callCallouts(isc::hooks::ServerHooks::CONTEXT_CREATE,
+                                   *handle);
+        handle->getArgument(RESULT, result);
+        EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
+
+        // Perform the first calculation.
+        handle->setArgument("data_1", d1);
+        HooksManager::callCallouts(hookpt_one_index_, *handle);
+        handle->getArgument(RESULT, result);
+        EXPECT_EQ(r1, result) << "hookpt_one" << COMMON_TEXT;
+
+        // ... the second ...
+        handle->setArgument("data_2", d2);
+        HooksManager::callCallouts(hookpt_two_index_, *handle);
+        handle->getArgument(RESULT, result);
+        EXPECT_EQ(r2, result) << "hookpt_two" << COMMON_TEXT;
+
+        // ... and the third.
+        handle->setArgument("data_3", d3);
+        HooksManager::callCallouts(hookpt_three_index_, *handle);
+        handle->getArgument(RESULT, result);
+        EXPECT_EQ(r3, result) << "hookpt_three" << COMMON_TEXT;
+    }
+
+};
+
+// This is effectively the same test as for LibraryManager, but using the
+// HooksManager object.
+
+TEST_F(HooksManagerTest, LoadLibraries) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Load the libraries.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // Execute the callouts.  The first library implements the calculation.
+    //
+    // r3 = (7 * d1 - d2) * d3
+    //
+    // The last-loaded library implements the calculation
+    //
+    // r3 = (10 + d1) * d2 - d3
+    //
+    // Putting the processing for each library together in the appropriate
+    // order, we get:
+    //
+    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    {
+        SCOPED_TRACE("Calculation with libraries loaded");
+        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+    }
+
+    // Try unloading the libraries.
+    EXPECT_NO_THROW(HooksManager::unloadLibraries());
+
+    // Re-execute the calculation - callouts can be called but as nothing
+    // happens, the result should always be -1.
+    {
+        SCOPED_TRACE("Calculation with libraries not loaded");
+        executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
+    }
+}
+
+// This is effectively the same test as above, but with a library generating
+// an error when loaded. It is expected that the failing library will not be
+// loaded, but others will be.
+
+TEST_F(HooksManagerTest, LoadLibrariesWithError) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(INCORRECT_VERSION_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Load the libraries.  We expect a failure return because one of the
+    // libraries fails to load.
+    EXPECT_FALSE(HooksManager::loadLibraries(library_names));
+
+    // Execute the callouts.  The first library implements the calculation.
+    //
+    // r3 = (7 * d1 - d2) * d3
+    //
+    // The last-loaded library implements the calculation
+    //
+    // r3 = (10 + d1) * d2 - d3
+    //
+    // Putting the processing for each library together in the appropriate
+    // order, we get:
+    //
+    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    {
+        SCOPED_TRACE("Calculation with libraries loaded");
+        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+    }
+
+    // Try unloading the libraries.
+    EXPECT_NO_THROW(HooksManager::unloadLibraries());
+
+    // Re-execute the calculation - callouts can be called but as nothing
+    // happens, the result should always be -1.
+    {
+        SCOPED_TRACE("Calculation with libraries not loaded");
+        executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
+    }
+}
+
+// Test that we can unload a set of libraries while we have a CalloutHandle
+// created on them in existence, and can delete the handle afterwards.
+
+TEST_F(HooksManagerTest, CalloutHandleUnloadLibrary) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+
+    // Load the libraries.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // Execute the callouts.  Thiis library implements:
+    //
+    // r3 = (7 * d1 - d2) * d3
+    {
+        SCOPED_TRACE("Calculation with full callout library loaded");
+        executeCallCallouts(7, 4, 28, 8, 20, 2, 40);
+    }
+
+    // Get an outstanding callout handle on this library.
+    CalloutHandlePtr handle = HooksManager::createCalloutHandle();
+
+    // Execute once of the callouts again to ensure that the handle contains
+    // memory allocated by the library.
+    HooksManager::callCallouts(ServerHooks::CONTEXT_CREATE, *handle);
+
+    // Unload the libraries.
+    HooksManager::unloadLibraries();
+
+    // Deleting the callout handle should not cause a segmentation fault.
+    handle.reset();
+}
+
+// Test that we can load a new set of libraries while we have a CalloutHandle
+// created on them in existence, and can delete the handle afterwards.
+
+TEST_F(HooksManagerTest, CalloutHandleLoadLibrary) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+
+    // Load the libraries.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // Execute the callouts.  Thiis library implements:
+    //
+    // r3 = (7 * d1 - d2) * d3
+    {
+        SCOPED_TRACE("Calculation with full callout library loaded");
+        executeCallCallouts(7, 4, 28, 8, 20, 2, 40);
+    }
+
+    // Get an outstanding callout handle on this library and execute one of
+    // the callouts again to ensure that the handle contains memory allocated
+    // by the library.
+    CalloutHandlePtr handle = HooksManager::createCalloutHandle();
+    HooksManager::callCallouts(ServerHooks::CONTEXT_CREATE, *handle);
+
+    // Load a new library that implements the calculation
+    //
+    // r3 = (10 + d1) * d2 - d3
+    std::vector<std::string> new_library_names;
+    new_library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Load the libraries.
+    EXPECT_TRUE(HooksManager::loadLibraries(new_library_names));
+
+    // Execute the calculation.  Note that we still have the CalloutHandle
+    // for the old library: however, this should not affect the new calculation.
+    {
+        SCOPED_TRACE("Calculation with basic callout library loaded");
+        executeCallCallouts(10, 7, 17, 3, 51, 16, 35);
+    }
+
+    // Deleting the old callout handle should not cause a segmentation fault.
+    handle.reset();
+}
+
+// This is effectively the same test as the LoadLibraries test.
+
+TEST_F(HooksManagerTest, ReloadSameLibraries) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Load the libraries.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // Execute the callouts.  See the LoadLibraries test for an explanation of
+    // the calculation.
+    {
+        SCOPED_TRACE("Calculation with libraries loaded");
+        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+    }
+
+    // Try reloading the libraries and re-execute the calculation - we should
+    // get the same results.
+    EXPECT_NO_THROW(HooksManager::loadLibraries(library_names));
+    {
+        SCOPED_TRACE("Calculation with libraries reloaded");
+        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+    }
+}
+
+TEST_F(HooksManagerTest, ReloadLibrariesReverseOrder) {
+
+    // Set up the list of libraries to be loaded and load them.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // Execute the callouts.  The first library implements the calculation.
+    //
+    // r3 = (7 * d1 - d2) * d3
+    //
+    // The last-loaded library implements the calculation
+    //
+    // r3 = (10 + d1) * d2 - d3
+    //
+    // Putting the processing for each library together in the given order
+    // gives.
+    //
+    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    {
+        SCOPED_TRACE("Calculation with libraries loaded");
+        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
+    }
+
+    // Reload the libraries in the reverse order.
+    std::reverse(library_names.begin(), library_names.end());
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // The calculation in the reverse order gives:
+    //
+    // r3 = ((((7 + d1) * d1) * d2 - d2) - d3) * d3
+    {
+        SCOPED_TRACE("Calculation with libraries loaded in reverse order");
+        executeCallCallouts(7, 3, 30, 3, 87, 7, 560);
+    }
+}
+
+// Local callouts for the test of server-registered callouts.
+
+namespace {
+
+    int
+testPreCallout(CalloutHandle& handle) {
+    handle.setArgument("result", static_cast<int>(1027));
+    return (0);
+}
+
+int
+testPostCallout(CalloutHandle& handle) {
+    int result;
+    handle.getArgument("result", result);
+    result *= 2;
+    handle.setArgument("result", result);
+    return (0);
+}
+
+}
+
+// The next test registers the pre and post- callouts above for hook hookpt_two,
+// and checks they are called.
+
+TEST_F(HooksManagerTest, PrePostCalloutTest) {
+
+    // Load a single library.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+
+    // Load the pre- and post- callouts.
+    HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two",
+                                                             testPreCallout);
+    HooksManager::postCalloutsLibraryHandle().registerCallout("hookpt_two",
+                                                              testPostCallout);
+
+    // Execute the callouts.  hookpt_two implements the calculation:
+    //
+    //  "result - data_2"
+    //
+    // With the pre- and post- callouts above, the result expected is
+    //
+    // (1027 - data_2) * 2
+    CalloutHandlePtr handle = HooksManager::createCalloutHandle();
+    handle->setArgument("result", static_cast<int>(0));
+    handle->setArgument("data_2", static_cast<int>(15));
+
+    HooksManager::callCallouts(hookpt_two_index_, *handle);
+
+    int result = 0;
+    handle->getArgument("result", result);
+    EXPECT_EQ(2024, result);
+
+    // ... and check that the pre- and post- callout functions don't survive a
+    // reload.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+    handle = HooksManager::createCalloutHandle();
+
+    handle->setArgument("result", static_cast<int>(0));
+    handle->setArgument("data_2", static_cast<int>(15));
+
+    HooksManager::callCallouts(hookpt_two_index_, *handle);
+
+    result = 0;
+    handle->getArgument("result", result);
+    EXPECT_EQ(-15, result);
+}
+
+// Check that everything works even with no libraries loaded.  First that
+// calloutsPresent() always returns false.
+
+TEST_F(HooksManagerTest, NoLibrariesCalloutsPresent) {
+    // No callouts should be present on any hooks.
+    EXPECT_FALSE(HooksManager::calloutsPresent(hookpt_one_index_));
+    EXPECT_FALSE(HooksManager::calloutsPresent(hookpt_two_index_));
+    EXPECT_FALSE(HooksManager::calloutsPresent(hookpt_three_index_));
+}
+
+TEST_F(HooksManagerTest, NoLibrariesCallCallouts) {
+    executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
+}
+
+// Test the encapsulation of the ServerHooks::registerHook() method.
+
+TEST_F(HooksManagerTest, RegisterHooks) {
+    ServerHooks::getServerHooks().reset();
+    EXPECT_EQ(2, ServerHooks::getServerHooks().getCount());
+
+    // Check that the hook indexes are as expected. (Use temporary variables
+    // as it appears that Google test can't access the constants.)
+    int sh_cc = ServerHooks::CONTEXT_CREATE;
+    int hm_cc = HooksManager::CONTEXT_CREATE;
+    EXPECT_EQ(sh_cc, hm_cc);
+
+    int sh_cd = ServerHooks::CONTEXT_DESTROY;
+    int hm_cd = HooksManager::CONTEXT_DESTROY;
+    EXPECT_EQ(sh_cd, hm_cd);
+
+    // Register a few hooks and check we have the indexes as expected.
+    EXPECT_EQ(2, HooksManager::registerHook(string("alpha")));
+    EXPECT_EQ(3, HooksManager::registerHook(string("beta")));
+    EXPECT_EQ(4, HooksManager::registerHook(string("gamma")));
+    EXPECT_THROW(static_cast<void>(HooksManager::registerHook(string("alpha"))),
+                 DuplicateHook);
+
+    // ... an check the hooks are as we expect.
+    EXPECT_EQ(5, ServerHooks::getServerHooks().getCount());
+    vector<string> names = ServerHooks::getServerHooks().getHookNames();
+    sort(names.begin(), names.end());
+
+    EXPECT_EQ(string("alpha"), names[0]);
+    EXPECT_EQ(string("beta"), names[1]);
+    EXPECT_EQ(string("context_create"), names[2]);
+    EXPECT_EQ(string("context_destroy"), names[3]);
+    EXPECT_EQ(string("gamma"), names[4]);
+}
+
+
+} // Anonymous namespace

src/lib/hooks/tests/library_manager_collection_unittest.cc

@@ -0,0 +1,184 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <hooks/callout_handle.h>
+#include <hooks/callout_manager.h>
+#include <hooks/library_manager.h>
+#include <hooks/library_manager_collection.h>
+
+#include <hooks/tests/common_test_class.h>
+#include <hooks/tests/test_libraries.h>
+
+#include <boost/shared_ptr.hpp>
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <string>
+
+
+using namespace isc;
+using namespace isc::hooks;
+using namespace std;
+
+namespace {
+
+/// @brief Library manager collection test class
+
+class LibraryManagerCollectionTest : public ::testing::Test,
+                                     public HooksCommonTestClass {
+};
+
+/// @brief Public library manager collection class
+///
+/// This is an instance of the LibraryManagerCollection class but with the
+/// protected methods made public for test purposes.
+
+class PublicLibraryManagerCollection
+                : public isc::hooks::LibraryManagerCollection {
+public:
+    /// @brief Constructor
+    ///
+    /// @param List of libraries that this collection will manage.  The order
+    ///        of the libraries is important.
+    PublicLibraryManagerCollection(const std::vector<std::string>& libraries)
+        : LibraryManagerCollection(libraries)
+    {}
+
+    /// Public methods that call protected methods on the superclass.
+    using LibraryManagerCollection::unloadLibraries;
+};
+
+
+// This is effectively the same test as for LibraryManager, but using the
+// LibraryManagerCollection object.
+
+TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Set up the library manager collection and get the callout manager we'll
+    // be using.
+    PublicLibraryManagerCollection lm_collection(library_names);
+
+    // Load the libraries.
+    EXPECT_TRUE(lm_collection.loadLibraries());
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
+
+    // Execute the callouts.  The first library implements the calculation.
+    //
+    // r3 = (7 * d1 - d2) * d3
+    //
+    // The last-loaded library implements the calculation
+    //
+    // r3 = (10 + d1) * d2 - d3
+    //
+    // Putting the processing for each library together in the appropriate
+    // order, we get:
+    //
+    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    {
+        SCOPED_TRACE("Doing calculation with libraries loaded");
+        executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
+    }
+
+    // Try unloading the libraries.
+    EXPECT_NO_THROW(lm_collection.unloadLibraries());
+
+    // Re-execute the calculation - callouts can be called but as nothing
+    // happens, the result should always be -1.
+    {
+        SCOPED_TRACE("Doing calculation with libraries not loaded");
+        executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
+    }
+}
+
+// This is effectively the same test as above, but with a library generating
+// an error when loaded. It is expected that the failing library will not be
+// loaded, but others will be.
+
+TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(INCORRECT_VERSION_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Set up the library manager collection and get the callout manager we'll
+    // be using.
+    PublicLibraryManagerCollection lm_collection(library_names);
+
+    // Load the libraries.  We expect a failure status to be returned as
+    // one of the libraries failed to load.
+    EXPECT_FALSE(lm_collection.loadLibraries());
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
+
+    // Expect only two libraries were loaded.
+    EXPECT_EQ(2, manager->getNumLibraries());
+
+    // Execute the callouts.  The first library implements the calculation.
+    //
+    // r3 = (7 * d1 - d2) * d3
+    //
+    // The last-loaded library implements the calculation
+    //
+    // r3 = (10 + d1) * d2 - d3
+    //
+    // Putting the processing for each library together in the appropriate
+    // order, we get:
+    //
+    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    {
+        SCOPED_TRACE("Doing calculation with libraries loaded");
+        executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
+    }
+
+    // Try unloading the libraries.
+    EXPECT_NO_THROW(lm_collection.unloadLibraries());
+
+    // Re-execute the calculation - callouts can be called but as nothing
+    // happens, the result should always be -1.
+    {
+        SCOPED_TRACE("Doing calculation with libraries not loaded");
+        executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
+    }
+}
+
+// Check that everything works even with no libraries loaded.
+
+TEST_F(LibraryManagerCollectionTest, NoLibrariesLoaded) {
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+
+    // Set up the library manager collection and get the callout manager we'll
+    // be using.
+    LibraryManagerCollection lm_collection(library_names);
+    EXPECT_TRUE(lm_collection.loadLibraries());
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
+
+    // Load the libraries.
+    EXPECT_TRUE(lm_collection.loadLibraries());
+
+    // Eecute the calculation - callouts can be called but as nothing
+    // happens, the result should always be -1.
+    executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
+}
+
+} // Anonymous namespace

src/lib/hooks/tests/library_manager_unittest.cc

@@ -16,7 +16,10 @@
 #include <hooks/callout_manager.h>
 #include <hooks/library_manager.h>
 #include <hooks/server_hooks.h>
+
+#include <hooks/tests/common_test_class.h>
 #include <hooks/tests/marker_file.h>
+#include <hooks/tests/test_libraries.h>
 
 #include <gtest/gtest.h>
 
@@ -35,23 +38,15 @@ namespace {
 
 /// @brief Library manager test class
 
-class LibraryManagerTest : public ::testing::Test {
+class LibraryManagerTest : public ::testing::Test,
+                           public HooksCommonTestClass {
 public:
     /// @brief Constructor
     ///
-    /// Sets up a collection of three LibraryHandle objects to use in the test.
+    /// Initializes the CalloutManager object used in the tests.  It sets it
+    /// up with the hooks initialized in the HooksCommonTestClass object and
+    /// with four libraries.
     LibraryManagerTest() {
-
-        // Set up the server hooks.  ServerHooks is a singleton, so we reset it
-        // between each test.
-        ServerHooks& hooks = ServerHooks::getServerHooks();
-        hooks.reset();
-        lm_one_index_ = hooks.registerHook("lm_one");
-        lm_two_index_ = hooks.registerHook("lm_two");
-        lm_three_index_ = hooks.registerHook("lm_three");
-
-        // Set up the callout manager with these hooks.  Assume a maximum of
-        // four libraries.
         callout_manager_.reset(new CalloutManager(4));
 
         // Ensure the marker file is not present at the start of a test.
@@ -65,85 +60,47 @@ public:
         static_cast<void>(unlink(MARKER_FILE));
     }
 
+    /// @brief Marker file present
+    ///
+    /// Convenience function to check whether a marker file is present.  It
+    /// does this by opening the file.
+    ///
+    /// @return true if the marker file is present.
+    bool markerFilePresent() const {
+
+        // Try to open it.
+        std::fstream marker;
+        marker.open(MARKER_FILE, std::fstream::in);
+
+        // Check if it is open and close it if so.
+        bool exists = marker.is_open();
+        if (exists) {
+            marker.close();
+        }
+
+        return (exists);
+    }
+
     /// @brief Call callouts test
     ///
-    /// All of the loaded libraries for which callouts are called register four
+    /// A wrapper around the method of the same name in the HooksCommonTestClass
-    /// callouts: a context_create callout and three callouts that are attached
+    /// object, this passes this class's CalloutManager to that method.
-    /// to hooks lm_one, lm_two and lm_three.  These four callouts, executed
-    /// in sequence, perform a series of calculations. Data is passed between
-    /// callouts in the argument list, in a variable named "result".
-    ///
-    /// context_create initializes the calculation by setting a seed
-    /// value, called r0 here.
-    ///
-    /// Callout lm_one is passed a value d1 and performs a simple arithmetic
-    /// operation on it and r0 yielding a result r1.  Hence we can say that
-    /// @f[ r1 = lm1(r0, d1) @f]
-    ///
-    /// Callout lm_two is passed a value d2 and peforms another simple
-    /// arithmetic operation on it and d2, yielding r2, i.e.
-    /// @f[ r2 = lm2(d1, d2) @f]
-    ///
-    /// lm_three does a similar operation giving @f[ r3 = lm3(r2, d3) @f].
-    ///
-    /// The details of the operations lm1, lm2 and lm3 depend on the library.
-    /// However the sequence of calls needed to do this set of calculations
-    /// is identical regardless of the exact functions. This method performs
-    /// those operations and checks the results of each step.
-    ///
-    /// It is assumed that callout_manager_ has been set up appropriately.
-    ///
-    /// @note The CalloutHandle used in the calls is declared locally here.
-    ///       The advantage of this (apart from scope reduction) is that on
-    ///       exit, it is destroyed.  This removes any references to memory
-    ///       allocated by loaded libraries while they are still loaded.
     ///
     /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
     ///        are ordered so that the variables appear in the argument list in
-    ///        the order they are used.
+    ///        the order they are used.  See HooksCommonTestClass::execute for
+    ///        a full description. (rN is used to indicate an expected result,
+    ///        dN is data to be passed to the calculation.)
     void executeCallCallouts(int r0, int d1, int r1, int d2, int r2, int d3,
                              int r3) {
-        static const char* COMMON_TEXT = " callout returned the wong value";
+        HooksCommonTestClass::executeCallCallouts(callout_manager_, r0, d1,
-        int result;
+                                                  r1, d2, r2, d3, r3);
-
-        // Set up a callout handle for the calls.
-        CalloutHandle callout_handle(callout_manager_);
-
-        // Seed the calculation.
-        callout_manager_->callCallouts(ServerHooks::CONTEXT_CREATE,
-                                       callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
-
-        // Perform the first calculation.
-        callout_handle.setArgument("data_1", d1);
-        callout_manager_->callCallouts(lm_one_index_, callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r1, result) << "lm_one" << COMMON_TEXT;
-
-        // ... the second ...
-        callout_handle.setArgument("data_2", d2);
-        callout_manager_->callCallouts(lm_two_index_, callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r2, result) << "lm_two" << COMMON_TEXT;
-
-        // ... and the third.
-        callout_handle.setArgument("data_3", d3);
-        callout_manager_->callCallouts(lm_three_index_, callout_handle);
-        callout_handle.getArgument("result", result);
-        EXPECT_EQ(r3, result) << "lm_three" << COMMON_TEXT;
     }
 
-    /// Hook indexes.  These are are made public for ease of reference.
-    int lm_one_index_;
-    int lm_two_index_;
-    int lm_three_index_;
-
     /// Callout manager used for the test.
     boost::shared_ptr<CalloutManager> callout_manager_;
 };
 
-
 /// @brief Library manager class
 ///
 /// This is an instance of the LibraryManager class but with the protected
@@ -174,20 +131,6 @@ public:
     using LibraryManager::runUnload;
 };
 
-// Names of the libraries used in these tests.  These libraries are built using
-// libtool, so we need to look in the hidden ".libs" directory to locate the
-// .so file.  Note that we access the .so file - libtool creates this as a
-// like to the real shared library.
-static const char* BASIC_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libbcl.so";
-static const char* FULL_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libfcl.so";
-static const char* INCORRECT_VERSION_LIBRARY = "@abs_builddir@/.libs/libivl.so";
-static const char* LOAD_CALLOUT_LIBRARY = "@abs_builddir@/.libs/liblcl.so";
-static const char* LOAD_ERROR_CALLOUT_LIBRARY =
-    "@abs_builddir@/.libs/liblecl.so";
-static const char* NOT_PRESENT_LIBRARY = "@abs_builddir@/.libs/libnothere.so";
-static const char* NO_VERSION_LIBRARY = "@abs_builddir@/.libs/libnvl.so";
-static const char* UNLOAD_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libucl.so";
-
 
 // Check that openLibrary() reports an error when it can't find the specified
 // library.
@@ -244,6 +187,22 @@ TEST_F(LibraryManagerTest, WrongVersion) {
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
 
+// Check that the code handles the case of a library where the version function
+// throws an exception.
+
+TEST_F(LibraryManagerTest, VersionException) {
+    PublicLibraryManager lib_manager(std::string(FRAMEWORK_EXCEPTION_LIBRARY),
+                                     0, callout_manager_);
+    // Open should succeed.
+    EXPECT_TRUE(lib_manager.openLibrary());
+
+    // Version check should fail.
+    EXPECT_FALSE(lib_manager.checkVersion());
+
+    // Tidy up.
+    EXPECT_TRUE(lib_manager.closeLibrary());
+}
+
 // Tests that checkVersion() function succeeds in the case of a library with a
 // version function that returns the correct version number.
 
@@ -302,12 +261,12 @@ TEST_F(LibraryManagerTest, CheckLoadCalled) {
     // Load the standard callouts
     EXPECT_NO_THROW(lib_manager.registerStandardCallouts());
 
-    // Check that only context_create and lm_one have callouts registered.
+    // Check that only context_create and hookpt_one have callouts registered.
     EXPECT_TRUE(callout_manager_->calloutsPresent(
                 ServerHooks::CONTEXT_CREATE));
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_three_index_));
     EXPECT_FALSE(callout_manager_->calloutsPresent(
                  ServerHooks::CONTEXT_DESTROY));
 
@@ -315,9 +274,9 @@ TEST_F(LibraryManagerTest, CheckLoadCalled) {
     EXPECT_TRUE(lib_manager.runLoad());
     EXPECT_TRUE(callout_manager_->calloutsPresent(
                 ServerHooks::CONTEXT_CREATE));
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_three_index_));
     EXPECT_FALSE(callout_manager_->calloutsPresent(
                  ServerHooks::CONTEXT_DESTROY));
 
@@ -331,6 +290,23 @@ TEST_F(LibraryManagerTest, CheckLoadCalled) {
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
 
+// Check handling of a "load" function that throws an exception
+
+TEST_F(LibraryManagerTest, CheckLoadException) {
+
+    // Load the only library, specifying the index of 0 as it's the only
+    // library.  This should load all callouts.
+    PublicLibraryManager lib_manager(std::string(FRAMEWORK_EXCEPTION_LIBRARY),
+                                     0, callout_manager_);
+    EXPECT_TRUE(lib_manager.openLibrary());
+
+    // Running the load function should fail.
+    EXPECT_FALSE(lib_manager.runLoad());
+
+    // Tidy up
+    EXPECT_TRUE(lib_manager.closeLibrary());
+}
+
 // Check handling of a "load" function that returns an error.
 
 TEST_F(LibraryManagerTest, CheckLoadError) {
@@ -382,6 +358,23 @@ TEST_F(LibraryManagerTest, CheckUnloadError) {
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
 
+// Unload function throws an exception.
+
+TEST_F(LibraryManagerTest, CheckUnloadException) {
+
+    // Load the only library, specifying the index of 0 as it's the only
+    // library.  This should load all callouts.
+    PublicLibraryManager lib_manager(std::string(FRAMEWORK_EXCEPTION_LIBRARY),
+                                     0, callout_manager_);
+    EXPECT_TRUE(lib_manager.openLibrary());
+
+    // Check that we detect that the unload function throws an exception.
+    EXPECT_FALSE(lib_manager.runUnload());
+
+    // Tidy up
+    EXPECT_TRUE(lib_manager.closeLibrary());
+}
+
 // Check that the case of the library's unload() function returning a
 // success is handled correcty.
 
@@ -393,21 +386,16 @@ TEST_F(LibraryManagerTest, CheckUnload) {
                                      0, callout_manager_);
     EXPECT_TRUE(lib_manager.openLibrary());
 
+
     // Check that the marker file is not present (at least that the file
     // open fails).
-    fstream marker;
+    EXPECT_FALSE(markerFilePresent());
-    marker.open(MARKER_FILE, fstream::in);
-    EXPECT_TRUE(marker.fail());
 
     // Check that unload function runs and returns a success
     EXPECT_TRUE(lib_manager.runUnload());
 
-    // Check that the open succeeded
+    // Check that the marker file was created.
-    marker.open(MARKER_FILE, fstream::in);
+    EXPECT_TRUE(markerFilePresent());
-    EXPECT_TRUE(marker.is_open());
-
-    // Tidy up
-    marker.close();
 
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
@@ -430,33 +418,33 @@ TEST_F(LibraryManagerTest, LibUnload) {
     EXPECT_TRUE(lib_manager.checkVersion());
 
     // No callouts should be registered at the moment.
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_three_index_));
 
     // Load the single standard callout and check it is registered correctly.
     EXPECT_NO_THROW(lib_manager.registerStandardCallouts());
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_three_index_));
 
     // Call the load function to load the other callouts.
     EXPECT_TRUE(lib_manager.runLoad());
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_TRUE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_TRUE(callout_manager_->calloutsPresent(hookpt_three_index_));
 
     // Unload the library and check that the callouts have been removed from
     // the CalloutManager.
     lib_manager.unloadLibrary();
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_three_index_));
 }
 
 // Now come the loadLibrary() tests that make use of all the methods tested
 // above.  These tests are really to make sure that the methods have been
-// tied toget correctly.
+// tied together correctly.
 
 // First test the basic error cases - no library, no version function, version
 // function returning an error.
@@ -500,9 +488,9 @@ TEST_F(LibraryManagerTest, LoadLibrary) {
     EXPECT_TRUE(lib_manager.unloadLibrary());
 
     // Check that the callouts have been removed from the callout manager.
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_one_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_one_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_two_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_two_index_));
-    EXPECT_FALSE(callout_manager_->calloutsPresent(lm_three_index_));
+    EXPECT_FALSE(callout_manager_->calloutsPresent(hookpt_three_index_));
 }
 
 // Now test for multiple libraries.  We'll load the full callout library
@@ -542,7 +530,7 @@ TEST_F(LibraryManagerTest, LoadMultipleLibraries) {
     // Execute the callouts.  The first library implements the calculation.
     //
     // r3 = (7 * d1 - d2) * d3
-    // 
+    //
     // The last-loaded library implements the calculation
     //
     // r3 = (10 + d1) * d2 - d3

src/lib/hooks/tests/load_callout_library.cc

@@ -20,9 +20,9 @@
 /// file are:
 ///
 /// - The "version" and "load" framework functions are supplied.  One "standard"
-///   callout is supplied ("lm_one") and two non-standard ones which are
+///   callout is supplied ("hookpt_one") and two non-standard ones which are
-///   registered during the call to "load" on the hooks "lm_two" and
+///   registered during the call to "load" on the hooks "hookpt_two" and
-///   "lm_three". 
+///   "hookpt_three".
 ///
 ///   All callouts do trivial calculations, the result of all being called in
 ///   sequence being
@@ -30,8 +30,8 @@
 ///   @f[ ((5 * data_1) + data_2) * data_3 @f]
 ///
 ///   ...where data_1, data_2 and data_3 are the values passed in arguments of
-///   the same name to the three callouts (data_1 passed to lm_one, data_2 to
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
-///   lm_two etc.) and the result is returned in the argument "result".
+///   hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 
@@ -54,7 +54,7 @@ context_create(CalloutHandle& handle) {
 // between callouts in the same library.)
 
 int
-lm_one(CalloutHandle& handle) {
+hookpt_one(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_1", data);
 
@@ -71,7 +71,7 @@ lm_one(CalloutHandle& handle) {
 // argument.
 
 static int
-lm_nonstandard_two(CalloutHandle& handle) {
+hook_nonstandard_two(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_2", data);
 
@@ -87,7 +87,7 @@ lm_nonstandard_two(CalloutHandle& handle) {
 // Final callout adds "data_3" to the result.
 
 static int
-lm_nonstandard_three(CalloutHandle& handle) {
+hook_nonstandard_three(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_3", data);
 
@@ -109,8 +109,8 @@ version() {
 
 int load(LibraryHandle& handle) {
     // Register the non-standard functions
-    handle.registerCallout("lm_two", lm_nonstandard_two);
+    handle.registerCallout("hookpt_two", hook_nonstandard_two);
-    handle.registerCallout("lm_three", lm_nonstandard_three);
+    handle.registerCallout("hookpt_three", hook_nonstandard_three);
 
     return (0);
 }

src/lib/hooks/tests/server_hooks_unittest.cc

@@ -175,118 +175,4 @@ TEST(ServerHooksTest, HookCount) {
     EXPECT_EQ(6, hooks.getCount());
 }
 
-// HookRegistrationFunction tests
-
-// Declare some hook registration functions.
-
-int alpha = 0;
-int beta = 0;
-int gamma = 0;
-int delta = 0;
-
-void registerAlphaBeta(ServerHooks& hooks) {
-    alpha = hooks.registerHook("alpha");
-    beta = hooks.registerHook("beta");
-}
-
-void registerGammaDelta(ServerHooks& hooks) {
-    gamma = hooks.registerHook("gamma");
-    delta = hooks.registerHook("delta");
-}
-
-// Add them to the registration vector.  This addition should happen before
-// any tests are run, so we should start off with two functions in the
-// registration vector.
-
-HookRegistrationFunction f1(registerAlphaBeta);
-HookRegistrationFunction f2(registerGammaDelta);
-
-// This is not registered statically: it is used in the latter part of the
-// test.
-
-int epsilon = 0;
-void registerEpsilon(ServerHooks& hooks) {
-    epsilon = hooks.registerHook("epsilon");
-}
-
-// Test that the registration functions were defined and can be executed.
-
-TEST(HookRegistrationFunction, Registration) {
-
-    // The first part of the tests checks the static registration.  As there
-    // is only one list of registration functions, we have to do this first
-    // as the static registration is done outside our control, before the
-    // tests are loaded.
-
-    // Ensure that the hook numbers are initialized.
-    EXPECT_EQ(0, alpha);
-    EXPECT_EQ(0, beta);
-    EXPECT_EQ(0, gamma);
-    EXPECT_EQ(0, delta);
-
-    // Should have two hook registration functions registered.
-    EXPECT_EQ(2, HookRegistrationFunction::getFunctionVector().size());
-
-    // Execute the functions and check that four new hooks were defined (two
-    // from each function).
-    ServerHooks& hooks = ServerHooks::getServerHooks();
-    hooks.reset();
-
-    EXPECT_EQ(2, hooks.getCount());
-    HookRegistrationFunction::execute(hooks);
-    EXPECT_EQ(6, hooks.getCount());
-
-    // Check the hook names are as expected.
-    vector<string> names = hooks.getHookNames();
-    ASSERT_EQ(6, names.size());
-    sort(names.begin(), names.end());
-    EXPECT_EQ(string("alpha"), names[0]);
-    EXPECT_EQ(string("beta"), names[1]);
-    EXPECT_EQ(string("context_create"), names[2]);
-    EXPECT_EQ(string("context_destroy"), names[3]);
-    EXPECT_EQ(string("delta"), names[4]);
-    EXPECT_EQ(string("gamma"), names[5]);
-
-    // Check that numbers in the range 2-5 inclusive were assigned as the
-    // hook indexes (0 and 1 being reserved for context_create and
-    // context_destroy).
-    vector<int> indexes;
-    indexes.push_back(alpha);
-    indexes.push_back(beta);
-    indexes.push_back(gamma);
-    indexes.push_back(delta);
-    sort(indexes.begin(), indexes.end());
-    EXPECT_EQ(2, indexes[0]);
-    EXPECT_EQ(3, indexes[1]);
-    EXPECT_EQ(4, indexes[2]);
-    EXPECT_EQ(5, indexes[3]);
-
-    // One last check.  We'll test that the constructor of does indeed
-    // add a function to the function vector and that the static initialization
-    // was not somehow by chance.
-    HookRegistrationFunction::getFunctionVector().clear();
-    EXPECT_TRUE(HookRegistrationFunction::getFunctionVector().empty());
-    epsilon = 0;
-
-    // Register a single registration function.
-    HookRegistrationFunction f3(registerEpsilon);
-    EXPECT_EQ(1, HookRegistrationFunction::getFunctionVector().size());
-
-    // Execute it...
-    hooks.reset();
-    EXPECT_EQ(0, epsilon);
-    EXPECT_EQ(2, hooks.getCount());
-    HookRegistrationFunction::execute(hooks);
-
-    // There should be three hooks, with the new one assigned an index of 2.
-    names = hooks.getHookNames();
-    ASSERT_EQ(3, names.size());
-    sort(names.begin(), names.end());
-    EXPECT_EQ(string("context_create"), names[0]);
-    EXPECT_EQ(string("context_destroy"), names[1]);
-    EXPECT_EQ(string("epsilon"), names[2]);
-
-    EXPECT_EQ(2, epsilon);
-}
-
 } // Anonymous namespace

src/lib/hooks/tests/test_libraries.h.in

@@ -0,0 +1,79 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef TEST_LIBRARIES_H
+#define TEST_LIBRARIES_H
+
+#include <config.h>
+
+namespace {
+
+
+// Take care of differences in DLL naming between operating systems.
+
+#ifdef OS_BSD
+#define DLL_SUFFIX ".dylib"
+
+#else
+#define DLL_SUFFIX ".so"
+
+#endif
+
+
+// Names of the libraries used in these tests.  These libraries are built using
+// libtool, so we need to look in the hidden ".libs" directory to locate the
+// .so file.  Note that we access the .so file - libtool creates this as a
+// like to the real shared library.
+
+// Basic library with context_create and three "standard" callouts.
+static const char* BASIC_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libbcl"
+                                           DLL_SUFFIX;
+
+// Library with context_create and three "standard" callouts, as well as
+// load() and unload() functions.
+static const char* FULL_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libfcl"
+                                          DLL_SUFFIX;
+
+// Library where the all framework functions throw an exception
+static const char* FRAMEWORK_EXCEPTION_LIBRARY = "@abs_builddir@/.libs/libfxl"
+                                                 DLL_SUFFIX;
+
+// Library where the version() function returns an incorrect result.
+static const char* INCORRECT_VERSION_LIBRARY = "@abs_builddir@/.libs/libivl"
+                                               DLL_SUFFIX;
+
+// Library where some of the callout registration is done with the load()
+// function.
+static const char* LOAD_CALLOUT_LIBRARY = "@abs_builddir@/.libs/liblcl"
+                                          DLL_SUFFIX;
+
+// Library where the load() function returns an error.
+static const char* LOAD_ERROR_CALLOUT_LIBRARY =
+    "@abs_builddir@/.libs/liblecl" DLL_SUFFIX;
+
+// Name of a library which is not present.
+static const char* NOT_PRESENT_LIBRARY = "@abs_builddir@/.libs/libnothere"
+                                         DLL_SUFFIX;
+
+// Library that does not include a version function.
+static const char* NO_VERSION_LIBRARY = "@abs_builddir@/.libs/libnvl"
+                                        DLL_SUFFIX;
+
+// Library where there is an unload() function.
+static const char* UNLOAD_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libucl"
+                                            DLL_SUFFIX;
+} // anonymous namespace
+
+
+#endif // TEST_LIBRARIES_H