BIND 10 master, updated. aa9c2bbcf4e4522525194310791d94fcf84fc605 [master] Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 23 18:22:05 UTC 2012
The branch, master has been updated
via aa9c2bbcf4e4522525194310791d94fcf84fc605 (commit)
via d02295dada44afbbfe9c8b7878cecca536ba3a13 (commit)
via 9ba0013f74bf3ed03d95add7c1d46c2d28a94492 (commit)
via d709ef90b8f23a6c41a05d2f5a62cb91840fde0d (commit)
via 246a87a4a681a574a0ecff9b26a49e471a60138a (commit)
via 113201b72be892ffd8af815806ccbb0e14ad6b67 (commit)
via a8dcc73698a14603967aec4dfb09e183e559f84a (commit)
via bb59b815c94d86c6d5d34cdfd707b7e6e3522122 (commit)
via e13d083a39e86bcb60867b281ccd0a045bb55993 (commit)
via 07b436b864bd3193ecafbfe1f9c1fa54f90ac342 (commit)
via 6cf94944f1431422449f5740299ee2d6ae5fd988 (commit)
via a61ab28f80639eb289b7b1cf74e89932fb11f1f5 (commit)
via 5ac7c2d114c23a7bdf4c15a96d619027d2092d63 (commit)
via 2f6a2e484a7c4838a11e0696323c5a38efa1c6cc (commit)
via 15a048bef56f34ac35ac38f6b43512129dac0350 (commit)
via a9af8c8a7a7d2448c86949fca578ec942e5413dd (commit)
via 86fe8fce9db80466c67e0553c9f5ef404cdbb185 (commit)
via c9b8a023d197a3726f4177409847c29e523a69fc (commit)
via 0e56f890880b684ebb71245155d0ecd435c75fdf (commit)
via 0e70ad361ad0d42b3d0aa63c51355e20c48b9c32 (commit)
via adc18e7e1019626410a642aa8271627659ff1d3b (commit)
via 8a4c6f4bbfd10a372fa4c4d40a6cb5ec64047cb6 (commit)
via 77d522cf9e3b0cfd5325461b76bbaa2cc8a859f2 (commit)
via eb90dff94fbb2df0632f51982fc2c512af6e9c65 (commit)
from d0b0208ae8f3de712fa1329c8b01d16e3eaf8ac4 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit aa9c2bbcf4e4522525194310791d94fcf84fc605
Merge: d02295d d0b0208
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Oct 23 11:21:56 2012 -0700
[master] Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10
commit d02295dada44afbbfe9c8b7878cecca536ba3a13
Merge: 5fd4177 9ba0013
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Oct 23 11:21:35 2012 -0700
[master] Merge branch 'trac2211'
with fixing Conflicts to:
src/bin/auth/auth_srv.cc
src/bin/auth/auth_srv.h
src/bin/auth/datasrc_clients_mgr.h
src/bin/auth/main.cc
src/bin/auth/tests/auth_srv_unittest.cc
src/bin/auth/tests/command_unittest.cc
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth_messages.mes | 9 +-
src/bin/auth/auth_srv.cc | 61 ++--------
src/bin/auth/auth_srv.h | 76 ++----------
src/bin/auth/benchmarks/query_bench.cc | 12 +-
src/bin/auth/command.cc | 13 +-
src/bin/auth/datasrc_clients_mgr.h | 124 +++++++++++++++++---
src/bin/auth/datasrc_config.h | 2 -
src/bin/auth/main.cc | 46 +++-----
src/bin/auth/tests/auth_srv_unittest.cc | 95 ++++-----------
src/bin/auth/tests/command_unittest.cc | 74 ++++++------
.../auth/tests/datasrc_clients_builder_unittest.cc | 8 +-
src/bin/auth/tests/datasrc_clients_mgr_unittest.cc | 119 +++++++++++++++++++
src/bin/auth/tests/datasrc_config_unittest.cc | 12 +-
src/bin/auth/tests/test_datasrc_clients_mgr.cc | 14 +++
src/bin/auth/tests/test_datasrc_clients_mgr.h | 8 ++
15 files changed, 366 insertions(+), 307 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_messages.mes b/src/bin/auth/auth_messages.mes
index c51d4eb..8f9fa22 100644
--- a/src/bin/auth/auth_messages.mes
+++ b/src/bin/auth/auth_messages.mes
@@ -121,11 +121,10 @@ The separate thread for maintaining data source clients has been stopped.
% AUTH_DATASRC_CLIENTS_SHUTDOWN_ERROR error on waiting for data source builder thread: %1
This indicates that the separate thread for maintaining data source
clients had been terminated due to an uncaught exception, and the
-manager notices that at its own termination. There should have been
-AUTH_DATASRC_CLIENTS_BUILDER_FAILED or
-AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED error messages in past
-logs. If this message appears, the maintenance of the data source
-clients hasn't been working properly for some time.
+manager notices that at its own termination. This is not an expected
+event, because the thread is implemented so it catches all exceptions
+internally. So, if this message is logged it's most likely some internal
+bug, and it would be nice to file a bug report.
% AUTH_DATASRC_CLIENTS_SHUTDOWN_UNEXPECTED_ERROR Unexpected error on waiting for data source builder thread
Some exception happens while waiting for the termination of the
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index b555ea5..157ae03 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -26,7 +26,6 @@
#include <exceptions/exceptions.h>
#include <util/buffer.h>
-#include <util/threads/sync.h>
#include <dns/edns.h>
#include <dns/exceptions.h>
@@ -53,6 +52,7 @@
#include <auth/query.h>
#include <auth/statistics.h>
#include <auth/auth_log.h>
+#include <auth/datasrc_clients_mgr.h>
#include <boost/bind.hpp>
#include <boost/lexical_cast.hpp>
@@ -268,24 +268,8 @@ public:
/// The TSIG keyring
const shared_ptr<TSIGKeyRing>* keyring_;
- /// The data source client list
- ClientListMapPtr datasrc_client_lists_;
-
- 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, 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);
- }
- }
+ /// The data source client list manager
+ auth::DataSrcClientsMgr datasrc_clients_mgr_;
/// Bind the ModuleSpec object in config_session_ with
/// isc:config::ModuleSpec::validateStatistics.
@@ -315,8 +299,6 @@ public:
isc::dns::Message& message,
bool done);
- mutable util::thread::Mutex mutex_;
-
private:
bool xfrout_connected_;
AbstractXfroutClient& xfrout_client_;
@@ -336,8 +318,6 @@ 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),
@@ -488,6 +468,11 @@ AuthSrv::getIOService() {
return (impl_->io_service_);
}
+isc::auth::DataSrcClientsMgr&
+AuthSrv::getDataSrcClientsMgr() {
+ return (impl_->datasrc_clients_mgr_);
+}
+
void
AuthSrv::setXfrinSession(AbstractSession* xfrin_session) {
impl_->xfrin_session_ = xfrin_session;
@@ -646,15 +631,15 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
local_edns->setUDPSize(AuthSrvImpl::DEFAULT_LOCAL_UDPSIZE);
message.setEDNS(local_edns);
}
- // 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::getDataSrcClientListMutex()).
- isc::util::thread::Mutex::Locker locker(mutex_);
+ // Get access to data source client list through the holder and keep the
+ // holder until the processing and rendering is done to avoid inter-thread
+ // race.
+ auth::DataSrcClientsMgr::Holder datasrc_holder(datasrc_clients_mgr_);
try {
const ConstQuestionPtr question = *message.beginQuestion();
const shared_ptr<datasrc::ClientList>
- list(getDataSrcClientList(question->getClass()));
+ list(datasrc_holder.findClientList(question->getClass()));
if (list) {
const RRType& qtype = question->getType();
const Name& qname = question->getName();
@@ -933,26 +918,6 @@ AuthSrv::destroyDDNSForwarder() {
}
}
-ClientListMapPtr
-AuthSrv::swapDataSrcClientLists(ClientListMapPtr new_lists) {
- // TODO: Debug-build only check
- if (!impl_->mutex_.locked()) {
- isc_throw(isc::Unexpected, "Not locked!");
- }
- std::swap(new_lists, impl_->datasrc_client_lists_);
- return (new_lists);
-}
-
-shared_ptr<ConfigurableClientList>
-AuthSrv::getDataSrcClientList(const RRClass& rrclass) {
- return (impl_->getDataSrcClientList(rrclass));
-}
-
-util::thread::Mutex&
-AuthSrv::getDataSrcClientListMutex() const {
- return (impl_->mutex_);
-}
-
void
AuthSrv::setTCPRecvTimeout(size_t timeout) {
dnss_->setTCPRecvTimeout(timeout);
diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h
index 94efd08..2d53666 100644
--- a/src/bin/auth/auth_srv.h
+++ b/src/bin/auth/auth_srv.h
@@ -35,7 +35,9 @@
#include <asiolink/asiolink.h>
#include <server_common/portconfig.h>
+
#include <auth/statistics.h>
+#include <auth/datasrc_clients_mgr.h>
#include <boost/shared_ptr.hpp>
@@ -194,6 +196,11 @@ public:
/// \brief Return pointer to the Checkin callback function
isc::asiolink::SimpleCallback* getCheckinProvider() const { return (checkin_); }
+ /// \brief Return data source clients manager.
+ ///
+ /// \throw None
+ isc::auth::DataSrcClientsMgr& getDataSrcClientsMgr();
+
/// \brief Set the communication session with a separate process for
/// outgoing zone transfers.
///
@@ -303,77 +310,10 @@ public:
/// If there was no forwarder yet, this method does nothing.
void destroyDDNSForwarder();
- /// \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.
- ///
- /// 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.
- ///
- /// 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.
- isc::datasrc::ClientListMapPtr
- swapDataSrcClientLists(isc::datasrc::ClientListMapPtr 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>
- getDataSrcClientList(const isc::dns::RRClass& rrclass);
-
- /// \brief Return a mutex for the client lists.
- ///
- /// Background loading of data uses threads. Therefore we need to protect
- /// the client lists by a mutex, so they don't change (or get destroyed)
- /// during query processing. Get (and lock) this mutex whenever you do
- /// something with the lists and keep it locked until you finish. This
- /// is correct:
- /// \code
- /// {
- /// Mutex::Locker locker(auth->getDataSrcClientListMutex());
- /// boost::shared_ptr<isc::datasrc::ConfigurableClientList>
- /// list(auth->getDataSrcClientList(RRClass::IN()));
- /// // Do some processing here
- /// }
- /// \endcode
- ///
- /// But this is not (it releases the mutex too soon):
- /// \code
- /// boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
- /// {
- /// Mutex::Locker locker(auth->getDataSrcClientListMutex());
- /// list = auth->getDataSrcClientList(RRClass::IN()));
- /// }
- /// // Do some processing here
- /// \endcode
- ///
- /// \note This method is const even if you are allowed to modify
- /// (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& getDataSrcClientListMutex() const;
-
/// \brief Sets the timeout for incoming TCP connections
///
/// Incoming TCP connections that have not sent their data
- /// withing this time are dropped.
+ /// within this time are dropped.
///
/// \param timeout The timeout (in milliseconds). If se to
/// zero, no timeouts are used, and the connection will remain
diff --git a/src/bin/auth/benchmarks/query_bench.cc b/src/bin/auth/benchmarks/query_bench.cc
index 44aeb8d..77b3377 100644
--- a/src/bin/auth/benchmarks/query_bench.cc
+++ b/src/bin/auth/benchmarks/query_bench.cc
@@ -18,7 +18,6 @@
#include <bench/benchmark_util.h>
#include <util/buffer.h>
-#include <util/threads/sync.h>
#include <dns/message.h>
#include <dns/name.h>
@@ -33,6 +32,7 @@
#include <auth/auth_srv.h>
#include <auth/auth_config.h>
#include <auth/datasrc_config.h>
+#include <auth/datasrc_clients_mgr.h>
#include <auth/query.h>
#include <asiodns/asiodns.h>
@@ -127,9 +127,9 @@ public:
OutputBuffer& buffer) :
QueryBenchMark(queries, query_message, buffer)
{
- isc::util::thread::Mutex::Locker locker(
- server_->getDataSrcClientListMutex());
- server_->swapDataSrcClientLists(
+ // Note: setDataSrcClientLists() may be deprecated, but until then
+ // we use it because we want to be synchronized with the server.
+ server_->getDataSrcClientsMgr().setDataSrcClientLists(
configureDataSource(
Element::fromJSON("{\"IN\":"
" [{\"type\": \"sqlite3\","
@@ -148,9 +148,7 @@ public:
OutputBuffer& buffer) :
QueryBenchMark(queries, query_message, buffer)
{
- isc::util::thread::Mutex::Locker locker(
- server_->getDataSrcClientListMutex());
- server_->swapDataSrcClientLists(
+ server_->getDataSrcClientsMgr().setDataSrcClientLists(
configureDataSource(
Element::fromJSON("{\"IN\":"
" [{\"type\": \"MasterFiles\","
diff --git a/src/bin/auth/command.cc b/src/bin/auth/command.cc
index efd60f2..886fd6c 100644
--- a/src/bin/auth/command.cc
+++ b/src/bin/auth/command.cc
@@ -15,13 +15,13 @@
#include <auth/command.h>
#include <auth/auth_log.h>
#include <auth/auth_srv.h>
+#include <auth/datasrc_clients_mgr.h>
#include <cc/data.h>
#include <datasrc/client_list.h>
#include <config/ccsession.h>
#include <exceptions/exceptions.h>
#include <dns/rrclass.h>
-#include <util/threads/sync.h>
#include <string>
@@ -188,14 +188,11 @@ public:
if (!origin_elem) {
isc_throw(AuthCommandError, "Zone origin is missing");
}
- Name origin(origin_elem->stringValue());
+ const Name origin(origin_elem->stringValue());
- // 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.getDataSrcClientListMutex());
- const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
- list(server.getDataSrcClientList(zone_class));
+ DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
+ boost::shared_ptr<ConfigurableClientList> list =
+ holder.findClientList(zone_class);
if (!list) {
isc_throw(AuthCommandError, "There's no client list for "
diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h
index 28ddab5..d42d26c 100644
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -21,16 +21,20 @@
#include <log/logger_support.h>
#include <log/log_dbglevels.h>
-#include <auth/datasrc_config.h>
+#include <dns/rrclass.h>
+
#include <cc/data.h>
+
+#include <datasrc/data_source.h>
#include <datasrc/client_list.h>
-#include <dns/rrclass.h>
#include <auth/auth_log.h>
+#include <auth/datasrc_config.h>
#include <boost/array.hpp>
#include <boost/bind.hpp>
#include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
#include <list>
#include <utility>
@@ -84,8 +88,63 @@ typedef std::pair<CommandID, data::ConstElementPtr> Command;
/// \c DataSrcClientsMgr.
template <typename ThreadType, typename BuilderType, typename MutexType,
typename CondVarType>
-class DataSrcClientsMgrBase {
+class DataSrcClientsMgrBase : boost::noncopyable {
+private:
+ typedef std::map<dns::RRClass,
+ boost::shared_ptr<datasrc::ConfigurableClientList> >
+ ClientListsMap;
+
public:
+ /// \brief Thread-safe accessor to the data source client lists.
+ ///
+ /// This class provides a simple wrapper for searching the client lists
+ /// stored in the DataSrcClientsMgr in a thread-safe manner.
+ /// It ensures the result of \c getClientList() can be used without
+ /// causing a race condition with other threads that can possibly use
+ /// the same manager throughout the lifetime of the holder object.
+ ///
+ /// This also means the holder object is expected to have a short lifetime.
+ /// The application shouldn't try to keep it unnecessarily long.
+ /// It's normally expected to create the holder object on the stack
+ /// of a small scope and automatically let it be destroyed at the end
+ /// of the scope.
+ class Holder {
+ public:
+ Holder(DataSrcClientsMgrBase& mgr) :
+ mgr_(mgr), locker_(mgr_.map_mutex_)
+ {}
+
+ /// \brief Find a data source client list of a specified RR class.
+ ///
+ /// It returns a pointer to the list stored in the manager if found,
+ /// otherwise it returns NULL. The manager keeps the ownership of
+ /// the pointed object. Also, it's not safe to get access to the
+ /// object beyond the scope of the holder object.
+ ///
+ /// \note Since the ownership isn't transferred the return value
+ /// could be a bare pointer (and it's probably better in several
+ /// points). Unfortunately, some unit tests currently don't work
+ /// unless this method effectively shares the ownership with the
+ /// tests. That's the only reason why we return a shared pointer
+ /// for now. We should eventually fix it and change the return value
+ /// type (see Trac ticket #2395). Other applications must not
+ /// assume the ownership is actually shared.
+ boost::shared_ptr<datasrc::ConfigurableClientList> findClientList(
+ const dns::RRClass& rrclass)
+ {
+ const ClientListsMap::const_iterator
+ it = mgr_.clients_map_->find(rrclass);
+ if (it == mgr_.clients_map_->end()) {
+ return (boost::shared_ptr<datasrc::ConfigurableClientList>());
+ } else {
+ return (it->second);
+ }
+ }
+ private:
+ DataSrcClientsMgrBase& mgr_;
+ typename MutexType::Locker locker_;
+ };
+
/// \brief Constructor.
///
/// It internally invokes a separate thread and waits for further
@@ -99,6 +158,7 @@ public:
/// \throw std::bad_alloc internal memory allocation failure.
/// \throw isc::Unexpected general unexpected system errors.
DataSrcClientsMgrBase() :
+ clients_map_(new ClientListsMap),
builder_(&command_queue_, &cond_, &queue_mutex_, &clients_map_,
&map_mutex_),
builder_thread_(boost::bind(&BuilderType::run, &builder_))
@@ -144,6 +204,35 @@ public:
cleanup(); // see below
}
+ /// \brief Handle new full configuration for data source clients.
+ ///
+ /// This method simply passes the new configuration to the builder
+ /// and immediately returns. This method is basically exception free
+ /// as long as the caller passes a non NULL value for \c config_arg;
+ /// it doesn't validate the argument further.
+ ///
+ /// \brief isc::InvalidParameter config_arg is NULL.
+ /// \brief std::bad_alloc
+ ///
+ /// \param config_arg The new data source configuration. Must not be NULL.
+ void reconfigure(data::ConstElementPtr config_arg) {
+ if (!config_arg) {
+ isc_throw(InvalidParameter, "Invalid null config argument");
+ }
+ sendCommand(datasrc_clientmgr_internal::RECONFIGURE, config_arg);
+ reconfigureHook(); // for test's customization
+ }
+
+ /// \brief Set the underlying data source client lists to new lists.
+ ///
+ /// This is provided only for some existing tests until we support a
+ /// cleaner way to use faked data source clients. Non test code or
+ /// newer tests must not use this.
+ void setDataSrcClientLists(datasrc::ClientListMapPtr new_lists) {
+ typename MutexType::Locker locker(map_mutex_);
+ clients_map_ = new_lists;
+ }
+
private:
// This is expected to be called at the end of the destructor. It
// actually does nothing, but provides a customization point for
@@ -151,14 +240,18 @@ private:
// state of the class.
void cleanup() {}
+ // same as cleanup(), for reconfigure().
+ void reconfigureHook() {}
+
void sendCommand(datasrc_clientmgr_internal::CommandID command,
data::ConstElementPtr arg)
{
- {
- typename MutexType::Locker locker(queue_mutex_);
- command_queue_.push_back(
- datasrc_clientmgr_internal::Command(command, arg));
- }
+ // The lock will be held until the end of this method. Only
+ // push_back has to be protected, but we can avoid having an extra
+ // block this way.
+ typename MutexType::Locker locker(queue_mutex_);
+ command_queue_.push_back(
+ datasrc_clientmgr_internal::Command(command, arg));
cond_.signal();
}
@@ -195,7 +288,7 @@ namespace datasrc_clientmgr_internal {
/// This class is templated so that we can test it without involving actual
/// threads or locks.
template <typename MutexType, typename CondVarType>
-class DataSrcClientsBuilderBase {
+class DataSrcClientsBuilderBase : boost::noncopyable {
public:
/// \brief Constructor.
///
@@ -294,13 +387,12 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::run() {
std::list<Command> current_commands;
{
// Move all new commands to local queue under the protection of
- // queue_mutex_. Note that list::splice() should never throw.
+ // queue_mutex_.
typename MutexType::Locker locker(*queue_mutex_);
while (command_queue_->empty()) {
cond_->wait(*queue_mutex_);
}
- current_commands.splice(current_commands.end(),
- *command_queue_);
+ current_commands.swap(*command_queue_);
} // the lock is released here.
while (keep_running && !current_commands.empty()) {
@@ -312,12 +404,12 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::run() {
LOG_INFO(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_STOPPED);
} catch (const std::exception& ex) {
// We explicitly catch exceptions so we can log it as soon as possible.
- LOG_ERROR(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED).
+ LOG_FATAL(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED).
arg(ex.what());
- throw;
+ assert(false);
} catch (...) {
- LOG_ERROR(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED);
- throw;
+ LOG_FATAL(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED);
+ assert(false);
}
}
diff --git a/src/bin/auth/datasrc_config.h b/src/bin/auth/datasrc_config.h
index 4b67e86..77a4177 100644
--- a/src/bin/auth/datasrc_config.h
+++ b/src/bin/auth/datasrc_config.h
@@ -15,8 +15,6 @@
#ifndef DATASRC_CONFIG_H
#define DATASRC_CONFIG_H
-#include "auth_srv.h"
-
#include <cc/data.h>
#include <datasrc/client_list.h>
diff --git a/src/bin/auth/main.cc b/src/bin/auth/main.cc
index 5d68d16..1fe0f48 100644
--- a/src/bin/auth/main.cc
+++ b/src/bin/auth/main.cc
@@ -18,7 +18,6 @@
#include <util/buffer.h>
#include <util/io/socketsession.h>
-#include <util/threads/sync.h>
#include <dns/message.h>
#include <dns/messagerenderer.h>
@@ -95,32 +94,23 @@ datasrcConfigHandler(AuthSrv* server, bool* first_time,
const isc::config::ConfigData&)
{
assert(server != NULL);
- if (config->contains("classes")) {
- isc::datasrc::ClientListMapPtr 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.
+ // Note: remote config handler is requested to be exception free.
+ // While the code below is not 100% exception free, such an exception
+ // is really fatal and the server should actually stop. So we don't
+ // bother to catch them; the exception would be propagated to the
+ // top level of the server and terminate it.
+
+ 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;
+ server->getDataSrcClientsMgr().reconfigure(
+ config_session->getRemoteConfigValue("data_sources", "classes"));
+ } else if (config->contains("classes")) {
+ server->getDataSrcClientsMgr().reconfigure(config->get("classes"));
}
}
@@ -232,10 +222,6 @@ main(int argc, char* argv[]) {
isc::server_common::initKeyring(*config_session);
auth_server->setTSIGKeyRing(&isc::server_common::keyring);
- // Instantiate the data source clients manager. At the moment
- // just so we actually create it in system tests.
- DataSrcClientsMgr datasrc_clients_mgr;
-
// Start the data source configuration. We pass first_time and
// config_session for the hack described in datasrcConfigHandler.
bool first_time = true;
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index 732cdcc..d498d9d 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -39,7 +39,6 @@
#include <auth/datasrc_config.h>
#include <util/unittests/mock_socketsession.h>
-#include <util/threads/sync.h>
#include <dns/tests/unittest_util.h>
#include <testutils/dnsmessage_test.h>
#include <testutils/srv_test.h>
@@ -70,6 +69,7 @@ using namespace isc::util::unittests;
using namespace isc::dns::rdata;
using namespace isc::data;
using namespace isc::xfr;
+using namespace isc::auth;
using namespace isc::asiodns;
using namespace isc::asiolink;
using namespace isc::testutils;
@@ -726,11 +726,11 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
}
void
-installDataSrcClientLists(AuthSrv& server,
- ClientListMapPtr lists)
-{
- thread::Mutex::Locker locker(server.getDataSrcClientListMutex());
- server.swapDataSrcClientLists(lists);
+installDataSrcClientLists(AuthSrv& server, ClientListMapPtr lists) {
+ // For now, we use explicit swap than reconfigure() because the latter
+ // involves a separate thread and cannot guarantee the new config is
+ // available for the subsequent test.
+ server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
}
void
@@ -1438,16 +1438,16 @@ TEST_F(AuthSrvTest,
{
// Set real inmem client to proxy
updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
+ boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
+ DataSrcClientsMgr& mgr = server.getDataSrcClientsMgr();
{
- isc::util::thread::Mutex::Locker locker(
- server.getDataSrcClientListMutex());
- boost::shared_ptr<isc::datasrc::ConfigurableClientList>
- list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
- THROW_NEVER, false));
- ClientListMapPtr lists(new std::map<RRClass, ListPtr>);
- lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
- server.swapDataSrcClientLists(lists);
+ DataSrcClientsMgr::Holder holder(mgr);
+ list.reset(new FakeList(holder.findClientList(RRClass::IN()),
+ THROW_NEVER, false));
}
+ ClientListMapPtr lists(new std::map<RRClass, ListPtr>);
+ lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+ server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
createDataFromFile("nsec3query_nodnssec_fromWire.wire");
server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1470,14 +1470,16 @@ setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
{
updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
- isc::util::thread::Mutex::Locker locker(
- server.getDataSrcClientListMutex());
- boost::shared_ptr<isc::datasrc::ConfigurableClientList>
- list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
- throw_when, isc_exception, rrset));
+ boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
+ DataSrcClientsMgr& mgr = server.getDataSrcClientsMgr();
+ { // we need to limit the scope so swap is outside of it
+ DataSrcClientsMgr::Holder holder(mgr);
+ list.reset(new FakeList(holder.findClientList(RRClass::IN()),
+ throw_when, isc_exception, rrset));
+ }
ClientListMapPtr lists(new std::map<RRClass, ListPtr>);
lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
- server.swapDataSrcClientLists(lists);
+ mgr.setDataSrcClientLists(lists);
}
TEST_F(AuthSrvTest,
@@ -1784,57 +1786,4 @@ TEST_F(AuthSrvTest, DDNSForwardCreateDestroy) {
Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
}
-// Check the client list accessors
-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.getDataSrcClientListMutex());
-
- ClientListMapPtr lists; // initially empty
-
- // The lists don't exist. Therefore, the list of RRClasses is empty.
- EXPECT_TRUE(server.swapDataSrcClientLists(lists)->empty());
-
- // Put something in.
- 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.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.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.getDataSrcClientListMutex());
- }, isc::InvalidOperation);
-}
-
}
diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc
index 15bd662..280def6 100644
--- a/src/bin/auth/tests/command_unittest.cc
+++ b/src/bin/auth/tests/command_unittest.cc
@@ -16,8 +16,7 @@
#include "datasrc_util.h"
-#include <util/threads/sync.h>
-
+#include <auth/auth_srv.h>
#include <auth/command.h>
#include <auth/datasrc_config.h>
@@ -56,6 +55,7 @@ using namespace isc::datasrc;
using namespace isc::config;
using namespace isc::util::unittests;
using namespace isc::testutils;
+using namespace isc::auth;
using namespace isc::auth::unittest;
namespace {
@@ -174,27 +174,26 @@ TEST_F(AuthCommandTest, shutdownIncorrectPID) {
// zones, and checks the zones are correctly loaded.
void
zoneChecks(AuthSrv& server) {
- 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.getDataSrcClientList(RRClass::IN())->
- find(Name("ns.test1.example")).finder_->
- find(Name("ns.test1.example"), RRType::AAAA())->code);
- 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.getDataSrcClientList(RRClass::IN())->
- find(Name("ns.test2.example")).finder_->
- find(Name("ns.test2.example"), RRType::AAAA())->code);
+ const RRClass rrclass(RRClass::IN());
+
+ DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
+ EXPECT_EQ(ZoneFinder::SUCCESS,
+ holder.findClientList(rrclass)->find(Name("ns.test1.example"))
+ .finder_->find(Name("ns.test1.example"), RRType::A())->code);
+ EXPECT_EQ(ZoneFinder::NXRRSET,
+ holder.findClientList(rrclass)->find(Name("ns.test1.example")).
+ finder_->find(Name("ns.test1.example"), RRType::AAAA())->code);
+ EXPECT_EQ(ZoneFinder::SUCCESS,
+ holder.findClientList(rrclass)->find(Name("ns.test2.example")).
+ finder_->find(Name("ns.test2.example"), RRType::A())->code);
+ EXPECT_EQ(ZoneFinder::NXRRSET,
+ holder.findClientList(rrclass)->find(Name("ns.test2.example")).
+ finder_->find(Name("ns.test2.example"), RRType::AAAA())->code);
}
void
installDataSrcClientLists(AuthSrv& server, ClientListMapPtr lists) {
- isc::util::thread::Mutex::Locker locker(
- server.getDataSrcClientListMutex());
- server.swapDataSrcClientLists(lists);
+ server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
}
void
@@ -223,22 +222,24 @@ configureZones(AuthSrv& server) {
void
newZoneChecks(AuthSrv& server) {
- isc::util::thread::Mutex::Locker locker(
- server.getDataSrcClientListMutex());
- EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
+ const RRClass rrclass(RRClass::IN());
+
+ DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
+ EXPECT_EQ(ZoneFinder::SUCCESS, holder.findClientList(rrclass)->
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.getDataSrcClientList(RRClass::IN())->
+ EXPECT_EQ(ZoneFinder::SUCCESS, holder.findClientList(rrclass)->
find(Name("ns.test1.example")).finder_->
find(Name("ns.test1.example"), RRType::AAAA())->code);
// test2.example shouldn't change
- EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
+ EXPECT_EQ(ZoneFinder::SUCCESS, holder.findClientList(rrclass)->
find(Name("ns.test2.example")).finder_->
find(Name("ns.test2.example"), RRType::A())->code);
EXPECT_EQ(ZoneFinder::NXRRSET,
- server.getDataSrcClientList(RRClass::IN())->
+ holder.findClientList(rrclass)->
find(Name("ns.test2.example")).finder_->
find(Name("ns.test2.example"), RRType::AAAA())->code);
}
@@ -284,11 +285,11 @@ TEST_F(AuthCommandTest,
installDataSrcClientLists(server_, configureDataSource(config));
{
- isc::util::thread::Mutex::Locker locker(
- server_.getDataSrcClientListMutex());
+ DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
+
// Check that the A record at www.example.org does not exist
EXPECT_EQ(ZoneFinder::NXDOMAIN,
- server_.getDataSrcClientList(RRClass::IN())->
+ holder.findClientList(RRClass::IN())->
find(Name("example.org")).finder_->
find(Name("www.example.org"), RRType::A())->code);
@@ -309,7 +310,7 @@ TEST_F(AuthCommandTest,
sql_updater->commit();
EXPECT_EQ(ZoneFinder::NXDOMAIN,
- server_.getDataSrcClientList(RRClass::IN())->
+ holder.findClientList(RRClass::IN())->
find(Name("example.org")).finder_->
find(Name("www.example.org"), RRType::A())->code);
}
@@ -321,11 +322,10 @@ TEST_F(AuthCommandTest,
checkAnswer(0, "Successful load");
{
- isc::util::thread::Mutex::Locker locker(
- server_.getDataSrcClientListMutex());
+ DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
// And now it should be present too.
EXPECT_EQ(ZoneFinder::SUCCESS,
- server_.getDataSrcClientList(RRClass::IN())->
+ holder.findClientList(RRClass::IN())->
find(Name("example.org")).finder_->
find(Name("www.example.org"), RRType::A())->code);
}
@@ -336,11 +336,10 @@ TEST_F(AuthCommandTest,
checkAnswer(1, "example.com");
{
- isc::util::thread::Mutex::Locker locker(
- server_.getDataSrcClientListMutex());
+ DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
// The previous zone is not hurt in any way
EXPECT_EQ(ZoneFinder::SUCCESS,
- server_.getDataSrcClientList(RRClass::IN())->
+ holder.findClientList(RRClass::IN())->
find(Name("example.org")).finder_->
find(Name("example.org"), RRType::SOA())->code);
}
@@ -359,11 +358,10 @@ TEST_F(AuthCommandTest,
Element::fromJSON("{\"origin\": \"example.com\"}"));
checkAnswer(1, "Unreadable");
- isc::util::thread::Mutex::Locker locker(
- server_.getDataSrcClientListMutex());
+ DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
// The previous zone is not hurt in any way
EXPECT_EQ(ZoneFinder::SUCCESS,
- server_.getDataSrcClientList(RRClass::IN())->
+ holder.findClientList(RRClass::IN())->
find(Name("example.org")).finder_->
find(Name("example.org"), RRType::SOA())->code);
}
diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
index 74216f6..22d33cf 100644
--- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
@@ -71,14 +71,16 @@ TEST_F(DataSrcClientsBuilderTest, runMultiCommands) {
TEST_F(DataSrcClientsBuilderTest, exception) {
// Let the noop command handler throw exceptions and see if we can see
- // them.
+ // them. Right now, we simply abort to prevent the system from running
+ // with half-broken state. Eventually we should introduce a better
+ // error handling.
command_queue.push_back(noop_cmd);
queue_mutex.throw_from_noop = TestMutex::EXCLASS;
- EXPECT_THROW(builder.run(), isc::Exception);
+ EXPECT_DEATH_IF_SUPPORTED({builder.run();}, "");
command_queue.push_back(noop_cmd);
queue_mutex.throw_from_noop = TestMutex::INTEGER;
- EXPECT_THROW(builder.run(), int);
+ EXPECT_DEATH_IF_SUPPORTED({builder.run();}, "");
}
TEST_F(DataSrcClientsBuilderTest, condWait) {
diff --git a/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc b/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
index 5ad76b8..7d1eb4d 100644
--- a/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
@@ -12,8 +12,14 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
+#include <exceptions/exceptions.h>
+
+#include <dns/rrclass.h>
+
#include <cc/data.h>
+#include <datasrc/client_list.h>
+
#include <auth/datasrc_clients_mgr.h>
#include "test_datasrc_clients_mgr.h"
@@ -21,6 +27,9 @@
#include <boost/function.hpp>
+using namespace isc::dns;
+using namespace isc::data;
+using namespace isc::datasrc;
using namespace isc::auth;
using namespace isc::auth::datasrc_clientmgr_internal;
@@ -41,6 +50,30 @@ shutdownCheck() {
EXPECT_TRUE(FakeDataSrcClientsBuilder::thread_waited);
}
+// Commonly used pattern of checking member variables shared between the
+// manager and builder.
+void
+checkSharedMembers(size_t expected_queue_lock_count,
+ size_t expected_queue_unlock_count,
+ size_t expected_map_lock_count,
+ size_t expected_map_unlock_count,
+ size_t expected_cond_signal_count,
+ size_t expected_command_queue_size)
+{
+ EXPECT_EQ(expected_queue_lock_count,
+ FakeDataSrcClientsBuilder::queue_mutex->lock_count);
+ EXPECT_EQ(expected_queue_unlock_count,
+ FakeDataSrcClientsBuilder::queue_mutex->unlock_count);
+ EXPECT_EQ(expected_map_lock_count,
+ FakeDataSrcClientsBuilder::map_mutex->lock_count);
+ EXPECT_EQ(expected_map_unlock_count,
+ FakeDataSrcClientsBuilder::map_mutex->unlock_count);
+ EXPECT_EQ(expected_cond_signal_count,
+ FakeDataSrcClientsBuilder::cond->signal_count);
+ EXPECT_EQ(expected_command_queue_size,
+ FakeDataSrcClientsBuilder::command_queue->size());
+}
+
TEST(DataSrcClientsMgrTest, start) {
// When we create a manager, builder's run() method should be called.
FakeDataSrcClientsBuilder::started = false;
@@ -77,6 +110,92 @@ TEST(DataSrcClientsMgrTest, shutdownWithException) {
});
}
+TEST(DataSrcClientsMgrTest, reconfigure) {
+ TestDataSrcClientsMgr mgr;
+
+ // Check pre-command condition
+ checkSharedMembers(0, 0, 0, 0, 0, 0);
+
+ // A valid reconfigure argument
+ ConstElementPtr reconfigure_arg = Element::fromJSON(
+ "{""\"IN\": [{\"type\": \"MasterFiles\", \"params\": {},"
+ " \"cache-enable\": true}]}");
+
+ // On reconfigure(), it just send the RECONFIGURE command to the builder
+ // thread with the given argument intact.
+ mgr.reconfigure(reconfigure_arg);
+
+ // The manager should have acquired the queue lock, send RECONFIGURE
+ // command with the arg, wake up the builder thread by signal. It doesn't
+ // touch or refer to the map, so it shouldn't acquire the map lock.
+ checkSharedMembers(1, 1, 0, 0, 1, 1);
+ const Command& cmd1 = FakeDataSrcClientsBuilder::command_queue->front();
+ EXPECT_EQ(RECONFIGURE, cmd1.first);
+ EXPECT_EQ(reconfigure_arg, cmd1.second);
+
+ // Non-null, but semantically invalid argument. The manager doesn't do
+ // this check, so it should result in the same effect.
+ FakeDataSrcClientsBuilder::command_queue->clear();
+ reconfigure_arg = isc::data::Element::create("{ \"foo\": \"bar\" }");
+ mgr.reconfigure(reconfigure_arg);
+ checkSharedMembers(2, 2, 0, 0, 2, 1);
+ const Command& cmd2 = FakeDataSrcClientsBuilder::command_queue->front();
+ EXPECT_EQ(RECONFIGURE, cmd2.first);
+ EXPECT_EQ(reconfigure_arg, cmd2.second);
+
+ // Passing NULL argument is immediately rejected
+ EXPECT_THROW(mgr.reconfigure(ConstElementPtr()), isc::InvalidParameter);
+ checkSharedMembers(2, 2, 0, 0, 2, 1); // no state change
+}
+
+TEST(DataSrcClientsMgrTest, holder) {
+ TestDataSrcClientsMgr mgr;
+
+ {
+ // Initially it's empty, so findClientList() will always return NULL
+ TestDataSrcClientsMgr::Holder holder(mgr);
+ EXPECT_FALSE(holder.findClientList(RRClass::IN()));
+ EXPECT_FALSE(holder.findClientList(RRClass::CH()));
+ // map should be protected here
+ EXPECT_EQ(1, FakeDataSrcClientsBuilder::map_mutex->lock_count);
+ EXPECT_EQ(0, FakeDataSrcClientsBuilder::map_mutex->unlock_count);
+ }
+ // map lock has been released
+ EXPECT_EQ(1, FakeDataSrcClientsBuilder::map_mutex->unlock_count);
+
+ // Put something in, that should become visible.
+ ConstElementPtr reconfigure_arg = Element::fromJSON(
+ "{\"IN\": [{\"type\": \"MasterFiles\", \"params\": {},"
+ " \"cache-enable\": true}],"
+ " \"CH\": [{\"type\": \"MasterFiles\", \"params\": {},"
+ " \"cache-enable\": true}]}");
+ mgr.reconfigure(reconfigure_arg);
+ {
+ TestDataSrcClientsMgr::Holder holder(mgr);
+ EXPECT_TRUE(holder.findClientList(RRClass::IN()));
+ EXPECT_TRUE(holder.findClientList(RRClass::CH()));
+ }
+ // We need to clear command queue by hand
+ FakeDataSrcClientsBuilder::command_queue->clear();
+
+ // Replace the lists with new lists containing only one list.
+ // The CH will disappear again.
+ reconfigure_arg = Element::fromJSON(
+ "{\"IN\": [{\"type\": \"MasterFiles\", \"params\": {},"
+ " \"cache-enable\": true}]}");
+ mgr.reconfigure(reconfigure_arg);
+ {
+ TestDataSrcClientsMgr::Holder holder(mgr);
+ EXPECT_TRUE(holder.findClientList(RRClass::IN()));
+ EXPECT_FALSE(holder.findClientList(RRClass::CH()));
+ }
+
+ // Duplicate lock acquisition is prohibited (only test mgr can detect
+ // this reliably, so this test may not be that useful)
+ TestDataSrcClientsMgr::Holder holder1(mgr);
+ EXPECT_THROW(TestDataSrcClientsMgr::Holder holder2(mgr), isc::Unexpected);
+}
+
TEST(DataSrcClientsMgrTest, realThread) {
// Using the non-test definition with a real thread. Just checking
// no disruption happens.
diff --git a/src/bin/auth/tests/datasrc_config_unittest.cc b/src/bin/auth/tests/datasrc_config_unittest.cc
index 2dc70d1..b555aa6 100644
--- a/src/bin/auth/tests/datasrc_config_unittest.cc
+++ b/src/bin/auth/tests/datasrc_config_unittest.cc
@@ -16,7 +16,6 @@
#include <config/tests/fake_session.h>
#include <config/ccsession.h>
-#include <util/threads/sync.h>
#include <gtest/gtest.h>
@@ -78,12 +77,8 @@ datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
class DatasrcConfigTest : public ::testing::Test {
public:
- // These pretend to be the server
- isc::util::thread::Mutex& getDataSrcClientListMutex() const {
- return (mutex_);
- }
- void swapDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
- new_lists)
+ void setDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
+ new_lists)
{
lists_.clear(); // first empty it
@@ -156,7 +151,6 @@ protected:
const string specfile;
std::map<RRClass, ListPtr> lists_;
string log_;
- mutable isc::util::thread::Mutex mutex_;
};
void
@@ -167,7 +161,7 @@ testConfigureDataSource(DatasrcConfigTest& test,
// possible to easily look that they were called.
shared_ptr<std::map<dns::RRClass, ListPtr> > lists =
configureDataSourceGeneric<FakeList>(config);
- test.swapDataSrcClientLists(lists);
+ test.setDataSrcClientLists(lists);
}
// Push there a configuration with a single list.
diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.cc b/src/bin/auth/tests/test_datasrc_clients_mgr.cc
index 37c225e..44c8b7d 100644
--- a/src/bin/auth/tests/test_datasrc_clients_mgr.cc
+++ b/src/bin/auth/tests/test_datasrc_clients_mgr.cc
@@ -17,6 +17,8 @@
#include "test_datasrc_clients_mgr.h"
+#include <cassert>
+
namespace isc {
namespace auth {
namespace datasrc_clientmgr_internal {
@@ -71,6 +73,18 @@ TestDataSrcClientsMgr::cleanup() {
&FakeDataSrcClientsBuilder::cond_copy;
}
+template<>
+void
+TestDataSrcClientsMgr::reconfigureHook() {
+ using namespace datasrc_clientmgr_internal;
+
+ // Simply replace the local map, ignoring bogus config value.
+ assert(command_queue_.front().first == RECONFIGURE);
+ try {
+ clients_map_ = configureDataSource(command_queue_.front().second);
+ } catch (...) {}
+}
+
} // namespace auth
} // namespace isc
diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.h b/src/bin/auth/tests/test_datasrc_clients_mgr.h
index 98c7682..4abffa2 100644
--- a/src/bin/auth/tests/test_datasrc_clients_mgr.h
+++ b/src/bin/auth/tests/test_datasrc_clients_mgr.h
@@ -51,6 +51,11 @@ public:
class Locker {
public:
Locker(TestMutex& mutex) : mutex_(mutex) {
+ if (mutex.lock_count != mutex.unlock_count) {
+ isc_throw(Unexpected,
+ "attempt of duplicate lock acquisition");
+ }
+
++mutex.lock_count;
if (mutex.lock_count > 100) { // 100 is an arbitrary choice
isc_throw(Unexpected,
@@ -204,6 +209,9 @@ template<>
void
TestDataSrcClientsMgr::cleanup();
+template<>
+void
+TestDataSrcClientsMgr::reconfigureHook();
} // namespace auth
} // namespace isc
More information about the bind10-changes
mailing list