Merge branch 'master' into trac2266

ChangeLog

@@ -1,3 +1,30 @@
+488.	[build]		jinmei
+	On configure, changed the search order for Python executable.
+	It first ties more specific file names such as "python3.2" before
+	more generic "python3".  This will prevent configure failure on
+	Mac OS X that installs Python3 via recent versions of Homebrew.
+	(Trac #2339, git 88db890d8d1c64de49be87f03c24a2021bcf63da)
+
+487.	[bug]		jinmei
+	The bind10 process now terminates a component (subprocess) by the
+	"config remove Boss/components" bindctl command even if the
+	process crashes immediately before the command is sent to bind10.
+	Previously this led to an inconsistent state between the
+	configuration and an internal component list of bind10, and bind10
+	kept trying to restart the component.  A known specific case of
+	this problem is that b10-ddns could keep failing (due to lack of
+	dependency modules) and the administrator couldn't stop the
+	restart via bindctl.
+	(Trac #2244, git 7565788d06f216ab254008ffdfae16678bcd00e5)
+
+486.	[bug]*		jinmei
+	All public header files for libb10-dns++ are now installed.
+	Template configure.ac and utility AC macros for external projects
+	using the library are provided under the "examples" directory.
+	The src/bin/host was moved as part of the examples (and not
+	installed with other BIND 10 programs any more).
+	(Trac #1870, git 4973e638d354d8b56dcadf71123ef23c15662021)
+
 485.	[bug]		jelte
 	Several bugs have been fixed in bindctl; tab-completion now works
 	within configuration lists, the problem where sometimes the

configure.ac

@@ -4,7 +4,7 @@
 AC_PREREQ([2.59])
 AC_INIT(bind10-devel, 20120817, bind10-dev@isc.org)
 AC_CONFIG_SRCDIR(README)
-AM_INIT_AUTOMAKE
+AM_INIT_AUTOMAKE([foreign])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])dnl be backward compatible
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIR([m4macros])
@@ -132,7 +132,7 @@ AM_CONDITIONAL(SET_ENV_LIBRARY_PATH, test $SET_ENV_LIBRARY_PATH = yes)
 AC_SUBST(SET_ENV_LIBRARY_PATH)
 AC_SUBST(ENV_LIBRARY_PATH)
 
-m4_define([_AM_PYTHON_INTERPRETER_LIST], [python python3 python3.1 python3.2])
+m4_define([_AM_PYTHON_INTERPRETER_LIST], [python python3.2 python3.1 python3])
 AC_ARG_WITH([pythonpath],
 AC_HELP_STRING([--with-pythonpath=PATH],
   [specify an absolute path to python executable when automatic version check (incorrectly) fails]),
@@ -1099,7 +1099,6 @@ AC_CONFIG_FILES([Makefile
                  src/bin/dbutil/Makefile
                  src/bin/dbutil/tests/Makefile
                  src/bin/dbutil/tests/testdata/Makefile
-                 src/bin/host/Makefile
                  src/bin/loadzone/Makefile
                  src/bin/loadzone/tests/correct/Makefile
                  src/bin/loadzone/tests/error/Makefile

examples/COPYING

@@ -0,0 +1,13 @@
+Copyright (C) 2012  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.

examples/INSTALL

@@ -0,0 +1,9 @@
+If using git (not the tarball), build the "configure" file:
+    autoreconf --install
+
+To then build from source:
+    ./configure
+    make
+
+You may have to specify the location of BIND 10 header files and
+library objects.  See configure options by ./configure --help.

examples/Makefile.am

@@ -0,0 +1,4 @@
+SUBDIRS = host
+
+# Make sure macros under m4 will be included
+ACLOCAL_AMFLAGS = -I m4

examples/README

@@ -0,0 +1,32 @@
+This is the top directory for sample programs that can be developed
+using public BIND 10 libraries outside of the BIND 10 project.  It's
+intended to be built with installed BIND 10 header files and library
+objects, so it's not a target of the main build tree, and does not
+refer to any other part of the BIND 10 source tree that contains
+this directory.
+
+On the top (sub) directory (where this README file is stored), we
+provide a sample configure.ac and Makefile.am files for GNU automake
+environments with helper autoconf macros to detect the availability and
+location of BIND 10 header files and library objects.
+
+You can use the configure.ac and Makefile.am files with macros under
+the "m4" subdirectory as a template for your own project.  The key is
+to call the AX_ISC_BIND10 function (as the sample configure.ac does)
+from your configure.ac.  Then it will check the availability of
+necessary stuff and set some corresponding AC variables.  You can then
+use the resulting variables in your Makefile.in or Makefile.ac.
+
+If you use automake, don't forget adding the following line to the top
+level Makefile.am:
+
+ACLOCAL_AMFLAGS = -I m4
+
+This is necessary to incorporate the helper macro definitions.
+
+If you don't use automake but autoconf, make sure to add the following
+to the configure.ac file:
+
+sinclude(m4/ax_boost_include.m4)
+sinclude(m4/ax_isc_bind10.m4)
+(and same for other m4 files as they are added under m4/)

examples/configure.ac

@@ -0,0 +1,28 @@
+#                                               -*- Autoconf -*-
+# Process this file with autoconf to produce a configure script.
+
+AC_PREREQ([2.59])
+AC_INIT(bind10-examples, 20120817, bind10-dev@isc.org)
+AC_CONFIG_SRCDIR([README])
+AM_INIT_AUTOMAKE
+AC_CONFIG_HEADERS([config.h])
+
+# Checks for programs.
+AC_PROG_CXX
+AC_LANG([C++])
+
+# Checks for BIND 10 headers and libraries
+AX_ISC_BIND10
+
+# For the example host program, we require the BIND 10 DNS library
+if test "x$BIND10_DNS_LIB" = "x"; then
+   AC_MSG_ERROR([unable to find BIND 10 DNS library needed to build 'host'])
+fi
+
+# Checks for typedefs, structures, and compiler characteristics.
+AC_HEADER_STDBOOL
+
+AC_CONFIG_FILES([Makefile
+                 host/Makefile])
+
+AC_OUTPUT

examples/host/Makefile.am

@@ -0,0 +1,6 @@
+AM_CPPFLAGS = $(BOOST_CPPFLAGS) $(BIND10_CPPFLAGS)
+
+bin_PROGRAMS = b10-host
+b10_host_SOURCES = host.cc
+b10_host_LDFLAGS = ${BIND10_LDFLAGS}
+b10_host_LDADD = ${BIND10_DNS_LIB}

examples/m4/ax_boost_include.m4

@@ -0,0 +1,64 @@
+dnl @synopsis AX_BOOST_INCLUDE
+dnl
+dnl Test for the Boost C++ header files
+dnl
+dnl If no path to the installed boost header files is given via the
+dnl --with-boost-include option,  the macro searchs under
+dnl /usr/local /usr/pkg /opt /opt/local directories.
+dnl
+dnl This macro calls:
+dnl
+dnl   AC_SUBST(BOOST_CPPFLAGS)
+dnl
+
+AC_DEFUN([AX_BOOST_INCLUDE], [
+AC_LANG_SAVE
+AC_LANG([C++])
+
+#
+# Configure Boost header path
+#
+# If explicitly specified, use it.
+AC_ARG_WITH([boost-include],
+  AS_HELP_STRING([--with-boost-include=PATH],
+    [specify exact directory for Boost headers]),
+    [boost_include_path="$withval"])
+# If not specified, try some common paths.
+if test -z "$with_boost_include"; then
+	boostdirs="/usr/local /usr/pkg /opt /opt/local"
+	for d in $boostdirs
+	do
+		if test -f $d/include/boost/shared_ptr.hpp; then
+			boost_include_path=$d/include
+			break
+		fi
+	done
+fi
+CPPFLAGS_SAVES="$CPPFLAGS"
+if test "${boost_include_path}" ; then
+	BOOST_CPPFLAGS="-I${boost_include_path}"
+	CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS"
+fi
+# Make sure some commonly used headers are available
+AC_CHECK_HEADERS([boost/shared_ptr.hpp boost/bind.hpp boost/function.hpp],,
+  AC_MSG_ERROR([Missing required Boost header files.]))
+
+# Detect whether Boost tries to use threads by default, and, if not,
+# make it sure explicitly.  In some systems the automatic detection
+# may depend on preceding header files, and if inconsistency happens
+# it could lead to a critical disruption.
+AC_MSG_CHECKING([whether Boost tries to use threads])
+AC_TRY_COMPILE([
+#include <boost/config.hpp>
+#ifdef BOOST_HAS_THREADS
+#error "boost will use threads"
+#endif],,
+[AC_MSG_RESULT(no)
+ CPPFLAGS_BOOST_THREADCONF="-DBOOST_DISABLE_THREADS=1"],
+[AC_MSG_RESULT(yes)])
+
+CPPFLAGS="$CPPFLAGS_SAVES $CPPFLAGS_BOOST_THREADCONF"
+AC_SUBST(BOOST_CPPFLAGS)
+
+AC_LANG_RESTORE
+])dnl AX_BOOST_INCLUDE

examples/m4/ax_isc_bind10.m4

@@ -0,0 +1,122 @@
+dnl @synopsis AX_BIND10
+dnl
+dnl @summary figure out how to build C++ programs using ISC BIND 10 libraries
+dnl
+dnl If no path to the installed BIND 10 header files or libraries is given
+dnl via the --with-bind10-include  or --with-bind10-lib option, the macro
+dnl searchs under /usr/local/{include, lib}, /usr/pkg/{include, lib},
+dnl /opt/{include, lib}, /opt/local/{include, lib} directories, respectively.
+dnl
+dnl This macro calls:
+dnl
+dnl   AC_SUBST(BIND10_CPPFLAGS)
+dnl   AC_SUBST(BIND10_LDFLAGS)
+dnl   AC_SUBST(BIND10_COMMON_LIB)
+dnl   AC_SUBST(BIND10_DNS_LIB)
+dnl
+dnl If this macro finds CPPFLAGS, LDFLAGS or COMMON_LIB unavailable, it treats
+dnl that as a fatal error.
+dnl Checks for other BIND 10 module libraries are option, as not all
+dnl applications need all libraries.  The main configure.ac can handle any
+dnl missing library as fatal by checking whether the corresponding
+dnl BIND10_xxx_LIB is defined.
+
+AC_DEFUN([AX_ISC_BIND10], [
+AC_REQUIRE([AX_BOOST_INCLUDE])
+AC_LANG_SAVE
+AC_LANG([C++])
+
+# Check for BIND10 common headers
+
+AC_ARG_WITH(bind10-include,
+  AS_HELP_STRING([--with-bind10-include=PATH],
+  [specify a path to BIND 10 header files]),
+    bind10_inc_path="$withval", bind10_inc_path="no")
+# If not specified, try some common paths.
+if test "$bind10_inc_path" = "no"; then
+   for d in /usr/local /usr/pkg /opt /opt/local
+   do
+	if test -f $d/include/util/buffer.h; then
+	   bind10_inc_path=$d
+	   break
+	fi
+   done
+fi
+CPPFLAGS_SAVES="$CPPFLAGS"
+if test "${bind10_inc_path}" != "no"; then
+   BIND10_CPPFLAGS="-I${bind10_inc_path}"
+   CPPFLAGS="$CPPFLAGS $BIND10_CPPFLAGS"
+fi
+AC_CHECK_HEADERS([util/buffer.h],,
+  AC_MSG_ERROR([Missing a commonly used BIND 10 header files]))
+CPPFLAGS="$CPPFLAGS_SAVES"
+AC_SUBST(BIND10_CPPFLAGS)
+
+# Check for BIND10 libraries
+CPPFLAGS_SAVED="$CPPFLAGS"
+CPPFLAGS="$CPPFLAGS $BIND10_CPPFLAGS"
+
+AC_ARG_WITH(bind10-lib,
+  AS_HELP_STRING([--with-bind10-lib=PATH],
+  [specify a path to BIND 10 library files]),
+    bind10_lib_path="$withval", bind10_lib_path="no")
+if test $bind10_lib_path != "no"; then
+   bind10_lib_dirs=$bind10_lib_path
+else
+   # If not specified, try some common paths.
+   bind10_lib_dirs="/usr/local/lib /usr/pkg/lib /opt/lib /opt/local/lib"
+fi
+
+# make sure we have buildable libraries
+AC_MSG_CHECKING([for BIND 10 common library])
+BIND10_COMMON_LIB="-lb10-util -lb10-exceptions"
+LDFLAGS="$LDFLAGS $BIND10_LDFLAGS"
+LIBS="$LIBS $BIND10_COMMON_LIB"
+for d in $bind10_lib_dirs
+do
+  LDFLAGS_SAVED="$LDFLAGS"
+  LDFLAGS="$LDFLAGS -L$d"
+  AC_TRY_LINK([
+#include <util/buffer.h>
+],[
+isc::util::OutputBuffer buffer(0);
+], [BIND10_LDFLAGS="-L${d}"])
+  if test "x$BIND10_LDFLAGS" != "x"; then
+     break
+  fi
+  LDFLAGS="$LDFLAGS_SAVED"
+done
+if test "x$BIND10_LDFLAGS" != "x"; then
+  AC_MSG_RESULT(yes)
+else
+  AC_MSG_RESULT(no)
+  AC_MSG_ERROR([unable to find required BIND 10 libraries])
+fi
+
+# restore LIBS once at this point
+LIBS="$LIBS_SAVES"
+
+AC_SUBST(BIND10_LDFLAGS)
+AC_SUBST(BIND10_COMMON_LIB)
+
+# Check per-module BIND 10 libraries
+
+# DNS library
+AC_MSG_CHECKING([for BIND 10 DNS library])
+LIBS="$LIBS $BIND10_COMMON_LIB -lb10-dns++"
+AC_TRY_LINK([
+#include <dns/rrtype.h>
+],[
+isc::dns::RRType rrtype(1);
+], [BIND10_DNS_LIB="-lb10-dns++"
+    AC_MSG_RESULT(yes)],
+   [AC_MSG_RESULT(no)])
+LIBS="$LIBS_SAVES"
+AC_SUBST(BIND10_DNS_LIB)
+
+# Restore other flags
+CPPFLAGS="$CPPFLAGS_SAVED"
+LDFLAGS="$LDFLAGS_SAVES"
+
+AC_LANG_RESTORE
+])dnl AX_ISC_BIND10

src/bin/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = bind10 bindctl cfgmgr ddns loadzone msgq host cmdctl auth xfrin \
+SUBDIRS = bind10 bindctl cfgmgr ddns loadzone msgq cmdctl auth xfrin \
 	xfrout usermgr zonemgr stats tests resolver sockcreator dhcp4 dhcp6 \
 	dbutil sysinfo
 

src/bin/auth/Makefile.am

@@ -55,7 +55,7 @@ b10_auth_SOURCES += auth_config.cc auth_config.h
 b10_auth_SOURCES += command.cc command.h
 b10_auth_SOURCES += common.h common.cc
 b10_auth_SOURCES += statistics.cc statistics.h
-b10_auth_SOURCES += datasrc_configurator.h
+b10_auth_SOURCES += datasrc_config.h datasrc_config.cc
 b10_auth_SOURCES += main.cc
 
 nodist_b10_auth_SOURCES = auth_messages.h auth_messages.cc

src/bin/auth/auth_srv.cc

@@ -69,6 +69,8 @@
 
 using namespace std;
 
+using boost::shared_ptr;
+
 using namespace isc;
 using namespace isc::cc;
 using namespace isc::datasrc;
@@ -264,23 +266,22 @@ public:
     AddressList listen_addresses_;
 
     /// The TSIG keyring
-    const boost::shared_ptr<TSIGKeyRing>* keyring_;
+    const shared_ptr<TSIGKeyRing>* keyring_;
 
-    /// The client list
-    std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >
-        client_lists_;
+    /// The data source client list
+    AuthSrv::DataSrcClientListsPtr datasrc_client_lists_;
 
-    boost::shared_ptr<ConfigurableClientList> getClientList(const RRClass&
-                                                            rrclass)
+    shared_ptr<ConfigurableClientList> getDataSrcClientList(
+        const RRClass& rrclass)
     {
         // TODO: Debug-build only check
         if (!mutex_.locked()) {
             isc_throw(isc::Unexpected, "Not locked!");
         }
-        const std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >::
-            const_iterator it(client_lists_.find(rrclass));
-        if (it == client_lists_.end()) {
-            return (boost::shared_ptr<ConfigurableClientList>());
+        const std::map<RRClass, shared_ptr<ConfigurableClientList> >::
+            const_iterator it(datasrc_client_lists_->find(rrclass));
+        if (it == datasrc_client_lists_->end()) {
+            return (shared_ptr<ConfigurableClientList>());
         } else {
             return (it->second);
         }
@@ -335,6 +336,8 @@ AuthSrvImpl::AuthSrvImpl(AbstractXfroutClient& xfrout_client,
     xfrin_session_(NULL),
     counters_(),
     keyring_(NULL),
+    datasrc_client_lists_(new std::map<RRClass,
+                          shared_ptr<ConfigurableClientList> >()),
     ddns_base_forwarder_(ddns_forwarder),
     ddns_forwarder_(NULL),
     xfrout_connected_(false),
@@ -645,13 +648,13 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
     }
     // Lock the client lists and keep them under the lock until the processing
     // and rendering is done (this is the same mutex as from
-    // AuthSrv::getClientListMutex()).
+    // AuthSrv::getDataSrcClientListMutex()).
     isc::util::thread::Mutex::Locker locker(mutex_);
 
     try {
         const ConstQuestionPtr question = *message.beginQuestion();
-        const boost::shared_ptr<datasrc::ClientList>
-            list(getClientList(question->getClass()));
+        const shared_ptr<datasrc::ClientList>
+            list(getDataSrcClientList(question->getClass()));
         if (list) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
@@ -911,7 +914,7 @@ AuthSrv::setDNSService(isc::asiodns::DNSServiceBase& dnss) {
 }
 
 void
-AuthSrv::setTSIGKeyRing(const boost::shared_ptr<TSIGKeyRing>* keyring) {
+AuthSrv::setTSIGKeyRing(const shared_ptr<TSIGKeyRing>* keyring) {
     impl_->keyring_ = keyring;
 }
 
@@ -930,43 +933,23 @@ AuthSrv::destroyDDNSForwarder() {
     }
 }
 
-void
-AuthSrv::setClientList(const RRClass& rrclass,
-                       const boost::shared_ptr<ConfigurableClientList>& list) {
+AuthSrv::DataSrcClientListsPtr
+AuthSrv::swapDataSrcClientLists(DataSrcClientListsPtr new_lists) {
     // TODO: Debug-build only check
     if (!impl_->mutex_.locked()) {
-        isc_throw(isc::Unexpected, "Not locked");
-    }
-
-    if (list) {
-        impl_->client_lists_[rrclass] = list;
-    } else {
-        impl_->client_lists_.erase(rrclass);
+        isc_throw(isc::Unexpected, "Not locked!");
     }
-}
-boost::shared_ptr<ConfigurableClientList>
-AuthSrv::getClientList(const RRClass& rrclass) {
-    return (impl_->getClientList(rrclass));
+    std::swap(new_lists, impl_->datasrc_client_lists_);
+    return (new_lists);
 }
 
-vector<RRClass>
-AuthSrv::getClientListClasses() const {
-    // TODO: Debug-build only check
-    if (!impl_->mutex_.locked()) {
-        isc_throw(isc::Unexpected, "Not locked");
-    }
-
-    vector<RRClass> result;
-    for (std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >::
-         const_iterator it(impl_->client_lists_.begin());
-         it != impl_->client_lists_.end(); ++it) {
-        result.push_back(it->first);
-    }
-    return (result);
+shared_ptr<ConfigurableClientList>
+AuthSrv::getDataSrcClientList(const RRClass& rrclass) {
+    return (impl_->getDataSrcClientList(rrclass));
 }
 
 util::thread::Mutex&
-AuthSrv::getClientListMutex() const {
+AuthSrv::getDataSrcClientListMutex() const {
     return (impl_->mutex_);
 }
 

src/bin/auth/auth_srv.h

@@ -15,10 +15,11 @@
 #ifndef __AUTH_SRV_H
 #define __AUTH_SRV_H 1
 
-#include <string>
-
 #include <config/ccsession.h>
+
 #include <datasrc/factory.h>
+#include <datasrc/client_list.h>
+
 #include <dns/message.h>
 #include <dns/opcode.h>
 #include <util/buffer.h>
@@ -35,6 +36,11 @@
 #include <server_common/portconfig.h>
 #include <auth/statistics.h>
 
+#include <boost/shared_ptr.hpp>
+
+#include <map>
+#include <string>
+
 namespace isc {
 namespace util {
 namespace io {
@@ -296,31 +302,46 @@ public:
     /// If there was no forwarder yet, this method does nothing.
     void destroyDDNSForwarder();
 
-    /// \brief Sets the currently used list for data sources of given
-    ///     class.
+    /// \brief Shortcut typedef used for swapDataSrcClientLists().
+    typedef boost::shared_ptr<std::map<
+        isc::dns::RRClass, boost::shared_ptr<
+                               isc::datasrc::ConfigurableClientList> > >
+    DataSrcClientListsPtr;
+
+    /// \brief Swap the currently used set of data source client lists with
+    /// given one.
+    ///
+    /// The "set" of lists is actually given in the form of map from
+    /// RRClasses to shared pointers to isc::datasrc::ConfigurableClientList.
+    ///
+    /// This method returns the swapped set of lists, which was previously
+    /// used by the server.
+    ///
+    /// This method is intended to be used by a separate method to update
+    /// the data source configuration "at once".  The caller must hold
+    /// a lock for the mutex object returned by  \c getDataSrcClientListMutex()
+    /// before calling this method.
     ///
-    /// Replaces the internally used client list with a new one. Other
-    /// classes are not changed.
+    /// The ownership of the returned pointer is transferred to the caller.
+    /// The caller is generally expected to release the resources used in
+    /// the old lists.  Note that it could take longer time if some of the
+    /// data source clients contain a large size of in-memory data.
     ///
-    /// \param rrclass The class to modify.
-    /// \param list Shared pointer to the client list. If it is NULL,
-    ///     the list is removed instead.
-    void setClientList(const isc::dns::RRClass& rrclass, const
-                       boost::shared_ptr<isc::datasrc::ConfigurableClientList>&
-                       list);
+    /// The caller can pass a NULL pointer.  This effectively disables
+    /// any data source for the server.
+    ///
+    /// \param new_lists Shared pointer to a new set of data source client
+    /// lists.
+    /// \return The previous set of lists.  It can be NULL.
+    DataSrcClientListsPtr swapDataSrcClientLists(DataSrcClientListsPtr
+                                                 new_lists);
 
     /// \brief Returns the currently used client list for the class.
     ///
     /// \param rrclass The class for which to get the list.
     /// \return The list, or NULL if no list is set for the class.
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        getClientList(const isc::dns::RRClass& rrclass);
-
-    /// \brief Returns a list of classes that have a client list.
-    ///
-    /// \return List of classes for which a non-NULL client list
-    ///     has been set by setClientList.
-    std::vector<isc::dns::RRClass> getClientListClasses() const;
+        getDataSrcClientList(const isc::dns::RRClass& rrclass);
 
     /// \brief Return a mutex for the client lists.
     ///
@@ -331,9 +352,9 @@ public:
     /// is correct:
     /// \code
     /// {
-    ///  Mutex::Locker locker(auth->getClientListMutex());
+    ///  Mutex::Locker locker(auth->getDataSrcClientListMutex());
     ///  boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-    ///    list(auth->getClientList(RRClass::IN()));
+    ///    list(auth->getDataSrcClientList(RRClass::IN()));
     ///  // Do some processing here
     /// }
     /// \endcode
@@ -342,8 +363,8 @@ public:
     /// \code
     /// boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
     /// {
-    ///     Mutex::Locker locker(auth->getClientListMutex());
-    ///     list = auth->getClientList(RRClass::IN()));
+    ///     Mutex::Locker locker(auth->getDataSrcClientListMutex());
+    ///     list = auth->getDataSrcClientList(RRClass::IN()));
     /// }
     /// // Do some processing here
     /// \endcode
@@ -352,7 +373,7 @@ public:
     ///    (lock) the mutex. It's because locking of the mutex is not really
     ///    a modification of the server object and it is needed to protect the
     ///    lists even on read-only operations.
-    isc::util::thread::Mutex& getClientListMutex() const;
+    isc::util::thread::Mutex& getDataSrcClientListMutex() const;
 
     /// \brief Sets the timeout for incoming TCP connections
     ///

src/bin/auth/benchmarks/Makefile.am

@@ -17,6 +17,7 @@ query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
 query_bench_SOURCES += ../auth_config.h ../auth_config.cc
 query_bench_SOURCES += ../statistics.h ../statistics.cc
 query_bench_SOURCES += ../auth_log.h ../auth_log.cc
+query_bench_SOURCES += ../datasrc_config.h ../datasrc_config.cc
 
 nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc
 

src/bin/auth/benchmarks/query_bench.cc

@@ -18,6 +18,8 @@
 #include <bench/benchmark_util.h>
 
 #include <util/buffer.h>
+#include <util/threads/lock.h>
+
 #include <dns/message.h>
 #include <dns/name.h>
 #include <dns/question.h>
@@ -30,7 +32,7 @@
 
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 #include <auth/query.h>
 
 #include <asiodns/asiodns.h>
@@ -125,13 +127,15 @@ public:
                           OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        DataSourceConfigurator::testReconfigure(
-            server_.get(),
-            Element::fromJSON("{\"IN\":"
-                              "  [{\"type\": \"sqlite3\","
-                              "    \"params\": {"
-                              "      \"database_file\": \"" +
-                              string(datasrc_file) + "\"}}]}"));
+        isc::util::thread::Mutex::Locker locker(
+                server_->getDataSrcClientListMutex());
+        server_->swapDataSrcClientLists(
+            configureDataSource(
+                Element::fromJSON("{\"IN\":"
+                                  "  [{\"type\": \"sqlite3\","
+                                  "    \"params\": {"
+                                  "      \"database_file\": \"" +
+                                  string(datasrc_file) + "\"}}]}")));
     }
 };
 
@@ -139,19 +143,21 @@ class MemoryQueryBenchMark  : public QueryBenchMark {
 public:
     MemoryQueryBenchMark(const char* const zone_file,
                          const char* const zone_origin,
-                          const BenchQueries& queries,
-                          Message& query_message,
-                          OutputBuffer& buffer) :
+                         const BenchQueries& queries,
+                         Message& query_message,
+                         OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        DataSourceConfigurator::testReconfigure(
-            server_.get(),
-            Element::fromJSON("{\"IN\":"
-                              "  [{\"type\": \"MasterFiles\","
-                              "    \"cache-enable\": true, "
-                              "    \"params\": {\"" +
-                              string(zone_origin) + "\": \"" +
-                              string(zone_file) + "\"}}]}"));
+        isc::util::thread::Mutex::Locker locker(
+                server_->getDataSrcClientListMutex());
+        server_->swapDataSrcClientLists(
+            configureDataSource(
+                Element::fromJSON("{\"IN\":"
+                                  "  [{\"type\": \"MasterFiles\","
+                                  "    \"cache-enable\": true, "
+                                  "    \"params\": {\"" +
+                                  string(zone_origin) + "\": \"" +
+                                  string(zone_file) + "\"}}]}")));
     }
 };
 

src/bin/auth/command.cc

@@ -192,9 +192,10 @@ public:
 
         // We're going to work with the client lists. They may be used
         // from a different thread too, protect them.
-        isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server.getDataSrcClientListMutex());
         const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(server.getClientList(zone_class));
+            list(server.getDataSrcClientList(zone_class));
 
         if (!list) {
             isc_throw(AuthCommandError, "There's no client list for "

src/bin/auth/datasrc_config.cc

@@ -0,0 +1,25 @@
+// Copyright (C) 2012  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 <cc/data.h>
+#include "auth_srv.h"
+#include "datasrc_config.h"
+
+// This is a trivial specialization for the commonly used version.
+// Defined in .cc to avoid accidental creation of multiple copies.
+AuthSrv::DataSrcClientListsPtr
+configureDataSource(const isc::data::ConstElementPtr& config) {
+    return (configureDataSourceGeneric<
+            isc::datasrc::ConfigurableClientList>(config));
+}

src/bin/auth/datasrc_config.h

@@ -0,0 +1,83 @@
+// Copyright (C) 2012  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 DATASRC_CONFIG_H
+#define DATASRC_CONFIG_H
+
+#include "auth_srv.h"
+
+#include <cc/data.h>
+#include <datasrc/client_list.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <utility>
+#include <set>
+
+/// \brief Configure data source client lists
+///
+/// This will hook into the data_sources module configuration and it will
+/// return a new set (in the form of a shared pointer to map) of data source
+/// client lists corresponding to the configuration.
+///
+/// This function is templated. This is simply because of easier testing.
+/// You don't need to pay attention to it, use the configureDataSource
+/// specialization instead.
+///
+/// \note In future we may want to make the reconfiguration more efficient
+/// by only creating newly configured data and just moving the rest from
+/// the running configuration if they are used in the new configuration
+/// without any parameter change.  We could probably do it by passing
+/// the old lists in addition to the new config, but further details are
+/// still to be defined yet.  It will surely require changes in the
+/// data source library, too.  So, right now, we don't introduce the
+/// possibility in the function interface.  If and when we decide to introduce
+/// the optimization, we'll extend the interface.
+///
+/// \param config The configuration value to parse. It is in the form
+///     as an update from the config manager.
+/// \return A map from RR classes to configured lists.
+template<class List>
+boost::shared_ptr<std::map<isc::dns::RRClass,
+                           boost::shared_ptr<List> > > // = ListMap below
+configureDataSourceGeneric(const isc::data::ConstElementPtr& config) {
+    typedef boost::shared_ptr<List> ListPtr;
+    typedef std::map<std::string, isc::data::ConstElementPtr> Map;
+    typedef std::map<isc::dns::RRClass, ListPtr> ListMap;
+
+    boost::shared_ptr<ListMap> new_lists(new ListMap);
+
+    // Go through the configuration and create corresponding list.
+    const Map& map(config->mapValue());
+    for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
+        const isc::dns::RRClass rrclass(it->first);
+        ListPtr list(new List(rrclass));
+        list->configure(it->second, true);
+        new_lists->insert(std::pair<isc::dns::RRClass, ListPtr>(rrclass,
+                                                                list));
+    }
+
+    return (new_lists);
+}
+
+/// \brief Concrete version of configureDataSource() for the
+///     use with authoritative server implementation.
+AuthSrv::DataSrcClientListsPtr
+configureDataSource(const isc::data::ConstElementPtr& config);
+
+#endif  // DATASRC_CONFIG_H
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/datasrc_configurator.h

@@ -1,226 +0,0 @@
-// Copyright (C) 2012  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 DATASRC_CONFIGURATOR_H
-#define DATASRC_CONFIGURATOR_H
-
-#include "auth_srv.h"
-
-#include <datasrc/client_list.h>
-#include <config/ccsession.h>
-#include <cc/data.h>
-#include <util/threads/lock.h>
-
-#include <set>
-
-/// \brief A class to configure the authoritative server's data source lists
-///
-/// This will hook into the data_sources module configuration and it will
-/// keep the local copy of data source clients in the list in the authoritative
-/// server.
-///
-/// The class is slightly unusual. Due to some technical limitations, the hook
-/// needs to be static method. Therefore it is not possible to create instances
-/// of the class.
-///
-/// Also, the class is a template. This is simply because of easier testing.
-/// You don't need to pay attention to it, use the DataSourceConfigurator
-/// type alias instead.
-template<class Server, class List>
-class DataSourceConfiguratorGeneric {
-private:
-    /// \brief Disallow creation of instances
-    DataSourceConfiguratorGeneric();
-    /// \brief Internal method to hook into the ModuleCCSession
-    ///
-    /// It simply calls reconfigure.
-    static void reconfigureInternal(const std::string&,
-                                    isc::data::ConstElementPtr config,
-                                    const isc::config::ConfigData&)
-    {
-        if (config->contains("classes")) {
-            reconfigure(config->get("classes"));
-        }
-    }
-    static Server* server_;
-    static isc::config::ModuleCCSession* session_;
-    typedef boost::shared_ptr<List> ListPtr;
-public:
-    /// \brief Initializes the class.
-    ///
-    /// This configures which session and server should be used.
-    /// It hooks to the session now and downloads the configuration.
-    /// It is synchronous (it may block for some time).
-    ///
-    /// Note that you need to call cleanup before the server or
-    /// session dies, otherwise it might access them after they
-    /// are destroyed.
-    ///
-    /// \param session The session to hook into and to access the configuration
-    ///     through.
-    /// \param server It is the server to configure.
-    /// \throw isc::InvalidOperation if this is called when already initialized.
-    /// \throw isc::InvalidParameter if any of the parameters is NULL
-    /// \throw isc::config::ModuleCCError if the remote configuration is not
-    ///     available for some reason.
-    static void init(isc::config::ModuleCCSession* session,
-                     Server* server)
-    {
-        if (session == NULL) {
-            isc_throw(isc::InvalidParameter, "The session must not be NULL");
-        }
-        if (server == NULL) {
-            isc_throw(isc::InvalidParameter, "The server must not be NULL");
-        }
-        if (server_ != NULL) {
-            isc_throw(isc::InvalidOperation,
-                      "The configurator is already initialized");
-        }
-        server_ = server;
-        session_ = session;
-        session->addRemoteConfig("data_sources", reconfigureInternal, false);
-    }
-    /// \brief Deinitializes the class.
-    ///
-    /// This detaches from the session and removes the server from internal
-    /// storage. The current configuration in the server is preserved.
-    ///
-    /// This can be called even if it is not initialized currently. You
-    /// can initialize it again after this.
-    static void cleanup() {
-        if (session_ != NULL) {
-            session_->removeRemoteConfig("data_sources");
-        }
-        session_ = NULL;
-        server_ = NULL;
-    }
-    /// \brief Reads new configuration and replaces the old one.
-    ///
-    /// It instructs the server to replace the lists with new ones as needed.
-    /// You don't need to call it directly (but you could, though the benefit
-    /// is unknown and it would be questionable at least). It is called
-    /// automatically on normal updates.
-    ///
-    /// \param config The configuration value to parse. It is in the form
-    ///     as an update from the config manager.
-    /// \throw InvalidOperation if it is called when not initialized.
-    static void reconfigure(const isc::data::ConstElementPtr& config) {
-        if (server_ == NULL) {
-            isc_throw(isc::InvalidOperation,
-                      "Can't reconfigure while not initialized by init()");
-        }
-        // Lock the client lists, we're going to manipulate them.
-        isc::util::thread::Mutex::Locker locker(server_->getClientListMutex());
-        typedef std::map<std::string, isc::data::ConstElementPtr> Map;
-        typedef std::pair<isc::dns::RRClass, ListPtr> RollbackPair;
-        typedef std::pair<isc::dns::RRClass, isc::data::ConstElementPtr>
-            RollbackConfiguration;
-        // Some structures to be able to perform a rollback
-        std::vector<RollbackPair> rollback_sets;
-        std::vector<RollbackConfiguration> rollback_configurations;
-        try {
-            // Get the configuration and current state.
-            const Map& map(config->mapValue());
-            const std::vector<isc::dns::RRClass>
-                activeVector(server_->getClientListClasses());
-            std::set<isc::dns::RRClass> active(activeVector.begin(),
-                                               activeVector.end());
-            // Go through the configuration and change everything.
-            for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
-                isc::dns::RRClass rrclass(it->first);
-                active.erase(rrclass);
-                ListPtr list(server_->getClientList(rrclass));
-                bool need_set(false);
-                if (list) {
-                    rollback_configurations.
-                        push_back(RollbackConfiguration(rrclass,
-                            list->getConfiguration()));
-                } else {
-                    list.reset(new List(rrclass));
-                    need_set = true;
-                    rollback_sets.push_back(RollbackPair(rrclass, ListPtr()));
-                }
-                list->configure(it->second, true);
-                if (need_set) {
-                    server_->setClientList(rrclass, list);
-                }
-            }
-            // Remove the ones that are not in the configuration.
-            for (std::set<isc::dns::RRClass>::iterator it(active.begin());
-                 it != active.end(); ++it) {
-                // There seems to be no way the setClientList could throw.
-                // But this is just to make sure in case it did to restore
-                // the original.
-                rollback_sets.push_back(
-                    RollbackPair(*it, server_->getClientList(*it)));
-                server_->setClientList(*it, ListPtr());
-            }
-        } catch (...) {
-            // Perform a rollback of the changes. The old configuration should
-            // work.
-            for (typename std::vector<RollbackPair>::const_iterator
-                 it(rollback_sets.begin()); it != rollback_sets.end(); ++it) {
-                server_->setClientList(it->first, it->second);
-            }
-            for (typename std::vector<RollbackConfiguration>::const_iterator
-                 it(rollback_configurations.begin());
-                 it != rollback_configurations.end(); ++it) {
-                server_->getClientList(it->first)->configure(it->second, true);
-            }
-            throw;
-        }
-    }
-    /// \brief Version of reconfigure for easier testing.
-    ///
-    /// This method can be used to reconfigure a server without first
-    /// initializing the configurator. This does not need a session.
-    /// Otherwise, it acts the same as reconfigure.
-    ///
-    /// This is not meant for production code. Do not use there.
-    ///
-    /// \param server The server to configure.
-    /// \param config The config to use.
-    /// \throw isc::InvalidOperation if the configurator is initialized.
-    /// \throw anything that reconfigure does.
-    static void testReconfigure(Server* server,
-                                const isc::data::ConstElementPtr& config)
-    {
-        if (server_ != NULL) {
-            isc_throw(isc::InvalidOperation, "Currently initialized.");
-        }
-        try {
-            server_ = server;
-            reconfigure(config);
-            server_ = NULL;
-        } catch (...) {
-            server_ = NULL;
-            throw;
-        }
-    }
-};
-
-template<class Server, class List>
-isc::config::ModuleCCSession*
-DataSourceConfiguratorGeneric<Server, List>::session_(NULL);
-
-template<class Server, class List>
-Server* DataSourceConfiguratorGeneric<Server, List>::server_(NULL);
-
-/// \brief Concrete version of DataSourceConfiguratorGeneric for the
-///     use in authoritative server.
-typedef DataSourceConfiguratorGeneric<AuthSrv,
-        isc::datasrc::ConfigurableClientList>
-    DataSourceConfigurator;
-
-#endif

src/bin/auth/main.cc

@@ -14,21 +14,11 @@
 
 #include <config.h>
 
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/select.h>
-#include <netdb.h>
-#include <netinet/in.h>
-#include <stdlib.h>
-#include <errno.h>
-
-#include <cassert>
-#include <iostream>
-
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
 #include <util/io/socketsession.h>
+#include <util/threads/lock.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -45,13 +35,26 @@
 #include <auth/command.h>
 #include <auth/auth_srv.h>
 #include <auth/auth_log.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
 #include <log/logger_support.h>
 #include <server_common/keyring.h>
 #include <server_common/socket_request.h>
 
+#include <boost/bind.hpp>
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/select.h>
+#include <netdb.h>
+#include <netinet/in.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include <cassert>
+#include <iostream>
+
 using namespace std;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
@@ -70,7 +73,7 @@ namespace {
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
-AuthSrv *auth_server;
+AuthSrv* auth_server;
 
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
@@ -84,6 +87,42 @@ my_command_handler(const string& command, ConstElementPtr args) {
 }
 
 void
+datasrcConfigHandler(AuthSrv* server, bool* first_time,
+                     ModuleCCSession* config_session, const std::string&,
+                     isc::data::ConstElementPtr config,
+                     const isc::config::ConfigData&)
+{
+    assert(server != NULL);
+    if (config->contains("classes")) {
+        AuthSrv::DataSrcClientListsPtr lists;
+
+        if (*first_time) {
+            // HACK: The default is not passed to the handler in the first
+            // callback. This one will get the default (or, current value).
+            // Further updates will work the usual way.
+            assert(config_session != NULL);
+            *first_time = false;
+            lists = configureDataSource(
+                config_session->getRemoteConfigValue("data_sources",
+                                                     "classes"));
+        } else {
+            lists = configureDataSource(config->get("classes"));
+        }
+
+        // Replace the server's lists.  The returned lists will be stored
+        // in a local variable 'lists', and will be destroyed outside of
+        // the temporary block for the lock scope.  That way we can minimize
+        // the range of the critical section.
+        {
+            isc::util::thread::Mutex::Locker locker(
+                server->getDataSrcClientListMutex());
+            lists = server->swapDataSrcClientLists(lists);
+        }
+        // The previous lists are destroyed here.
+    }
+}
+
+void
 usage() {
     cerr << "Usage:  b10-auth [-v]"
          << endl;
@@ -191,13 +230,15 @@ main(int argc, char* argv[]) {
         isc::server_common::initKeyring(*config_session);
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
 
-        // Start the data source configuration
-        DataSourceConfigurator::init(config_session, auth_server);
-        // HACK: The default is not passed to the handler. This one will
-        // get the default (or, current value). Further updates will work
-        // the usual way.
-        DataSourceConfigurator::reconfigure(
-            config_session->getRemoteConfigValue("data_sources", "classes"));
+        // Start the data source configuration.  We pass first_time and
+        // config_session for the hack described in datasrcConfigHandler.
+        bool first_time = true;
+        config_session->addRemoteConfig("data_sources",
+                                        boost::bind(datasrcConfigHandler,
+                                                    auth_server, &first_time,
+                                                    config_session,
+                                                    _1, _2, _3),
+                                        false);
 
         // Now start asynchronous read.
         config_session->start();
@@ -221,7 +262,10 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
     }
 
-    DataSourceConfigurator::cleanup();
+    // If we haven't registered callback for data sources, this will be just
+    // no-op.
+    config_session->removeRemoteConfig("data_sources");
+
     delete xfrin_session;
     delete config_session;
     delete cc_session;

src/bin/auth/tests/Makefile.am

@@ -42,6 +42,7 @@ run_unittests_SOURCES += ../auth_config.h ../auth_config.cc
 run_unittests_SOURCES += ../command.h ../command.cc
 run_unittests_SOURCES += ../common.h ../common.cc
 run_unittests_SOURCES += ../statistics.h ../statistics.cc
+run_unittests_SOURCES += ../datasrc_config.h ../datasrc_config.cc
 run_unittests_SOURCES += datasrc_util.h datasrc_util.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
 run_unittests_SOURCES += config_unittest.cc
@@ -50,7 +51,7 @@ run_unittests_SOURCES += command_unittest.cc
 run_unittests_SOURCES += common_unittest.cc
 run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += statistics_unittest.cc
-run_unittests_SOURCES += datasrc_configurator_unittest.cc
+run_unittests_SOURCES += datasrc_config_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 
 nodist_run_unittests_SOURCES = ../auth_messages.h ../auth_messages.cc

src/bin/auth/tests/auth_srv_unittest.cc

@@ -36,7 +36,7 @@
 #include <auth/command.h>
 #include <auth/common.h>
 #include <auth/statistics.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <util/unittests/mock_socketsession.h>
 #include <util/threads/lock.h>
@@ -63,6 +63,7 @@
 using namespace std;
 using namespace isc::cc;
 using namespace isc::dns;
+using namespace isc::datasrc;
 using namespace isc::util;
 using namespace isc::util::io::internal;
 using namespace isc::util::unittests;
@@ -90,6 +91,9 @@ const char* const STATIC_DSRC_FILE = DSRC_DIR "/static.zone";
 // a signed example zone.
 const char* const CONFIG_INMEMORY_EXAMPLE = TEST_DATA_DIR "/rfc5155-example.zone.signed";
 
+// shortcut commonly used in tests
+typedef boost::shared_ptr<ConfigurableClientList> ListPtr;
+
 class AuthSrvTest : public SrvTestBase {
 protected:
     AuthSrvTest() :
@@ -722,17 +726,25 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 
 void
-updateDatabase(AuthSrv* server, const char* params) {
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    thread::Mutex::Locker locker(server.getDataSrcClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
+void
+updateDatabase(AuthSrv& server, const char* params) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 }
 
 void
-updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
+updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "   \"type\": \"MasterFiles\","
@@ -745,17 +757,17 @@ updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 }
 
 void
-updateBuiltin(AuthSrv* server) {
+updateBuiltin(AuthSrv& server) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"CH\": [{"
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -766,7 +778,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin
 TEST_F(AuthSrvTest, TSIGSigned) {
 #endif
     // Prepare key, the client message, etc
-    updateBuiltin(&server);
+    updateBuiltin(server);
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
     TSIGContext context(key);
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
@@ -814,7 +826,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) {
 #else
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -846,7 +858,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) {
 #else
 TEST_F(AuthSrvTest, builtInQuery) {
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -867,7 +879,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin
 #else
 TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     createDataFromFile("iquery_fromWire.wire");
     (*server.getDNSLookupProvider())(*io_message, parse_message,
                                      response_message,
@@ -889,7 +901,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) {
 #else
 TEST_F(AuthSrvTest, updateConfig) {
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // query for existent data in the installed data source.  The resulting
     // response should have the AA flag on, and have an RR in each answer
@@ -907,7 +919,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) {
 #else
 TEST_F(AuthSrvTest, datasourceFail) {
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // This query will hit a corrupted entry of the data source (the zoneload
     // tool and the data source itself naively accept it).  This will result
@@ -927,10 +939,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) {
 TEST_F(AuthSrvTest, updateConfigFail) {
 #endif
     // First, load a valid data source.
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // Next, try to update it with a non-existent one.  This should fail.
-    EXPECT_THROW(updateDatabase(&server, BADCONFIG_TESTDB),
+    EXPECT_THROW(updateDatabase(server, BADCONFIG_TESTDB),
                  isc::datasrc::DataSourceError);
 
     // The original data source should still exist.
@@ -953,7 +965,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
     // after successful configuration, we should have one (with empty zoneset).
 
     // The memory data source is empty, should return REFUSED rcode.
@@ -974,7 +986,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
     // query handler class, and confirm it returns no error and a non empty
     // answer section.  Detailed examination on the response content
     // for various types of queries are tested in the query tests.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -993,7 +1005,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
     // Similar to the previous test, but the query has the DO bit on.
     // The response should contain RRSIGs, and should have more RRs than
     // the previous case.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1013,7 +1025,7 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set up the in-memory
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     // This shouldn't affect the result of class CH query
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
@@ -1425,13 +1437,16 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set real inmem client to proxy
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
     {
-        isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server.getDataSrcClientListMutex());
         boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
-                              false));
-        server.setClientList(RRClass::IN(), list);
+            list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
+                              THROW_NEVER, false));
+        AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
+        lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+        server.swapDataSrcClientLists(lists);
     }
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
@@ -1450,16 +1465,19 @@ TEST_F(AuthSrvTest,
 // If non null rrset is given, it will be passed to the proxy so it can
 // return some faked response.
 void
-setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception,
+setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
            ConstRRsetPtr rrset = ConstRRsetPtr())
 {
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
-    isc::util::thread::Mutex::Locker locker(server->getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new FakeList(server->getClientList(RRClass::IN()), throw_when,
-                          isc_exception, rrset));
-    server->setClientList(RRClass::IN(), list);
+        list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
+                          throw_when, isc_exception, rrset));
+    AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    server.swapDataSrcClientLists(lists);
 }
 
 TEST_F(AuthSrvTest,
@@ -1482,11 +1500,11 @@ TEST_F(AuthSrvTest,
                                              RRClass::IN(), RRType::TXT());
     for (ThrowWhen* when(throws); *when != THROW_NEVER; ++when) {
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, true);
+        setupThrow(server, *when, true);
         processAndCheckSERVFAIL();
         // To be sure, check same for non-isc-exceptions
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, false);
+        setupThrow(server, *when, false);
         processAndCheckSERVFAIL();
     }
 }
@@ -1502,7 +1520,7 @@ TEST_F(AuthSrvTest,
     )
 {
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
-    setupThrow(&server, THROW_AT_GET_CLASS, true);
+    setupThrow(server, THROW_AT_GET_CLASS, true);
 
     // getClass is not called so it should just answer
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1526,7 +1544,7 @@ TEST_F(AuthSrvTest,
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),
                                         RRClass::IN(), RRType::TXT(),
                                         RRTTL(0)));
-    setupThrow(&server, THROW_NEVER, true, empty_rrset);
+    setupThrow(server, THROW_NEVER, true, empty_rrset);
 
     // Repeat the query processing two times.  Due to the faked RRset,
     // toWire() should throw, and it should result in SERVFAIL.
@@ -1771,46 +1789,51 @@ TEST_F(AuthSrvTest, clientList) {
     // We need to lock the mutex to make the (get|set)ClientList happy.
     // There's a debug-build only check in them to make sure everything
     // locks them and we call them directly here.
-    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+
+    AuthSrv::DataSrcClientListsPtr lists; // initially empty
+
     // The lists don't exist. Therefore, the list of RRClasses is empty.
-    // We also have no IN list.
-    EXPECT_TRUE(server.getClientListClasses().empty());
-    EXPECT_EQ(boost::shared_ptr<const isc::datasrc::ClientList>(),
-              server.getClientList(RRClass::IN()));
+    EXPECT_TRUE(server.swapDataSrcClientLists(lists)->empty());
+
     // Put something in.
-    const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new isc::datasrc::ConfigurableClientList(RRClass::IN()));
-    const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list2(new isc::datasrc::ConfigurableClientList(RRClass::CH()));
-    server.setClientList(RRClass::IN(), list);
-    server.setClientList(RRClass::CH(), list2);
-    // There are two things in the list and they are IN and CH
-    vector<RRClass> classes(server.getClientListClasses());
-    ASSERT_EQ(2, classes.size());
-    EXPECT_EQ(RRClass::IN(), classes[0]);
-    EXPECT_EQ(RRClass::CH(), classes[1]);
+    const ListPtr list(new ConfigurableClientList(RRClass::IN()));
+    const ListPtr list2(new ConfigurableClientList(RRClass::CH()));
+
+    lists.reset(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    lists->insert(pair<RRClass, ListPtr>(RRClass::CH(), list2));
+    server.swapDataSrcClientLists(lists);
+
     // And the lists can be retrieved.
-    EXPECT_EQ(list, server.getClientList(RRClass::IN()));
-    EXPECT_EQ(list2, server.getClientList(RRClass::CH()));
-    // Remove one of them
-    server.setClientList(RRClass::CH(),
-        boost::shared_ptr<isc::datasrc::ConfigurableClientList>());
-    // This really got deleted, including the class.
-    classes = server.getClientListClasses();
-    ASSERT_EQ(1, classes.size());
-    EXPECT_EQ(RRClass::IN(), classes[0]);
-    EXPECT_EQ(list, server.getClientList(RRClass::IN()));
+    EXPECT_EQ(list, server.getDataSrcClientList(RRClass::IN()));
+    EXPECT_EQ(list2, server.getDataSrcClientList(RRClass::CH()));
+
+    // Replace the lists with new lists containing only one list.
+    lists.reset(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    lists = server.swapDataSrcClientLists(lists);
+
+    // Old one had two lists.  That confirms our swap for IN and CH classes
+    // (i.e., no other entries were there).
+    EXPECT_EQ(2, lists->size());
+
+    // The CH list really got deleted.
+    EXPECT_EQ(list, server.getDataSrcClientList(RRClass::IN()));
+    EXPECT_FALSE(server.getDataSrcClientList(RRClass::CH()));
 }
 
 // We just test the mutex can be locked (exactly once).
 TEST_F(AuthSrvTest, mutex) {
-    isc::util::thread::Mutex::Locker l1(server.getClientListMutex());
+    isc::util::thread::Mutex::Locker l1(server.getDataSrcClientListMutex());
     // TODO: Once we have non-debug build, this one will not work, since
     // we currently use the fact that we can't lock twice from the same
     // thread. In the non-debug mode, this would deadlock.
     // Skip then.
     EXPECT_THROW({
-        isc::util::thread::Mutex::Locker l2(server.getClientListMutex());
+        isc::util::thread::Mutex::Locker l2(
+            server.getDataSrcClientListMutex());
     }, isc::InvalidOperation);
 }
 

src/bin/auth/tests/command_unittest.cc

@@ -16,10 +16,12 @@
 
 #include "datasrc_util.h"
 
+#include <util/threads/lock.h>
+
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
 #include <auth/command.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
@@ -174,22 +176,32 @@ TEST_F(AuthCommandTest, shutdownIncorrectPID) {
 // zones, and checks the zones are correctly loaded.
 void
 zoneChecks(AuthSrv& server) {
-    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::AAAA())->code);
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
 
 void
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
+void
 configureZones(AuthSrv& server) {
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
@@ -208,27 +220,29 @@ configureZones(AuthSrv& server) {
         "   \"cache-enable\": true"
         "}]}"));
 
-    DataSourceConfigurator::testReconfigure(&server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 
     zoneChecks(server);
 }
 
 void
 newZoneChecks(AuthSrv& server) {
-    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::A())->code);
     // now test1.example should have ns/AAAA
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::AAAA())->code);
 
     // test2.example shouldn't change
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::NXRRSET,
+              server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
@@ -271,12 +285,14 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server_, config);
+    installDataSrcClientLists(server_, configureDataSource(config));
 
     {
-        isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server_.getDataSrcClientListMutex());
         // Check that the A record at www.example.org does not exist
-        EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::NXDOMAIN,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
 
@@ -296,7 +312,8 @@ TEST_F(AuthCommandTest,
         sql_updater->addRRset(*rrset);
         sql_updater->commit();
 
-        EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::NXDOMAIN,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
     }
@@ -308,9 +325,11 @@ TEST_F(AuthCommandTest,
     checkAnswer(0, "Successful load");
 
     {
-        isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server_.getDataSrcClientListMutex());
         // And now it should be present too.
-        EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::SUCCESS,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
     }
@@ -321,9 +340,11 @@ TEST_F(AuthCommandTest,
     checkAnswer(1, "example.com");
 
     {
-        isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server_.getDataSrcClientListMutex());
         // The previous zone is not hurt in any way
-        EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::SUCCESS,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("example.org"), RRType::SOA())->code);
     }
@@ -335,16 +356,18 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.com\"]"
         "}]}"));
-    EXPECT_THROW(DataSourceConfigurator::testReconfigure(&server_, config2),
+    EXPECT_THROW(configureDataSource(config2),
                  ConfigurableClientList::ConfigurationError);
 
     result_ = execAuthServerCommand(server_, "loadzone",
         Element::fromJSON("{\"origin\": \"example.com\"}"));
     checkAnswer(1, "Unreadable");
 
-    isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(
+        server_.getDataSrcClientListMutex());
     // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS,
+              server_.getDataSrcClientList(RRClass::IN())->
               find(Name("example.org")).finder_->
               find(Name("example.org"), RRType::SOA())->code);
 }

src/bin/auth/tests/datasrc_configurator_unittest.cc

@@ -12,16 +12,19 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <config/tests/fake_session.h>
 #include <config/ccsession.h>
 #include <util/threads/lock.h>
 
 #include <gtest/gtest.h>
-#include <memory>
+
+#include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 
+#include <memory>
+
 using namespace isc;
 using namespace isc::cc;
 using namespace isc::config;
@@ -32,7 +35,7 @@ using namespace boost;
 
 namespace {
 
-class DatasrcConfiguratorTest;
+class DatasrcConfigTest;
 
 class FakeList {
 public:
@@ -57,36 +60,50 @@ private:
 
 typedef shared_ptr<FakeList> ListPtr;
 
-// We use the test fixture as both parameters, this makes it possible
-// to easily fake all needed methods and look that they were called.
-typedef DataSourceConfiguratorGeneric<DatasrcConfiguratorTest,
-        FakeList> Configurator;
+// Forward declaration.  We need precise definition of DatasrcConfigTest
+// to complete this function.
+void
+testConfigureDataSource(DatasrcConfigTest& test,
+                        const isc::data::ConstElementPtr& config);
 
-class DatasrcConfiguratorTest : public ::testing::Test {
+void
+datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
+                     isc::data::ConstElementPtr config,
+                     const isc::config::ConfigData&)
+{
+    if (config->contains("classes")) {
+        testConfigureDataSource(*fake_server, config->get("classes"));
+    }
+}
+
+class DatasrcConfigTest : public ::testing::Test {
 public:
     // These pretend to be the server
-    ListPtr getClientList(const RRClass& rrclass) {
-        log_ += "get " + rrclass.toText() + "\n";
-        return (lists_[rrclass]);
-    }
-    void setClientList(const RRClass& rrclass, const ListPtr& list) {
-        log_ += "set " + rrclass.toText() + " " +
-            (list ? list->getConf() : "") + "\n";
-        lists_[rrclass] = list;
+    isc::util::thread::Mutex& getDataSrcClientListMutex() const {
+        return (mutex_);
     }
-    vector<RRClass> getClientListClasses() const {
-        vector<RRClass> result;
-        for (std::map<RRClass, ListPtr>::const_iterator it(lists_.begin());
-             it != lists_.end(); ++it) {
-            result.push_back(it->first);
+    void swapDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
+                                new_lists)
+    {
+        lists_.clear();         // first empty it
+
+        // Record the operation and results.  Note that map elements are
+        // sorted by RRClass, so the ordering should be predictable.
+        for (std::map<dns::RRClass, ListPtr>::const_iterator it =
+                 new_lists->begin();
+             it != new_lists->end();
+             ++it)
+        {
+            const RRClass rrclass = it->first;
+            ListPtr list = it->second;
+            log_ += "set " + rrclass.toText() + " " +
+                (list ? list->getConf() : "") + "\n";
+            lists_[rrclass] = list;
         }
-        return (result);
-    }
-    isc::util::thread::Mutex& getClientListMutex() const {
-        return (mutex_);
     }
+
 protected:
-    DatasrcConfiguratorTest() :
+    DatasrcConfigTest() :
         session(ElementPtr(new ListElement), ElementPtr(new ListElement),
                 ElementPtr(new ListElement)),
         specfile(string(TEST_OWN_DATA_DIR) + "/spec.spec")
@@ -99,25 +116,24 @@ protected:
                                        false));
     }
     void TearDown() {
-        // Make sure no matter what we did, it is cleaned up.
-        Configurator::cleanup();
+        // Make sure no matter what we did, it is cleaned up.  Also check
+        // we really have subscribed to the configuration, and after removing
+        // it we actually cancel it.
+        EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
+        mccs->removeRemoteConfig("data_sources");
+        EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
     }
-    void init(const ElementPtr& config = ElementPtr()) {
+    void SetUp() {
         session.getMessages()->
             add(createAnswer(0,
                              moduleSpecFromFile(string(PLUGIN_DATA_PATH) +
                                                 "/datasrc.spec").
                              getFullSpec()));
-        if (config) {
-            session.getMessages()->add(createAnswer(0, config));
-        } else {
-            session.getMessages()->
-                add(createAnswer(0, ElementPtr(new MapElement)));
-        }
-        Configurator::init(mccs.get(), this);
-    }
-    void SetUp() {
-        init();
+        session.getMessages()->add(createAnswer(0,
+                                                ElementPtr(new MapElement)));
+        mccs->addRemoteConfig("data_sources",
+                              boost::bind(datasrcConfigHandler,
+                                          this, _1, _2, _3), false);
     }
     ElementPtr buildConfig(const string& config) const {
         const ElementPtr internal(Element::fromJSON(config));
@@ -126,14 +142,13 @@ protected:
         return (external);
     }
     void initializeINList() {
-        const ElementPtr
+        const ConstElementPtr
             config(buildConfig("{\"IN\": [{\"type\": \"xxx\"}]}"));
-        session.addMessage(createCommand("config_update", config), "data_sources",
-                           "*");
+        session.addMessage(createCommand("config_update", config),
+                           "data_sources", "*");
         mccs->checkCommand();
-        // Check it called the correct things (check that there's no IN yet and
-        // set a new one.
-        EXPECT_EQ("get IN\nset IN xxx\n", log_);
+        // Check that the passed config is stored.
+        EXPECT_EQ("set IN xxx\n", log_);
         EXPECT_EQ(1, lists_.size());
     }
     FakeSession session;
@@ -144,35 +159,27 @@ protected:
     mutable isc::util::thread::Mutex mutex_;
 };
 
-// Check the initialization (and cleanup)
-TEST_F(DatasrcConfiguratorTest, initialization) {
-    // It can't be initialized again
-    EXPECT_THROW(init(), InvalidOperation);
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
-    // Deinitialize to make the tests reasonable
-    Configurator::cleanup();
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // We can't reconfigure now (not even manually)
-    EXPECT_THROW(Configurator::reconfigure(ElementPtr(new MapElement())),
-                 InvalidOperation);
-    // If one of them is NULL, it does not work
-    EXPECT_THROW(Configurator::init(NULL, this), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    EXPECT_THROW(Configurator::init(mccs.get(), NULL), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // But we can initialize it again now
-    EXPECT_NO_THROW(init());
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
+void
+testConfigureDataSource(DatasrcConfigTest& test,
+                        const isc::data::ConstElementPtr& config)
+{
+    // We use customized (faked lists) for the List type.  This makes it
+    // possible to easily look that they were called.
+    shared_ptr<std::map<dns::RRClass, ListPtr> > lists =
+        configureDataSourceGeneric<FakeList>(config);
+    test.swapDataSrcClientLists(lists);
 }
 
 // Push there a configuration with a single list.
-TEST_F(DatasrcConfiguratorTest, createList) {
+TEST_F(DatasrcConfigTest, createList) {
     initializeINList();
 }
 
-TEST_F(DatasrcConfiguratorTest, modifyList) {
-    // First, initialize the list
+TEST_F(DatasrcConfigTest, modifyList) {
+    // First, initialize the list, and confirm the current config
     initializeINList();
+    EXPECT_EQ("xxx", lists_[RRClass::IN()]->getConf());
+
     // And now change the configuration of the list
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}]}"));
@@ -180,15 +187,13 @@ TEST_F(DatasrcConfiguratorTest, modifyList) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    // This one does not set
-    EXPECT_EQ("get IN\n", log_);
-    // But this should contain the yyy configuration
+    // Now the new one should be installed.
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ(1, lists_.size());
 }
 
 // Check we can have multiple lists at once
-TEST_F(DatasrcConfiguratorTest, multiple) {
+TEST_F(DatasrcConfigTest, multiple) {
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], "
                                  "\"CH\": [{\"type\": \"xxx\"}]}"));
@@ -196,7 +201,7 @@ TEST_F(DatasrcConfiguratorTest, multiple) {
                        "*");
     mccs->checkCommand();
     // We have set commands for both classes.
-    EXPECT_EQ("get CH\nset CH xxx\nget IN\nset IN yyy\n", log_);
+    EXPECT_EQ("set IN yyy\nset CH xxx\n", log_);
     // We should have both there
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
@@ -208,7 +213,7 @@ TEST_F(DatasrcConfiguratorTest, multiple) {
 //
 // It's almost like above, but we initialize first with single-list
 // config.
-TEST_F(DatasrcConfiguratorTest, updateAdd) {
+TEST_F(DatasrcConfigTest, updateAdd) {
     initializeINList();
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], "
@@ -217,16 +222,14 @@ TEST_F(DatasrcConfiguratorTest, updateAdd) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    // The CH is set, IN not
-    EXPECT_EQ("get CH\nset CH xxx\nget IN\n", log_);
-    // But this should contain the yyy configuration
+    EXPECT_EQ("set IN yyy\nset CH xxx\n", log_);
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ(2, lists_.size());
 }
 
 // We delete a class list in this test.
-TEST_F(DatasrcConfiguratorTest, updateDelete) {
+TEST_F(DatasrcConfigTest, updateDelete) {
     initializeINList();
     const ElementPtr
         config(buildConfig("{}"));
@@ -234,18 +237,18 @@ TEST_F(DatasrcConfiguratorTest, updateDelete) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    EXPECT_EQ("get IN\nset IN \n", log_);
-    EXPECT_FALSE(lists_[RRClass::IN()]);
-    // In real auth server, the NULL one would be removed. However, we just
-    // store it, so the IN bucket is still in there. This checks there's nothing
-    // else.
-    EXPECT_EQ(1, lists_.size());
+
+    // No operation takes place in the configuration, and the old one is
+    // just dropped
+    EXPECT_EQ("", log_);
+    EXPECT_TRUE(lists_.empty());
 }
 
-// Check that we can rollback an addition if something else fails
-TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
+// Check that broken new configuration doesn't break the running configuration.
+TEST_F(DatasrcConfigTest, brokenConfigForAdd) {
     initializeINList();
-    // The configuration is wrong. However, the CH one will get done first.
+    // The configuration is wrong. However, the CH one will be handled
+    // without an error first.
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": 13}], "
                            "\"CH\": [{\"type\": \"xxx\"}]}"));
@@ -261,41 +264,40 @@ TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
     EXPECT_FALSE(lists_[RRClass::CH()]);
 }
 
-// Check that we can rollback a deletion if something else fails
-TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
+// Similar to the previous one, but the broken config would delete part of
+// the running config.
+TEST_F(DatasrcConfigTest, brokenConfigForDelete) {
     initializeINList();
     // Put the CH there
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
+    testConfigureDataSource(*this, config1);
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
-    // This would delete CH. However, the IN one fails.
-    // As the deletions happen after the additions/settings
-    // and there's no known way to cause an exception during the
-    // deletions, it is not a true rollback, but the result should
-    // be the same.
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    // This would delete CH. However, the new config is broken, so it won't
+    // actually apply.
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }
 
-// Check that we can roll back configuration change if something
-// fails later on.
-TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) {
+// Similar to the previous cases, but the broken config would modify the
+// running config of one of the classes.
+TEST_F(DatasrcConfigTest, brokenConfigForModify) {
     initializeINList();
     // Put the CH there
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
-    // Now, the CH happens first. But nevertheless, it should be
-    // restored to the previoeus version.
+    testConfigureDataSource(*this, config1);
+    // Now, the CH change will be handled first without an error, then
+    // the change to the IN class will fail, and the none of the changes
+    // will apply.
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
                                   "\"CH\": [{\"type\": \"yyy\"}]}"));
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }

src/bin/bind10/bind10_messages.mes

@@ -141,6 +141,16 @@ it now. The new configuration is printed.
 % BIND10_RECEIVED_SIGNAL received signal %1
 The boss module received the given signal.
 
+% BIND10_RESTART_COMPONENT_SKIPPED Skipped restarting a component %1
+The boss module tried to restart a component after it failed (crashed)
+unexpectedly, but the boss then found that the component had been removed
+from its local configuration of components to run.  This is an unusal
+situation but can happen if the administrator removes the component from
+the configuration after the component's crash and before the restart time.
+The boss module simply skipped restarting that module, and the whole system
+went back to the expected state (except that the crash itself is likely
+to be a bug).
+
 % BIND10_RESURRECTED_PROCESS resurrected %1 (PID %2)
 The given process has been restarted successfully, and is now running
 with the given process id.

src/bin/bind10/bind10_src.py.in

@@ -739,7 +739,7 @@ class BoB:
                 component = self.components.pop(pid)
                 logger.info(BIND10_PROCESS_ENDED, component.name(), pid,
                             exit_status)
-                if component.running() and self.runnable:
+                if component.is_running() and self.runnable:
                     # Tell it it failed. But only if it matters (we are
                     # not shutting down and the component considers itself
                     # to be running.
@@ -771,7 +771,12 @@ class BoB:
         next_restart_time = None
         now = time.time()
         for component in self.components_to_restart:
-            if not component.restart(now):
+            # If the component was removed from the configurator between since
+            # scheduled to restart, just ignore it.  The object will just be
+            # dropped here.
+            if not self._component_configurator.has_component(component):
+                logger.info(BIND10_RESTART_COMPONENT_SKIPPED, component.name())
+            elif not component.restart(now):
                 still_dead.append(component)
                 if next_restart_time is None or\
                    next_restart_time > component.get_restart_time():

src/bin/bind10/tests/bind10_test.py.in

@@ -929,7 +929,14 @@ class MockComponent:
         self.name = lambda: name
         self.pid = lambda: pid
         self.address = lambda: address
+        self.restarted = False
 
+    def get_restart_time(self):
+        return 0                # arbitrary dummy value
+
+    def restart(self, now):
+        self.restarted = True
+        return True
 
 class TestBossCmd(unittest.TestCase):
     def test_ping(self):
@@ -1266,6 +1273,34 @@ class TestBossComponents(unittest.TestCase):
         bob.start_all_components()
         self.__check_extended(self.__param)
 
+    def __setup_restart(self, bob, component):
+        '''Common procedure for restarting a component used below.'''
+        bob.components_to_restart = { component }
+        component.restarted = False
+        bob.restart_processes()
+
+    def test_restart_processes(self):
+        '''Check some behavior on restarting processes.'''
+        bob = MockBob()
+        bob.runnable = True
+        component = MockComponent('test', 53)
+
+        # A component to be restarted will actually be restarted iff it's
+        # in the configurator's configuration.
+        # We bruteforce the configurator internal below; ugly, but the easiest
+        # way for the test.
+        bob._component_configurator._components['test'] = (None, component)
+        self.__setup_restart(bob, component)
+        self.assertTrue(component.restarted)
+        self.assertFalse(component in bob.components_to_restart)
+
+        # Remove the component from the configuration.  It won't be restarted
+        # even if scheduled, nor will remain in the to-be-restarted list.
+        del bob._component_configurator._components['test']
+        self.__setup_restart(bob, component)
+        self.assertFalse(component.restarted)
+        self.assertFalse(component in bob.components_to_restart)
+
 class SocketSrvTest(unittest.TestCase):
     """
     This tests some methods of boss related to the unix domain sockets used

src/bin/dbutil/tests/Makefile.am

@@ -2,6 +2,8 @@ SUBDIRS = . testdata
 
 # Tests of the update script.
 
+noinst_SCRIPTS = dbutil_test.sh
+
 check-local:
 	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(SHELL) $(abs_builddir)/dbutil_test.sh

src/bin/dhcp4/tests/Makefile.am

@@ -16,6 +16,7 @@ check-local:
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 		$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/dhcp6/tests/Makefile.am

@@ -15,6 +15,7 @@ check-local:
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
 		$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/host/Makefile.am

@@ -1,37 +0,0 @@
-AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns
-AM_CPPFLAGS += $(BOOST_INCLUDES)
-
-AM_CXXFLAGS = $(B10_CXXFLAGS)
-
-if USE_STATIC_LINK
-AM_LDFLAGS = -static
-endif
-
-CLEANFILES = *.gcno *.gcda
-
-bin_PROGRAMS = b10-host
-b10_host_SOURCES = host.cc
-b10_host_LDADD = $(top_builddir)/src/lib/dns/libb10-dns++.la
-b10_host_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
-b10_host_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-
-man_MANS = b10-host.1
-DISTCLEANFILES = $(man_MANS)
-EXTRA_DIST = $(man_MANS) b10-host.xml
-
-.PHONY: man
-if GENERATE_DOCS
-
-man: b10-host.1
-
-b10-host.1: b10-host.xml
-	@XSLTPROC@ --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-host.xml
-
-else
-
-$(man_MANS):
-	@echo Man generation disabled.  Creating dummy $@.  Configure with --enable-generate-docs to enable it.
-	@echo Man generation disabled.  Remove this file, configure with --enable-generate-docs, and rebuild BIND 10 > $@
-
-endif

src/lib/config/ccsession.cc

@@ -338,7 +338,7 @@ getRelatedLoggers(ConstElementPtr loggers) {
 
     // Now find the wildcard names (the one that start with "*").
     BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
-        std::string cur_name = cur_logger->get("name")->stringValue();
+        const std::string cur_name = cur_logger->get("name")->stringValue();
         // If name is '*', or starts with '*.', replace * with root
         // logger name.
         if (cur_name == "*" || cur_name.length() > 1 &&
@@ -671,9 +671,7 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
 
 std::string
 ModuleCCSession::addRemoteConfig(const std::string& spec_name,
-                                 void (*handler)(const std::string& module,
-                                                 ConstElementPtr,
-                                                 const ConfigData&),
+                                 RemoteHandler handler,
                                  bool spec_is_filename)
 {
     // First get the module name, specification and default config

src/lib/config/ccsession.h

@@ -283,7 +283,7 @@ public:
      *                  the configuration manager must know it). If
      *                  spec_is_filename is true (the default), then a
      *                  filename is assumed, otherwise a module name.
-     * \param handler The handler function called whenever there's a change.
+     * \param handler The handler functor called whenever there's a change.
      *                Called once initally from this function. May be NULL
      *                if you don't want any handler to be called and you're
      *                fine with requesting the data through
@@ -296,11 +296,11 @@ public:
      * \return The name of the module specified in the given specification
      *         file
      */
+    typedef boost::function<void(const std::string&,
+                                 isc::data::ConstElementPtr,
+                                 const ConfigData&)> RemoteHandler;
     std::string addRemoteConfig(const std::string& spec_name,
-                                void (*handler)(const std::string& module_name,
-                                                isc::data::ConstElementPtr
-                                                update,
-                                                const ConfigData& config_data) = NULL,
+                                RemoteHandler handler = RemoteHandler(),
                                 bool spec_is_filename = true);
 
     /**
@@ -513,9 +513,6 @@ private:
         const std::string& command,
         isc::data::ConstElementPtr args);
 
-    typedef void (*RemoteHandler)(const std::string&,
-                                  isc::data::ConstElementPtr,
-                                  const ConfigData&);
     std::map<std::string, ConfigData> remote_module_configs_;
     std::map<std::string, RemoteHandler> remote_module_handlers_;
 

src/lib/datasrc/memory/domaintree.h

@@ -947,7 +947,7 @@ public:
         PARTIALMATCH, ///< A superdomain node was found
         NOTFOUND,   ///< Not even any superdomain was found
         /// \brief Returned by insert() if a node of the name already exists
-        ALREADYEXISTS,
+        ALREADYEXISTS
     };
 
     /// \brief Allocate and construct \c DomainTree
@@ -1080,55 +1080,25 @@ public:
     ///    of it. In that case, node parameter is left intact.
     //@{
 
-    /// \brief Simple find.
+    /// \brief Simple find
     ///
     /// Acts as described in the \ref find section.
     Result find(const isc::dns::Name& name,
-                DomainTreeNode<T>** node) const {
-        DomainTreeNodeChain<T> node_path;
-        const isc::dns::LabelSequence ls(name);
-        return (find<void*>(ls, node, node_path, NULL, NULL));
-    }
-
-    /// \brief Simple find returning immutable node.
-    ///
-    /// Acts as described in the \ref find section, but returns immutable node
-    /// pointer.
-    Result find(const isc::dns::Name& name,
                 const DomainTreeNode<T>** node) const {
         DomainTreeNodeChain<T> node_path;
-        DomainTreeNode<T> *target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
+        Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
         return (ret);
     }
 
     /// \brief Simple find, with node_path tracking
     ///
     /// Acts as described in the \ref find section.
-    Result find(const isc::dns::Name& name, DomainTreeNode<T>** node,
-                DomainTreeNodeChain<T>& node_path) const
-    {
-        const isc::dns::LabelSequence ls(name);
-        return (find<void*>(ls, node, node_path, NULL, NULL));
-    }
-
-    /// \brief Simple find returning immutable node, with node_path tracking
-    ///
-    /// Acts as described in the \ref find section, but returns immutable node
-    /// pointer.
     Result find(const isc::dns::Name& name, const DomainTreeNode<T>** node,
                 DomainTreeNodeChain<T>& node_path) const
     {
-        DomainTreeNode<T> *target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
+        Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
         return (ret);
     }
 
@@ -1143,13 +1113,9 @@ public:
                 bool (*callback)(const DomainTreeNode<T>&, CBARG),
                 CBARG callback_arg) const
     {
-        DomainTreeNode<T>* target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = find(ls, &target_node, node_path, callback,
+        Result ret = find(ls, node, node_path, callback,
                           callback_arg);
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
         return (ret);
     }
 
@@ -1229,30 +1195,10 @@ public:
     ///     \c true, it returns immediately with the current node.
     template <typename CBARG>
     Result find(const isc::dns::LabelSequence& target_labels_orig,
-                DomainTreeNode<T>** node,
-                DomainTreeNodeChain<T>& node_path,
-                bool (*callback)(const DomainTreeNode<T>&, CBARG),
-                CBARG callback_arg) const;
-
-    /// \brief Simple find returning immutable node.
-    ///
-    /// Acts as described in the \ref find section, but returns immutable
-    /// node pointer.
-    template <typename CBARG>
-    Result find(const isc::dns::LabelSequence& target_labels,
                 const DomainTreeNode<T>** node,
                 DomainTreeNodeChain<T>& node_path,
                 bool (*callback)(const DomainTreeNode<T>&, CBARG),
-                CBARG callback_arg) const
-    {
-        DomainTreeNode<T>* target_node = NULL;
-        Result ret = find(target_labels, &target_node, node_path,
-                          callback, callback_arg);
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
-        return (ret);
-    }
+                CBARG callback_arg) const;
     //@}
 
     /// \brief return the next bigger node in DNSSEC order from a given node
@@ -1515,7 +1461,7 @@ template <typename T>
 template <typename CBARG>
 typename DomainTree<T>::Result
 DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
-                    DomainTreeNode<T>** target,
+                    const DomainTreeNode<T>** target,
                     DomainTreeNodeChain<T>& node_path,
                     bool (*callback)(const DomainTreeNode<T>&, CBARG),
                     CBARG callback_arg) const
@@ -1526,11 +1472,11 @@ DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
                   " and label sequence");
     }
 
-    DomainTreeNode<T>* node;
+    const DomainTreeNode<T>* node;
 
     if (!node_path.isEmpty()) {
         // Get the top node in the node chain
-        node = const_cast<DomainTreeNode<T>*>(node_path.top());
+        node = node_path.top();
         // Start searching from its down pointer
         node = node->getDown();
     } else {

src/lib/datasrc/memory/memory_client.cc

@@ -626,22 +626,20 @@ InMemoryClient::InMemoryClientImpl::load(
     // node must point to a valid node now
     assert(node != NULL);
 
-    std::string* tstr = node->setData(new std::string(filename));
+    const std::string* tstr = node->setData(new std::string(filename));
     delete tstr;
 
-    ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
-                                                     zone_name));
+    const ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
+                                                           zone_name,
+                                                           holder.release()));
     if (result.code == result::SUCCESS) {
         // Only increment the zone count if the zone doesn't already
         // exist.
         ++zone_count_;
     }
-
-    ZoneTable::FindResult fr(zone_table_->setZoneData(zone_name,
-                                                      holder.release()));
-    assert(fr.code == result::SUCCESS);
-    if (fr.zone_data != NULL) {
-        ZoneData::destroy(mem_sgmt_, fr.zone_data, rrclass_);
+    // Destroy the old instance of the zone if there was any
+    if (result.zone_data != NULL) {
+        ZoneData::destroy(mem_sgmt_, result.zone_data, rrclass_);
     }
 
     return (result.code);
@@ -731,9 +729,9 @@ InMemoryClient::load(const isc::dns::Name& zone_name,
 
 const std::string
 InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
-    FileNameNode* node(NULL);
-    FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
-                                                               &node);
+    const FileNameNode* node(NULL);
+    const FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
+                                                                     &node);
     if (result == FileNameTree::EXACTMATCH) {
         return (*node->getData());
     } else {
@@ -741,24 +739,6 @@ InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
     }
 }
 
-result::Result
-InMemoryClient::add(const isc::dns::Name& zone_name,
-                    const ConstRRsetPtr& rrset)
-{
-    const ZoneTable::FindResult result =
-        impl_->zone_table_->findZone(zone_name);
-    if (result.code != result::SUCCESS) {
-        isc_throw(DataSourceError, "No such zone: " + zone_name.toText());
-    }
-
-    const ConstRRsetPtr sig_rrset =
-        rrset ? rrset->getRRsig() : ConstRRsetPtr();
-    impl_->add(rrset, sig_rrset, zone_name, *result.zone_data);
-
-    // add() doesn't allow duplicate add, so we always return SUCCESS.
-    return (result::SUCCESS);
-}
-
 namespace {
 
 class MemoryIterator : public ZoneIterator {

src/lib/datasrc/memory/memory_client.h

@@ -139,35 +139,6 @@ public:
     /// zone from a file before.
     const std::string getFileName(const isc::dns::Name& zone_name) const;
 
-    /// \brief Inserts an rrset into the zone.
-    ///
-    /// It puts another RRset into the zone.
-    ///
-    /// In the current implementation, this method doesn't allow an existing
-    /// RRset to be updated or overridden.  So the caller must make sure that
-    /// all RRs of the same type and name must be given in the form of a
-    /// single RRset.  The current implementation will also require that
-    /// when an RRSIG is added, the RRset to be covered has already been
-    /// added.  These restrictions are probably too strict when this data
-    /// source accepts various forms of input, so they should be revisited
-    /// later.
-    ///
-    /// Except for NullRRset and OutOfZone, this method does not guarantee
-    /// strong exception safety (it is currently not needed, if it is needed
-    /// in future, it should be implemented).
-    ///
-    /// \throw NullRRset \c rrset is a NULL pointer.
-    /// \throw OutOfZone The owner name of \c rrset is outside of the
-    /// origin of the zone.
-    /// \throw AddError Other general errors.
-    /// \throw Others This method might throw standard allocation exceptions.
-    ///
-    /// \param rrset The set to add.
-    /// \return SUCCESS or EXIST (if an rrset for given name and type already
-    ///    exists).
-    result::Result add(const isc::dns::Name& zone_name,
-                       const isc::dns::ConstRRsetPtr& rrset);
-
     /// \brief RRset is NULL exception.
     ///
     /// This is thrown if the provided RRset parameter is NULL.

src/lib/datasrc/memory/zone_finder.cc

@@ -428,7 +428,7 @@ FindNodeResult findNode(const ZoneData& zone_data,
                         ZoneFinder::FindOptions options,
                         bool out_of_zone_ok = false)
 {
-    ZoneNode* node = NULL;
+    const ZoneNode* node = NULL;
     FindState state((options & ZoneFinder::FIND_GLUE_OK) != 0);
 
     const ZoneTree& tree(zone_data.getZoneTree());

src/lib/datasrc/memory/zone_table.cc

@@ -69,17 +69,13 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
 
 ZoneTable::AddResult
 ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
-                   const Name& zone_name)
+                   const Name& zone_name, ZoneData* content)
 {
-    // Create a new ZoneData instance first.  If the specified name already
-    // exists in the table, the new data will soon be destroyed, but we want
-    // to make sure if this allocation fails the tree won't be changed to
-    // provide as strong guarantee as possible.  In practice, we generally
-    // expect the caller tries to add a zone only when it's a new one, so
-    // this should be a minor concern.
-    SegmentObjectHolder<ZoneData, RRClass> holder(
-        mem_sgmt, ZoneData::create(mem_sgmt, zone_name), zone_class);
-
+    if (content == NULL) {
+        isc_throw(isc::BadValue, "Zone content must not be NULL");
+    }
+    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, content,
+                                                  zone_class);
     // Get the node where we put the zone
     ZoneTableNode* node(NULL);
     switch (zones_->insert(mem_sgmt, zone_name, &node)) {
@@ -94,18 +90,18 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     // Can Not Happen
     assert(node != NULL);
 
-    // Is it empty? We either just created it or it might be nonterminal
-    if (node->isEmpty()) {
-        node->setData(holder.get());
-        return (AddResult(result::SUCCESS, holder.release()));
-    } else { // There's something there already
-        return (AddResult(result::EXIST, node->getData()));
+    // We can release now, setData never throws
+    ZoneData* old = node->setData(holder.release());
+    if (old != NULL) {
+        return (AddResult(result::EXIST, old));
+    } else {
+        return (AddResult(result::SUCCESS, NULL));
     }
 }
 
 ZoneTable::FindResult
 ZoneTable::findZone(const Name& name) const {
-    ZoneTableNode* node(NULL);
+    const ZoneTableNode* node(NULL);
     result::Result my_result;
 
     // Translate the return codes
@@ -132,20 +128,6 @@ ZoneTable::findZone(const Name& name) const {
     return (FindResult(my_result, node->getData()));
 }
 
-ZoneTable::FindResult
-ZoneTable::setZoneData(const Name& name, ZoneData* data)
-{
-    ZoneTableNode* node(NULL);
-
-    ZoneTableTree::Result result(zones_->find(name, &node));
-
-    if (result != ZoneTableTree::EXACTMATCH) {
-        return (FindResult(result::NOTFOUND, NULL));
-    } else {
-        return (FindResult(result::SUCCESS, node->setData(data)));
-    }
-}
-
 } // end of namespace memory
 } // end of namespace datasrc
 } // end of namespace isc

src/lib/datasrc/memory/zone_table.h

@@ -74,23 +74,23 @@ private:
     typedef DomainTreeNode<ZoneData> ZoneTableNode;
 
 public:
-    /// \brief Result data of addZone() method.
-    struct AddResult {
-        AddResult(result::Result param_code, ZoneData* param_zone_data) :
-            code(param_code), zone_data(param_zone_data)
-        {}
-        const result::Result code;
-        ZoneData* const zone_data;
-    };
+     /// \brief Result data of addZone() method.
+     struct AddResult {
+         AddResult(result::Result param_code, ZoneData* param_zone_data) :
+             code(param_code), zone_data(param_zone_data)
+         {}
+         const result::Result code;
+         ZoneData* const zone_data;
+     };
 
     /// \brief Result data of findZone() method.
     struct FindResult {
         FindResult(result::Result param_code,
-                   ZoneData* param_zone_data) :
+                   const ZoneData* param_zone_data) :
             code(param_code), zone_data(param_zone_data)
         {}
         const result::Result code;
-        ZoneData* const zone_data;
+        const ZoneData* const zone_data;
     };
 
 private:
@@ -140,30 +140,29 @@ public:
 
     /// Add a new zone to the \c ZoneTable.
     ///
-    /// This method creates a new \c ZoneData for the given zone name and
-    /// holds it in the internal table.  The newly created zone data will be
-    /// returned via the \c zone_data member of the return value.  If the given
-    /// zone name already exists in the table, a new data object won't be
-    /// created; instead, the existing corresponding data will be returned.
-    ///
-    /// The zone table keeps the ownership of the created zone data; the
-    /// caller must not try to destroy it directly.  (We'll eventually
-    /// add an interface to delete specific zone data from the table).
+    /// This method adds a given zone data to the internal table.
     ///
     /// \throw std::bad_alloc Internal resource allocation fails.
     ///
     /// \param mem_sgmt The \c MemorySegment to allocate zone data to be
-    /// created.  It must be the same segment that was used to create
-    /// the zone table at the time of create().
+    ///     created.  It must be the same segment that was used to create
+    ///     the zone table at the time of create().
     /// \param zone_name The name of the zone to be added.
     /// \param zone_class The RR class of the zone.  It must be the RR class
-    /// that is supposed to be associated to the zone table.
+    ///     that is supposed to be associated to the zone table.
+    /// \param content This one should hold the zone content (the ZoneData).
+    ///     The ownership is passed onto the zone table. Must not be null.
+    ///     Must correspond to the name and class and must be allocated from
+    ///     mem_sgmt.
     /// \return \c result::SUCCESS If the zone is successfully
-    /// added to the zone table.
-    /// \return \c result::EXIST The zone table already contains
-    /// zone of the same origin.
-    AddResult addZone(util::MemorySegment& mem_sgmt, dns::RRClass zone_class,
-                      const dns::Name& zone_name);
+    ///     added to the zone table.
+    /// \return \c result::EXIST The zone table already contained
+    ///     zone of the same origin. The old data is replaced and returned
+    ///     inside the result.
+    AddResult addZone(util::MemorySegment& mem_sgmt,
+                      dns::RRClass zone_class,
+                      const dns::Name& zone_name,
+                      ZoneData* content);
 
     /// Find a zone that best matches the given name in the \c ZoneTable.
     ///
@@ -185,16 +184,6 @@ public:
     /// \return A \c FindResult object enclosing the search result (see above).
     FindResult findZone(const isc::dns::Name& name) const;
 
-    /// Override the ZoneData for a node (zone) in the zone tree.
-    ///
-    /// \throw none
-    ///
-    /// \param name A domain name for which the zone data is set.
-    /// \param data The new zone data to set.
-    /// \return A \c FindResult object containing the old data if the
-    /// zone was found.
-    FindResult setZoneData(const isc::dns::Name& name, ZoneData* data);
-
 private:
     boost::interprocess::offset_ptr<ZoneTableTree> zones_;
 };

src/lib/datasrc/tests/memory/domaintree_unittest.cc

@@ -256,8 +256,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // "g.h" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
+    EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 
     // fission the node "g.h"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -270,8 +270,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // "g.h" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 }
 
 TEST_F(DomainTreeTest, additionalNodeFission) {
@@ -286,8 +286,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // "t.0" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
+    EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 
     // fission the node "t.0"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -300,8 +300,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // "t.0" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 }
 
 TEST_F(DomainTreeTest, findName) {
@@ -328,10 +328,10 @@ TEST_F(DomainTreeTest, findName) {
     EXPECT_EQ(TestDomainTree::PARTIALMATCH,
               dtree_expose_empty_node.find(Name("m.d.e.f"), &cdtnode));
 
-    // find dtnode
+    // find cdtnode
     EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("q.w.y.d.e.f"),
-                                                   &dtnode));
-    EXPECT_EQ(Name("q"), dtnode->getName());
+                                                   &cdtnode));
+    EXPECT_EQ(Name("q"), cdtnode->getName());
 }
 
 TEST_F(DomainTreeTest, findError) {
@@ -411,11 +411,12 @@ performCallbackTest(TestDomainTree& dtree,
                                                         Name("example"),
                                                         &parentdtnode));
     // the child/parent nodes shouldn't "inherit" the callback flag.
-    // "dtnode" may be invalid due to the insertion, so we need to re-find
-    // it.
+    // "dtnode" should still validly point to "callback.example", but we
+    // explicitly confirm it.
     EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("callback.example"),
-                                                   &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
+                                                     &cdtnode));
+    ASSERT_EQ(dtnode, cdtnode);
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
     EXPECT_FALSE(subdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
     EXPECT_FALSE(parentdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
 

src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -542,29 +542,6 @@ TEST_F(MemoryClientTest, loadRRSIGs) {
     EXPECT_EQ(1, client_->getZoneCount());
 }
 
-TEST_F(MemoryClientTest, loadRRSIGsRdataMixedCoveredTypes) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    RRsetPtr rrset(new RRset(Name("example.org"),
-                             RRClass::IN(), RRType::A(), RRTTL(3600)));
-    rrset->addRdata(in::A("192.0.2.1"));
-    rrset->addRdata(in::A("192.0.2.2"));
-
-    RRsetPtr rrsig(new RRset(Name("example.org"), zclass_,
-                             RRType::RRSIG(), RRTTL(300)));
-    rrsig->addRdata(generic::RRSIG("A 5 3 3600 20000101000000 20000201000000 "
-                                   "12345 example.org. FAKEFAKEFAKE"));
-    rrsig->addRdata(generic::RRSIG("NS 5 3 3600 20000101000000 20000201000000 "
-                                   "54321 example.org. FAKEFAKEFAKEFAKE"));
-    rrset->addRRsig(rrsig);
-
-    EXPECT_THROW(client_->add(Name("example.org"), rrset),
-                 InMemoryClient::AddError);
-
-    // Teardown checks for memory segment leaks
-}
-
 TEST_F(MemoryClientTest, getZoneCount) {
     EXPECT_EQ(0, client_->getZoneCount());
     client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
@@ -654,75 +631,6 @@ TEST_F(MemoryClientTest, getIteratorGetSOAThrowsNotImplemented) {
     EXPECT_THROW(iterator->getSOA(), isc::NotImplemented);
 }
 
-TEST_F(MemoryClientTest, addRRsetToNonExistentZoneThrows) {
-    // The zone "example.org" doesn't exist, so we can't add an RRset to
-    // it.
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a), DataSourceError);
-}
-
-TEST_F(MemoryClientTest, addOutOfZoneThrows) {
-    // Out of zone names should throw.
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-empty.zone");
-
-    RRsetPtr rrset_a(new RRset(Name("a.example.com"),
-                               RRClass::IN(), RRType::A(), RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
-                 OutOfZone);
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, addNullRRsetThrows) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    EXPECT_THROW(client_->add(Name("example.org"), ConstRRsetPtr()),
-                 InMemoryClient::NullRRset);
-
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
-                 InMemoryClient::AddError);
-
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, add) {
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
-
-    // Add another RRset
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-    client_->add(Name("example.org"), rrset_a);
-
-    ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
-
-    // First we have the SOA
-    ConstRRsetPtr rrset(iterator->getNextRRset());
-    EXPECT_TRUE(rrset);
-    EXPECT_EQ(RRType::A(), rrset->getType());
-
-    rrset = iterator->getNextRRset();
-    EXPECT_TRUE(rrset);
-    EXPECT_EQ(RRType::SOA(), rrset->getType());
-
-    // There's nothing else in this zone
-    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
-}
-
 TEST_F(MemoryClientTest, findZoneData) {
     client_->load(Name("example.org"),
                   TEST_DATA_DIR "/example.org-rrsigs.zone");
@@ -773,4 +681,16 @@ TEST_F(MemoryClientTest, getJournalReaderNotImplemented) {
     EXPECT_THROW(client_->getJournalReader(Name("."), 0, 0),
                  isc::NotImplemented);
 }
+
+// TODO (upon merge of #2268): Re-add (and modify not to need
+// InMemoryClient::add) the tests removed in
+// 7a628baa1a158b5837d6f383e10b30542d2ac59b. Maybe some of them
+// are really not needed.
+//
+// * MemoryClientTest::loadRRSIGsRdataMixedCoveredTypes
+// * MemoryClientTest::addRRsetToNonExistentZoneThrows
+// * MemoryClientTest::addOutOfZoneThrows
+// * MemoryClientTest::addNullRRsetThrows
+// * MemoryClientTest::addEmptyRRsetThrows
+// * MemoryClientTest::add
 }

src/lib/datasrc/tests/memory/zone_data_unittest.cc

@@ -108,7 +108,7 @@ void
 checkFindRdataSet(const ZoneTree& tree, const Name& name, RRType type,
                   const RdataSet* expected_set)
 {
-    ZoneNode* node = NULL;
+    const ZoneNode* node = NULL;
     tree.find(name, &node);
     ASSERT_NE(static_cast<ZoneNode*>(NULL), node);
     EXPECT_EQ(expected_set, RdataSet::find(node->getData(), type));

src/lib/datasrc/tests/memory/zone_table_unittest.cc

@@ -22,6 +22,7 @@
 #include <datasrc/result.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table.h>
+#include <datasrc/memory/segment_object_holder.h>
 
 #include <gtest/gtest.h>
 
@@ -30,6 +31,7 @@
 using namespace isc::dns;
 using namespace isc::datasrc;
 using namespace isc::datasrc::memory;
+using namespace isc::datasrc::memory::detail;
 
 namespace {
 // Memory segment specified for tests.  It normally behaves like a "local"
@@ -87,46 +89,89 @@ TEST_F(ZoneTableTest, create) {
 }
 
 TEST_F(ZoneTableTest, addZone) {
+    // It doesn't accept empty (NULL) zones
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1, NULL),
+                 isc::BadValue);
+
+    SegmentObjectHolder<ZoneData, RRClass> holder1(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    const ZoneData* data1(holder1.get());
     // Normal successful case.
-    const ZoneTable::AddResult result1 =
-        zone_table->addZone(mem_sgmt_, zclass_, zname1);
+    const ZoneTable::AddResult result1(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder1.release()));
     EXPECT_EQ(result::SUCCESS, result1.code);
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), result1.zone_data);
+    // It got released by it
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), holder1.get());
 
     // Duplicate add doesn't replace the existing data.
-    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 zname1).code);
-    EXPECT_EQ(result1.zone_data,
-              zone_table->addZone(mem_sgmt_, zclass_, zname1).zone_data);
+    SegmentObjectHolder<ZoneData, RRClass> holder2(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder2.release()));
+    EXPECT_EQ(result::EXIST, result2.code);
+    // The old one gets out
+    EXPECT_EQ(data1, result2.zone_data);
+    // It releases this one even when we replace the old zone
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), holder2.get());
+    // We need to release the old one manually
+    ZoneData::destroy(mem_sgmt_, result2.zone_data, zclass_);
+
+    SegmentObjectHolder<ZoneData, RRClass> holder3(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("EXAMPLE.COM")),
+                                    zclass_);
     // names are compared in a case insensitive manner.
-    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 Name("EXAMPLE.COM")).code);
+    const ZoneTable::AddResult result3(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           Name("EXAMPLE.COM"),
+                                                           holder3.release()));
+    EXPECT_EQ(result::EXIST, result3.code);
+    ZoneData::destroy(mem_sgmt_, result3.zone_data, zclass_);
     // Add some more different ones.  Should just succeed.
+    SegmentObjectHolder<ZoneData, RRClass> holder4(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder4.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder5(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder5.release()).code);
 
     // Have the memory segment throw an exception in extending the internal
     // tree.  It still shouldn't cause memory leak (which would be detected
     // in TearDown()).
-    mem_sgmt_.setThrowCount(2);
-    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org")),
+    SegmentObjectHolder<ZoneData, RRClass> holder6(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("example.org")), zclass_);
+    mem_sgmt_.setThrowCount(1);
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org"),
+                                     holder6.release()),
                  std::bad_alloc);
 }
 
 TEST_F(ZoneTableTest, findZone) {
-    const ZoneTable::AddResult add_result1 =
-        zone_table->addZone(mem_sgmt_, zclass_, zname1);
-    EXPECT_EQ(result::SUCCESS, add_result1.code);
+    SegmentObjectHolder<ZoneData, RRClass> holder1(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    ZoneData* zone_data = holder1.get();
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_, zname1,
+                                                   holder1.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder2(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder2.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder3(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder3.release()).code);
 
     const ZoneTable::FindResult find_result1 =
         zone_table->findZone(Name("example.com"));
     EXPECT_EQ(result::SUCCESS, find_result1.code);
-    EXPECT_EQ(add_result1.zone_data, find_result1.zone_data);
+    EXPECT_EQ(zone_data, find_result1.zone_data);
 
     EXPECT_EQ(result::NOTFOUND,
               zone_table->findZone(Name("example.org")).code);
@@ -137,14 +182,17 @@ TEST_F(ZoneTableTest, findZone) {
     // and the code should be PARTIALMATCH.
     EXPECT_EQ(result::PARTIALMATCH,
               zone_table->findZone(Name("www.example.com")).code);
-    EXPECT_EQ(add_result1.zone_data,
+    EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
+    SegmentObjectHolder<ZoneData, RRClass> holder4(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("com")), zclass_);
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
-                                                   Name("com")).code);
-    EXPECT_EQ(add_result1.zone_data,
+                                                   Name("com"),
+                                                   holder4.release()).code);
+    EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 }
 }

src/lib/dhcp/tests/.gitignore

@@ -1 +1,2 @@
 /libdhcp++_unittests
+/libdhcpsrv_unittests

src/lib/dns/Makefile.am

@@ -148,6 +148,7 @@ libdns___include_HEADERS = \
 	messagerenderer.h \
 	name.h \
 	question.h \
+	opcode.h \
 	rcode.h \
 	rdata.h \
 	rrparamregistry.h \

src/lib/python/isc/bind10/component.py

@@ -45,6 +45,7 @@ COMPONENT_RESTART_DELAY = 10
 
 STATE_DEAD = 'dead'
 STATE_STOPPED = 'stopped'
+STATE_RESTARTING = 'restarting'
 STATE_RUNNING = 'running'
 
 def get_signame(signal_number):
@@ -68,6 +69,7 @@ class BaseComponent:
       explicitly).
     - Running - after start() was called, it started successfully and is
       now running.
+    - Restarting - the component failed (crashed) and is waiting for a restart
     - Dead - it failed and can not be resurrected.
 
     Init
@@ -79,11 +81,11 @@ class BaseComponent:
                     |            |                |
                     |failure     | failed()       |
                     |            |                |
-                    v            |                |
+                    v            |                | start()/restart()
                     +<-----------+                |
                     |                             |
                     |  kind == dispensable or kind|== needed and failed late
-                    +-----------------------------+
+                    +-----------------------> Restarting
                     |
                     | kind == core or kind == needed and it failed too soon
                     v
@@ -163,7 +165,7 @@ class BaseComponent:
         """
         if self.__state == STATE_DEAD:
             raise ValueError("Can't resurrect already dead component")
-        if self.running():
+        if self.is_running():
             raise ValueError("Can't start already running component")
         logger.info(BIND10_COMPONENT_START, self.name())
         self.__state = STATE_RUNNING
@@ -188,7 +190,7 @@ class BaseComponent:
         """
         # This is not tested. It talks with the outher world, which is out
         # of scope of unittests.
-        if not self.running():
+        if not self.is_running():
             raise ValueError("Can't stop a component which is not running")
         logger.info(BIND10_COMPONENT_STOP, self.name())
         self.__state = STATE_STOPPED
@@ -234,9 +236,9 @@ class BaseComponent:
 
         logger.error(BIND10_COMPONENT_FAILED, self.name(), self.pid(),
                      exit_str)
-        if not self.running():
+        if not self.is_running():
             raise ValueError("Can't fail component that isn't running")
-        self.__state = STATE_STOPPED
+        self.__state = STATE_RESTARTING # tentatively set, maybe changed to DEAD
         self._failed_internal()
         # If it is a core component or the needed component failed to start
         # (including it stopped really soon)
@@ -284,7 +286,7 @@ class BaseComponent:
         else:
             return False
 
-    def running(self):
+    def is_running(self):
         """
         Informs if the component is currently running. It assumes the failed
         is called whenever the component really fails and there might be some
@@ -296,6 +298,15 @@ class BaseComponent:
         """
         return self.__state == STATE_RUNNING
 
+    def is_restarting(self):
+        """Informs if the component has failed and is waiting for a restart.
+
+        Unlike the case of is_running(), if this returns True it always means
+        the corresponding process has died and not yet restarted.
+
+        """
+        return self.__state == STATE_RESTARTING
+
     def _start_internal(self):
         """
         This method does the actual starting of a process. You need to override
@@ -560,6 +571,13 @@ class Configurator:
         self._running = False
         self.__reconfigure_internal(self._components, {})
 
+    def has_component(self, component):
+        '''Return if a specified component is configured.'''
+        # Values of self._components are tuples of (config, component).
+        # Extract the components of the tuples and see if the given one
+        # is included.
+        return component in map(lambda x: x[1], self._components.values())
+
     def reconfigure(self, configuration):
         """
         Changes configuration from the current one to the provided. It
@@ -591,7 +609,7 @@ class Configurator:
         for cname in old.keys():
             if cname not in new:
                 component = self._components[cname][1]
-                if component.running():
+                if component.is_running() or component.is_restarting():
                     plan.append({
                         'command': STOP_CMD,
                         'component': component,
@@ -674,7 +692,7 @@ class Configurator:
                     self._components[task['name']] = (task['config'],
                                                       component)
                 elif command == STOP_CMD:
-                    if component.running():
+                    if component.is_running():
                         component.stop()
                     del self._components[task['name']]
                 else:

src/lib/python/isc/bind10/tests/component_test.py

@@ -191,7 +191,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertFalse(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertFalse(self.__failed_called)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertFalse(component.is_restarting())
         # We can't stop or fail the component yet
         self.assertRaises(ValueError, component.stop)
         self.assertRaises(ValueError, component.failed, 1)
@@ -204,7 +205,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertFalse(self.__failed_called)
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
+        self.assertFalse(component.is_restarting())
 
     def __check_dead(self, component):
         """
@@ -215,7 +217,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
         self.assertEqual(1, self._exitcode)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertFalse(component.is_restarting())
         # Surely it can't be stopped when already dead
         self.assertRaises(ValueError, component.stop)
         # Nor started
@@ -234,7 +237,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
+        self.assertFalse(component.is_restarting())
         # Check it can't be started again
         self.assertRaises(ValueError, component.start)
 
@@ -246,7 +250,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertTrue(component.is_restarting())
 
     def __do_start_stop(self, kind):
         """
@@ -270,7 +275,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertTrue(self.__stop_called)
         self.assertFalse(self.__failed_called)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertFalse(component.is_restarting())
         # Check it can't be stopped twice
         self.assertRaises(ValueError, component.stop)
         # Or failed
@@ -553,10 +559,10 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertIsNone(component.pid())
         self.assertEqual(['hello'], component._params)
         self.assertEqual('Address', component._address)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
         self.assertEqual({}, self.__registered_processes)
         component.start()
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
         # Some versions of unittest miss assertIsInstance
         self.assertTrue(isinstance(component._procinfo, TestProcInfo))
         self.assertEqual(42, component.pid())
@@ -580,11 +586,11 @@ class ComponentTests(BossUtils, unittest.TestCase):
         """
         component = Component('component', self, 'needed', 'Address')
         component.start()
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
         self.assertEqual('component', self.__start_simple_params)
         component.pid = lambda: 42
         component.stop()
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
         self.assertEqual(('component', 'Address', 42),
                          self.__stop_process_params)
 
@@ -609,7 +615,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         component = Component('component', self, 'needed', 'Address',
                               [], ProcInfo)
         component.start()
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
         component.kill()
         self.assertTrue(process.terminated)
         self.assertFalse(process.killed)
@@ -872,12 +878,13 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
             'priority': 6,
             'special': 'test',
             'process': 'additional',
-            'kind': 'needed'
+            'kind': 'dispensable' # need to be dispensable so it could restart
         }
         self.log = []
         plan = configurator._build_plan(self.__build_components(self.__core),
                                         bigger)
-        self.assertEqual([('additional', 'init'), ('additional', 'needed')],
+        self.assertEqual([('additional', 'init'),
+                          ('additional', 'dispensable')],
                          self.log)
         self.assertEqual(1, len(plan))
         self.assertTrue('component' in plan[0])
@@ -888,15 +895,29 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         # Now remove the one component again
         # We run the plan so the component is wired into internal structures
         configurator._run_plan(plan)
-        self.log = []
-        plan = configurator._build_plan(self.__build_components(bigger),
-                                        self.__core)
-        self.assertEqual([], self.log)
-        self.assertEqual([{
-            'command': 'stop',
-            'name': 'additional',
-            'component': component
-        }], plan)
+        # We should have the 'additional' component in the configurator.
+        self.assertTrue(configurator.has_component(component))
+        for count in [0, 1]:    # repeat two times, see below
+            self.log = []
+            plan = configurator._build_plan(self.__build_components(bigger),
+                                            self.__core)
+            self.assertEqual([], self.log)
+            self.assertEqual([{
+                        'command': 'stop',
+                        'name': 'additional',
+                        'component': component
+                        }], plan)
+
+            if count is 0:
+                # We then emulate unexpected failure of the component (but
+                # before it restarts).  It shouldn't confuse the
+                # configurator in the second phase of the test
+                component.failed(0)
+        # Run the plan, confirm the specified component is gone.
+        configurator._run_plan(plan)
+        self.assertFalse(configurator.has_component(component))
+        # There shouldn't be any other object that has the same name
+        self.assertFalse('additional' in configurator._components)
 
         # We want to switch a component. So, prepare the configurator so it
         # holds one

src/lib/util/Makefile.am

@@ -33,3 +33,6 @@ EXTRA_DIST = python/pycppwrapper_util.h
 
 libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 CLEANFILES = *.gcno *.gcda
+
+libb10_util_includedir = $(includedir)/$(PACKAGE_NAME)/util
+libb10_util_include_HEADERS = buffer.h

src/lib/util/threads/tests/.gitignore

@@ -0,0 +1 @@
+/run_unittests

tests/tools/perfdhcp/templates/.gitignore

@@ -0,0 +1,5 @@
+/test1.hex
+/test2.hex
+/test3.hex
+/test4.hex
+/test5.hex