BIND 10 master, updated. 01f159a6b4277ccbbb76b18e1de811a0a71b80cd Merge #2861
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Jul 11 08:28:21 UTC 2013
The branch, master has been updated
via 01f159a6b4277ccbbb76b18e1de811a0a71b80cd (commit)
via c2dfa3796558372cb69c9bdad6963835c18b3937 (commit)
via 54c8dec80aa3198a5f6dd7123c1ca415a85a27f8 (commit)
via 009e306698d3c8018fc2a83aa2c9cb6bd787a875 (commit)
via e746e58a9fec019dd8a944baeed75c685a6cdf28 (commit)
via bc25129780f807429a8c9f4cec569ed35f6b066c (commit)
via d2cf6a85554d5d9e02f1073bf41c61206a82effc (commit)
via 32044b839f83bba409fad29df68e94247e7a94a0 (commit)
via 6ad2bdd472dd5ef1156577667c09bc0bd90d3f39 (commit)
via 497bf6025e70ca767c703dc192a11d968c0c031d (commit)
via 3ce8c82c2a3ef441510d2c7b0765169c2177f358 (commit)
via d1c359da0db07875b967e616b07e5d56bcc51588 (commit)
via d432cd1889063c53a6d5249be9bf9c6029736364 (commit)
via 6917e1335e35a4465b9e242a6c3e9b38c909d201 (commit)
via 2e7cd3c5a4935c2957d800c9e9f29e191e9bcece (commit)
from dd01c785f0d8eb616ee4179299dc53e0d2c0015c (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 01f159a6b4277ccbbb76b18e1de811a0a71b80cd
Merge: dd01c78 c2dfa37
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Jul 11 10:27:44 2013 +0200
Merge #2861
Synchronize the workthread and main thread in the authoritative server,
by providing callbacks executed in the main thread after a work in the
workthread is completed.
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth_messages.mes | 4 +
src/bin/auth/auth_srv.cc | 1 +
src/bin/auth/datasrc_clients_mgr.h | 200 +++++++++++++++++---
.../auth/tests/datasrc_clients_builder_unittest.cc | 129 ++++++++++---
src/bin/auth/tests/datasrc_clients_mgr_unittest.cc | 60 +++++-
src/bin/auth/tests/test_datasrc_clients_mgr.cc | 15 +-
src/bin/auth/tests/test_datasrc_clients_mgr.h | 30 ++-
7 files changed, 371 insertions(+), 68 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_messages.mes b/src/bin/auth/auth_messages.mes
index f0e9e46..e38468a 100644
--- a/src/bin/auth/auth_messages.mes
+++ b/src/bin/auth/auth_messages.mes
@@ -151,6 +151,10 @@ A separate thread for maintaining data source clients has been started.
% AUTH_DATASRC_CLIENTS_BUILDER_STOPPED data source builder thread stopped
The separate thread for maintaining data source clients has been stopped.
+% AUTH_DATASRC_CLIENTS_BUILDER_WAKE_ERR failed to wake up main thread: %1
+A low-level error happened when trying to send data to the main thread to wake
+it up. Terminating to prevent inconsistent state and possiblu hang ups.
+
% 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
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index ee0306f..27779a9 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -319,6 +319,7 @@ AuthSrvImpl::AuthSrvImpl(AbstractXfroutClient& xfrout_client,
xfrin_session_(NULL),
counters_(),
keyring_(NULL),
+ datasrc_clients_mgr_(io_service_),
ddns_base_forwarder_(ddns_forwarder),
ddns_forwarder_(NULL),
xfrout_connected_(false),
diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h
index 11475e2..a2d3d1f 100644
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -29,6 +29,9 @@
#include <datasrc/client_list.h>
#include <datasrc/memory/zone_writer.h>
+#include <asiolink/io_service.h>
+#include <asiolink/local_socket.h>
+
#include <auth/auth_log.h>
#include <auth/datasrc_config.h>
@@ -36,11 +39,16 @@
#include <boost/bind.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/noncopyable.hpp>
+#include <boost/function.hpp>
+#include <boost/foreach.hpp>
#include <exception>
#include <cassert>
+#include <cerrno>
#include <list>
#include <utility>
+#include <sys/types.h>
+#include <sys/socket.h>
namespace isc {
namespace auth {
@@ -77,13 +85,40 @@ enum CommandID {
NUM_COMMANDS
};
+/// \brief Callback to be called when the command is completed.
+typedef boost::function<void ()> FinishedCallback;
+
/// \brief The data type passed from DataSrcClientsMgr to
-/// DataSrcClientsBuilder.
+/// DataSrcClientsBuilder.
///
-/// The first element of the pair is the command ID, and the second element
-/// is its argument. If the command doesn't take an argument it should be
-/// a null pointer.
-typedef std::pair<CommandID, data::ConstElementPtr> Command;
+/// This just holds the data items together, no logic or protection
+/// is present here.
+struct Command {
+ /// \brief Constructor
+ ///
+ /// It just initializes the member variables of the same names
+ /// as the parameters.
+ Command(CommandID id, const data::ConstElementPtr& params,
+ const FinishedCallback& callback) :
+ id(id),
+ params(params),
+ callback(callback)
+ {}
+ /// \brief The command to execute
+ CommandID id;
+ /// \brief Argument of the command.
+ ///
+ /// If the command takes no argument, it should be null pointer.
+ ///
+ /// This may be a null pointer if the command takes no parameters.
+ data::ConstElementPtr params;
+ /// \brief A callback to be called once the command finishes.
+ ///
+ /// This may be an empty boost::function. In such case, no callback
+ /// will be called after completion.
+ FinishedCallback callback;
+};
+
} // namespace datasrc_clientmgr_internal
/// \brief Frontend to the manager object for data source clients.
@@ -113,6 +148,24 @@ private:
boost::shared_ptr<datasrc::ConfigurableClientList> >
ClientListsMap;
+ class FDGuard : boost::noncopyable {
+ public:
+ FDGuard(DataSrcClientsMgrBase *mgr) :
+ mgr_(mgr)
+ {}
+ ~FDGuard() {
+ if (mgr_->read_fd_ != -1) {
+ close(mgr_->read_fd_);
+ }
+ if (mgr_->write_fd_ != -1) {
+ close(mgr_->write_fd_);
+ }
+ }
+ private:
+ DataSrcClientsMgrBase* mgr_;
+ };
+ friend class FDGuard;
+
public:
/// \brief Thread-safe accessor to the data source client lists.
///
@@ -176,12 +229,20 @@ public:
///
/// \throw std::bad_alloc internal memory allocation failure.
/// \throw isc::Unexpected general unexpected system errors.
- DataSrcClientsMgrBase() :
+ DataSrcClientsMgrBase(asiolink::IOService& service) :
clients_map_(new ClientListsMap),
- builder_(&command_queue_, &cond_, &queue_mutex_, &clients_map_,
- &map_mutex_),
- builder_thread_(boost::bind(&BuilderType::run, &builder_))
- {}
+ fd_guard_(new FDGuard(this)),
+ read_fd_(-1), write_fd_(-1),
+ builder_(&command_queue_, &callback_queue_, &cond_, &queue_mutex_,
+ &clients_map_, &map_mutex_, createFds()),
+ builder_thread_(boost::bind(&BuilderType::run, &builder_)),
+ wakeup_socket_(service, read_fd_)
+ {
+ // Schedule wakeups when callbacks are pushed.
+ wakeup_socket_.asyncRead(
+ boost::bind(&DataSrcClientsMgrBase::processCallbacks, this, _1),
+ buffer, 1);
+ }
/// \brief The destructor.
///
@@ -220,6 +281,7 @@ public:
AUTH_DATASRC_CLIENTS_SHUTDOWN_UNEXPECTED_ERROR);
}
+ processCallbacks(); // Any leftover callbacks
cleanup(); // see below
}
@@ -234,11 +296,18 @@ public:
/// \brief std::bad_alloc
///
/// \param config_arg The new data source configuration. Must not be NULL.
- void reconfigure(data::ConstElementPtr config_arg) {
+ /// \param callback Called once the reconfigure command completes. It is
+ /// called in the main thread (not in the work one). It should be
+ /// exceptionless.
+ void reconfigure(const data::ConstElementPtr& config_arg,
+ const datasrc_clientmgr_internal::FinishedCallback&
+ callback = datasrc_clientmgr_internal::FinishedCallback())
+ {
if (!config_arg) {
isc_throw(InvalidParameter, "Invalid null config argument");
}
- sendCommand(datasrc_clientmgr_internal::RECONFIGURE, config_arg);
+ sendCommand(datasrc_clientmgr_internal::RECONFIGURE, config_arg,
+ callback);
reconfigureHook(); // for test's customization
}
@@ -257,12 +326,18 @@ public:
/// \param args Element argument that should be a map of the form
/// { "class": "IN", "origin": "example.com" }
/// (but class is optional and will default to IN)
+ /// \param callback Called once the loadZone command completes. It
+ /// is called in the main thread, not in the work thread. It should
+ /// be exceptionless.
///
/// \exception CommandError if the args value is null, or not in
/// the expected format, or contains
/// a bad origin or class string
void
- loadZone(data::ConstElementPtr args) {
+ loadZone(const data::ConstElementPtr& args,
+ const datasrc_clientmgr_internal::FinishedCallback& callback =
+ datasrc_clientmgr_internal::FinishedCallback())
+ {
if (!args) {
isc_throw(CommandError, "loadZone argument empty");
}
@@ -303,7 +378,7 @@ public:
// implement it would be to factor out the code from
// the start of doLoadZone(), and call it here too
- sendCommand(datasrc_clientmgr_internal::LOADZONE, args);
+ sendCommand(datasrc_clientmgr_internal::LOADZONE, args, callback);
}
private:
@@ -317,30 +392,79 @@ private:
void reconfigureHook() {}
void sendCommand(datasrc_clientmgr_internal::CommandID command,
- data::ConstElementPtr arg)
+ const data::ConstElementPtr& arg,
+ const datasrc_clientmgr_internal::FinishedCallback&
+ callback = datasrc_clientmgr_internal::FinishedCallback())
{
// 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));
+ datasrc_clientmgr_internal::Command(command, arg, callback));
cond_.signal();
}
+ int createFds() {
+ int fds[2];
+ int result = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds);
+ if (result != 0) {
+ isc_throw(Unexpected, "Can't create socket pair: " <<
+ strerror(errno));
+ }
+ read_fd_ = fds[0];
+ write_fd_ = fds[1];
+ return write_fd_;
+ }
+
+ void processCallbacks(const std::string& error = std::string()) {
+ // Schedule the next read.
+ wakeup_socket_.asyncRead(
+ boost::bind(&DataSrcClientsMgrBase::processCallbacks, this, _1),
+ buffer, 1);
+ if (!error.empty()) {
+ // Generally, there should be no errors (as we are the other end
+ // as well), but check just in case.
+ isc_throw(Unexpected, error);
+ }
+
+ // Steal the callbacks into local copy.
+ std::list<datasrc_clientmgr_internal::FinishedCallback> queue;
+ {
+ typename MutexType::Locker locker(queue_mutex_);
+ queue.swap(callback_queue_);
+ }
+
+ // Execute the callbacks
+ BOOST_FOREACH(const datasrc_clientmgr_internal::FinishedCallback&
+ callback, queue) {
+ callback();
+ }
+ }
+
//
// The following are shared with the builder.
//
// The list is used as a one-way queue: back-in, front-out
std::list<datasrc_clientmgr_internal::Command> command_queue_;
+ // Similar to above, for the callbacks that are ready to be called.
+ // While the command queue is for sending commands from the main thread
+ // to the work thread, this one is for the other direction. Protected
+ // by the same mutex (queue_mutex_).
+ std::list<datasrc_clientmgr_internal::FinishedCallback> callback_queue_;
CondVarType cond_; // condition variable for queue operations
MutexType queue_mutex_; // mutex to protect the queue
datasrc::ClientListMapPtr clients_map_;
// map of actual data source client objects
+ boost::scoped_ptr<FDGuard> fd_guard_; // A guard to close the fds.
+ int read_fd_, write_fd_; // Descriptors for wakeup
MutexType map_mutex_; // mutex to protect the clients map
BuilderType builder_;
ThreadType builder_thread_; // for safety this should be placed last
+ isc::asiolink::LocalSocket wakeup_socket_; // For integration of read_fd_
+ // to the asio loop
+ char buffer[1]; // Buffer for the wakeup socket.
};
namespace datasrc_clientmgr_internal {
@@ -385,12 +509,15 @@ public:
///
/// \throw None
DataSrcClientsBuilderBase(std::list<Command>* command_queue,
+ std::list<FinishedCallback>* callback_queue,
CondVarType* cond, MutexType* queue_mutex,
datasrc::ClientListMapPtr* clients_map,
- MutexType* map_mutex
+ MutexType* map_mutex,
+ int wake_fd
) :
- command_queue_(command_queue), cond_(cond), queue_mutex_(queue_mutex),
- clients_map_(clients_map), map_mutex_(map_mutex)
+ command_queue_(command_queue), callback_queue_(callback_queue),
+ cond_(cond), queue_mutex_(queue_mutex),
+ clients_map_(clients_map), map_mutex_(map_mutex), wake_fd_(wake_fd)
{}
/// \brief The main loop.
@@ -457,10 +584,12 @@ private:
// The following are shared with the manager
std::list<Command>* command_queue_;
+ std::list<FinishedCallback> *callback_queue_;
CondVarType* cond_;
MutexType* queue_mutex_;
datasrc::ClientListMapPtr* clients_map_;
MutexType* map_mutex_;
+ int wake_fd_;
};
// Shortcut typedef for normal use
@@ -494,6 +623,31 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::run() {
AUTH_DATASRC_CLIENTS_BUILDER_COMMAND_ERROR).
arg(e.what());
}
+ if (current_commands.front().callback) {
+ // Lock the queue
+ typename MutexType::Locker locker(*queue_mutex_);
+ callback_queue_->
+ push_back(current_commands.front().callback);
+ // Wake up the other end. If it would block, there are data
+ // and it'll wake anyway.
+ int result = send(wake_fd_, "w", 1, MSG_DONTWAIT);
+ if (result == -1 &&
+ (errno != EWOULDBLOCK && errno != EAGAIN)) {
+ // Note: the strerror might not be thread safe, as
+ // subsequent call to it might change the returned
+ // string. But that is unlikely and strerror_r is
+ // not portable and we are going to terminate anyway,
+ // so that's better than nothing.
+ //
+ // Also, this error handler is not tested. It should
+ // be generally impossible to happen, so it is hard
+ // to trigger in controlled way.
+ LOG_FATAL(auth_logger,
+ AUTH_DATASRC_CLIENTS_BUILDER_WAKE_ERR).
+ arg(strerror(errno));
+ std::terminate();
+ }
+ }
current_commands.pop_front();
}
}
@@ -515,7 +669,7 @@ bool
DataSrcClientsBuilderBase<MutexType, CondVarType>::handleCommand(
const Command& command)
{
- const CommandID cid = command.first;
+ const CommandID cid = command.id;
if (cid >= NUM_COMMANDS) {
// This shouldn't happen except for a bug within this file.
isc_throw(Unexpected, "internal bug: invalid command, ID: " << cid);
@@ -526,12 +680,12 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::handleCommand(
};
LOG_DEBUG(auth_logger, DBGLVL_TRACE_BASIC,
AUTH_DATASRC_CLIENTS_BUILDER_COMMAND).arg(command_desc.at(cid));
- switch (command.first) {
+ switch (command.id) {
case RECONFIGURE:
- doReconfigure(command.second);
+ doReconfigure(command.params);
break;
case LOADZONE:
- doLoadZone(command.second);
+ doLoadZone(command.params);
break;
case SHUTDOWN:
return (false);
diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
index 8aa223b..9b41388 100644
--- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
@@ -36,9 +36,13 @@
#include <boost/function.hpp>
+#include <sys/types.h>
+#include <sys/socket.h>
+
#include <cstdlib>
#include <string>
#include <sstream>
+#include <cerrno>
using isc::data::ConstElementPtr;
using namespace isc::dns;
@@ -54,17 +58,24 @@ protected:
DataSrcClientsBuilderTest() :
clients_map(new std::map<RRClass,
boost::shared_ptr<ConfigurableClientList> >),
- builder(&command_queue, &cond, &queue_mutex, &clients_map, &map_mutex),
+ write_end(-1), read_end(-1),
+ builder(&command_queue, &callback_queue, &cond, &queue_mutex,
+ &clients_map, &map_mutex, generateSockets()),
cond(command_queue, delayed_command_queue), rrclass(RRClass::IN()),
- shutdown_cmd(SHUTDOWN, ConstElementPtr()),
- noop_cmd(NOOP, ConstElementPtr())
+ shutdown_cmd(SHUTDOWN, ConstElementPtr(), FinishedCallback()),
+ noop_cmd(NOOP, ConstElementPtr(), FinishedCallback())
{}
+ ~ DataSrcClientsBuilderTest() {
+
+ }
void configureZones(); // used for loadzone related tests
ClientListMapPtr clients_map; // configured clients
std::list<Command> command_queue; // test command queue
std::list<Command> delayed_command_queue; // commands available after wait
+ std::list<FinishedCallback> callback_queue; // Callbacks from commands
+ int write_end, read_end;
TestDataSrcClientsBuilder builder;
TestCondVar cond;
TestMutex queue_mutex;
@@ -72,6 +83,15 @@ protected:
const RRClass rrclass;
const Command shutdown_cmd;
const Command noop_cmd;
+private:
+ int generateSockets() {
+ int pair[2];
+ int result = socketpair(AF_LOCAL, SOCK_STREAM, 0, pair);
+ assert(result == 0);
+ write_end = pair[0];
+ read_end = pair[1];
+ return write_end;
+ }
};
TEST_F(DataSrcClientsBuilderTest, runSingleCommand) {
@@ -82,6 +102,44 @@ TEST_F(DataSrcClientsBuilderTest, runSingleCommand) {
EXPECT_EQ(0, cond.wait_count); // no wait because command queue is not empty
EXPECT_EQ(1, queue_mutex.lock_count);
EXPECT_EQ(1, queue_mutex.unlock_count);
+ // No callback scheduled, none called.
+ EXPECT_TRUE(callback_queue.empty());
+ // Not woken up.
+ char c;
+ int result = recv(read_end, &c, 1, MSG_DONTWAIT);
+ EXPECT_EQ(-1, result);
+ EXPECT_TRUE(errno == EAGAIN || errno == EWOULDBLOCK);
+}
+
+// Just to have a valid function callback to pass
+void emptyCallsback() {}
+
+// Check a command finished callback is passed
+TEST_F(DataSrcClientsBuilderTest, commandFinished) {
+ command_queue.push_back(Command(SHUTDOWN, ConstElementPtr(),
+ emptyCallsback));
+ builder.run();
+ EXPECT_EQ(0, cond.wait_count); // no wait because command queue is not empty
+ // Once for picking up data, once for putting the callback there
+ EXPECT_EQ(2, queue_mutex.lock_count);
+ EXPECT_EQ(2, queue_mutex.unlock_count);
+ // There's one callback in the queue
+ ASSERT_EQ(1, callback_queue.size());
+ EXPECT_EQ(emptyCallsback, callback_queue.front());
+ // And we are woken up.
+ char c;
+ int result = recv(read_end, &c, 1, MSG_DONTWAIT);
+ EXPECT_EQ(1, result);
+}
+
+// Test that low-level errors with the synchronization socket
+// (an unexpected condition) is detected and program aborted.
+TEST_F(DataSrcClientsBuilderTest, finishedCrash) {
+ command_queue.push_back(Command(SHUTDOWN, ConstElementPtr(),
+ emptyCallsback));
+ // Break the socket
+ close(write_end);
+ EXPECT_DEATH_IF_SUPPORTED({builder.run();}, "");
}
TEST_F(DataSrcClientsBuilderTest, runMultiCommands) {
@@ -138,7 +196,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// the error handling
// A command structure we'll modify to send different commands
- Command reconfig_cmd(RECONFIGURE, ConstElementPtr());
+ Command reconfig_cmd(RECONFIGURE, ConstElementPtr(), FinishedCallback());
// Initially, no clients should be there
EXPECT_TRUE(clients_map->empty());
@@ -166,7 +224,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
"}"
);
- reconfig_cmd.second = good_config;
+ reconfig_cmd.params = good_config;
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(1, clients_map->size());
EXPECT_EQ(1, map_mutex.lock_count);
@@ -177,7 +235,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// If a 'bad' command argument got here, the config validation should
// have failed already, but still, the handler should return true,
// and the clients_map should not be updated.
- reconfig_cmd.second = Element::create("{ \"foo\": \"bar\" }");
+ reconfig_cmd.params = Element::create("{ \"foo\": \"bar\" }");
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(working_config_clients, clients_map);
// Building failed, so map mutex should not have been locked again
@@ -185,7 +243,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// The same for a configuration that has bad data for the type it
// specifies
- reconfig_cmd.second = bad_config;
+ reconfig_cmd.params = bad_config;
builder.handleCommand(reconfig_cmd);
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(working_config_clients, clients_map);
@@ -194,21 +252,21 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// The same goes for an empty parameter (it should at least be
// an empty map)
- reconfig_cmd.second = ConstElementPtr();
+ reconfig_cmd.params = ConstElementPtr();
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(working_config_clients, clients_map);
EXPECT_EQ(1, map_mutex.lock_count);
// Reconfigure again with the same good clients, the result should
// be a different map than the original, but not an empty one.
- reconfig_cmd.second = good_config;
+ reconfig_cmd.params = good_config;
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_NE(working_config_clients, clients_map);
EXPECT_EQ(1, clients_map->size());
EXPECT_EQ(2, map_mutex.lock_count);
// And finally, try an empty config to disable all datasource clients
- reconfig_cmd.second = Element::createMap();
+ reconfig_cmd.params = Element::createMap();
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(0, clients_map->size());
EXPECT_EQ(3, map_mutex.lock_count);
@@ -224,7 +282,8 @@ TEST_F(DataSrcClientsBuilderTest, shutdown) {
TEST_F(DataSrcClientsBuilderTest, badCommand) {
// out-of-range command ID
EXPECT_THROW(builder.handleCommand(Command(NUM_COMMANDS,
- ConstElementPtr())),
+ ConstElementPtr(),
+ FinishedCallback())),
isc::Unexpected);
}
@@ -308,7 +367,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZone) {
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"test1.example\"}"));
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback());
EXPECT_TRUE(builder.handleCommand(loadzone_cmd));
// loadZone involves two critical sections: one for getting the zone
@@ -369,7 +429,8 @@ TEST_F(DataSrcClientsBuilderTest,
// Now send the command to reload it
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}"));
+ " \"origin\": \"example.org\"}"),
+ FinishedCallback());
EXPECT_TRUE(builder.handleCommand(loadzone_cmd));
// And now it should be present too.
EXPECT_EQ(ZoneFinder::SUCCESS,
@@ -380,7 +441,8 @@ TEST_F(DataSrcClientsBuilderTest,
// An error case: the zone has no configuration. (note .com here)
const Command nozone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.com\"}"));
+ " \"origin\": \"example.com\"}"),
+ FinishedCallback());
EXPECT_THROW(builder.handleCommand(nozone_cmd),
TestDataSrcClientsBuilder::InternalCommandError);
// The previous zone is not hurt in any way
@@ -403,7 +465,8 @@ TEST_F(DataSrcClientsBuilderTest,
builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}")));
+ " \"origin\": \"example.org\"}"),
+ FinishedCallback()));
// Only one mutex was needed because there was no actual reload.
EXPECT_EQ(orig_lock_count + 1, map_mutex.lock_count);
EXPECT_EQ(orig_unlock_count + 1, map_mutex.unlock_count);
@@ -421,7 +484,8 @@ TEST_F(DataSrcClientsBuilderTest,
builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"nosuchzone.example\"}"))),
+ " \"origin\": \"nosuchzone.example\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
// basically impossible case: in-memory cache is completely disabled.
@@ -441,7 +505,8 @@ TEST_F(DataSrcClientsBuilderTest,
EXPECT_THROW(builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}"))),
+ " \"origin\": \"example.org\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
}
@@ -454,7 +519,8 @@ TEST_F(DataSrcClientsBuilderTest, loadBrokenZone) {
// there's an error in the new zone file. reload will be rejected.
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"test1.example\"}"));
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback());
EXPECT_THROW(builder.handleCommand(loadzone_cmd),
TestDataSrcClientsBuilder::InternalCommandError);
zoneChecks(clients_map, rrclass); // zone shouldn't be replaced
@@ -469,7 +535,8 @@ TEST_F(DataSrcClientsBuilderTest, loadUnreadableZone) {
TEST_DATA_BUILDDIR "/test1.zone.copied"));
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"test1.example\"}"));
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback());
EXPECT_THROW(builder.handleCommand(loadzone_cmd),
TestDataSrcClientsBuilder::InternalCommandError);
zoneChecks(clients_map, rrclass); // zone shouldn't be replaced
@@ -482,7 +549,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneWithoutDataSrc) {
Command(LOADZONE,
Element::fromJSON(
"{\"class\": \"IN\", "
- " \"origin\": \"test1.example\"}"))),
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
}
@@ -492,7 +560,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
if (!isc::util::unittests::runningOnValgrind()) {
// null arg: this causes assertion failure
EXPECT_DEATH_IF_SUPPORTED({
- builder.handleCommand(Command(LOADZONE, ElementPtr()));
+ builder.handleCommand(Command(LOADZONE, ElementPtr(),
+ FinishedCallback()));
}, "");
}
@@ -501,7 +570,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
Command(LOADZONE,
Element::fromJSON(
"{\"origin\": \"test1.example\","
- " \"class\": \"no_such_class\"}"))),
+ " \"class\": \"no_such_class\"}"),
+ FinishedCallback())),
InvalidRRClass);
// not a string
@@ -509,7 +579,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
Command(LOADZONE,
Element::fromJSON(
"{\"origin\": \"test1.example\","
- " \"class\": 1}"))),
+ " \"class\": 1}"),
+ FinishedCallback())),
isc::data::TypeError);
// class or origin is missing: result in assertion failure
@@ -517,7 +588,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
EXPECT_DEATH_IF_SUPPORTED({
builder.handleCommand(Command(LOADZONE,
Element::fromJSON(
- "{\"class\": \"IN\"}")));
+ "{\"class\": \"IN\"}"),
+ FinishedCallback()));
}, "");
}
@@ -525,12 +597,14 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
EXPECT_THROW(builder.handleCommand(
Command(LOADZONE,
Element::fromJSON(
- "{\"class\": \"IN\", \"origin\": \"...\"}"))),
+ "{\"class\": \"IN\", \"origin\": \"...\"}"),
+ FinishedCallback())),
EmptyLabel);
EXPECT_THROW(builder.handleCommand(
Command(LOADZONE,
Element::fromJSON(
- "{\"origin\": 10, \"class\": 1}"))),
+ "{\"origin\": 10, \"class\": 1}"),
+ FinishedCallback())),
isc::data::TypeError);
}
@@ -561,7 +635,8 @@ TEST_F(DataSrcClientsBuilderTest,
Command(LOADZONE,
Element::fromJSON(
"{\"origin\": \"test1.example\","
- " \"class\": \"IN\"}"))),
+ " \"class\": \"IN\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
}
diff --git a/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc b/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
index c37ef11..1bf8101 100644
--- a/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
@@ -38,13 +38,13 @@ void
shutdownCheck() {
// Check for common points on shutdown. The manager should have acquired
// the lock, put a SHUTDOWN command to the queue, and should have signaled
- // the builder.
- EXPECT_EQ(1, FakeDataSrcClientsBuilder::queue_mutex->lock_count);
+ // the builder. It should check again for the callback queue, with the lock
+ EXPECT_EQ(2, FakeDataSrcClientsBuilder::queue_mutex->lock_count);
EXPECT_EQ(1, FakeDataSrcClientsBuilder::cond->signal_count);
EXPECT_EQ(1, FakeDataSrcClientsBuilder::command_queue->size());
const Command& cmd = FakeDataSrcClientsBuilder::command_queue->front();
- EXPECT_EQ(SHUTDOWN, cmd.first);
- EXPECT_FALSE(cmd.second); // no argument
+ EXPECT_EQ(SHUTDOWN, cmd.id);
+ EXPECT_FALSE(cmd.params); // no argument
// Finally, the manager should wait for the thread to terminate.
EXPECT_TRUE(FakeDataSrcClientsBuilder::thread_waited);
@@ -130,8 +130,8 @@ TEST(DataSrcClientsMgrTest, reconfigure) {
// 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);
+ EXPECT_EQ(RECONFIGURE, cmd1.id);
+ EXPECT_EQ(reconfigure_arg, cmd1.params);
// Non-null, but semantically invalid argument. The manager doesn't do
// this check, so it should result in the same effect.
@@ -140,8 +140,8 @@ TEST(DataSrcClientsMgrTest, reconfigure) {
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);
+ EXPECT_EQ(RECONFIGURE, cmd2.id);
+ EXPECT_EQ(reconfigure_arg, cmd2.params);
// Passing NULL argument is immediately rejected
EXPECT_THROW(mgr.reconfigure(ConstElementPtr()), isc::InvalidParameter);
@@ -245,10 +245,52 @@ TEST(DataSrcClientsMgrTest, reload) {
EXPECT_EQ(3, FakeDataSrcClientsBuilder::command_queue->size());
}
+void
+callback(bool* called, int *tag_target, int tag_value) {
+ *called = true;
+ *tag_target = tag_value;
+}
+
+// Test we can wake up the main thread by writing to the file descriptor and
+// that the callbacks are executed and removed when woken up.
+TEST(DataSrcClientsMgrTest, wakeup) {
+ bool called = false;
+ int tag;
+ {
+ TestDataSrcClientsMgr mgr;
+ // There's some real file descriptor (or something that looks so)
+ ASSERT_GT(FakeDataSrcClientsBuilder::wakeup_fd, 0);
+ // Push a callback in and wake the manager
+ FakeDataSrcClientsBuilder::callback_queue->
+ push_back(boost::bind(callback, &called, &tag, 1));
+ EXPECT_EQ(1, write(FakeDataSrcClientsBuilder::wakeup_fd, "w", 1));
+ mgr.run_one();
+ EXPECT_TRUE(called);
+ EXPECT_EQ(1, tag);
+ EXPECT_TRUE(FakeDataSrcClientsBuilder::callback_queue->empty());
+
+ called = false;
+ // If we wake up and don't push anything, it doesn't break.
+ EXPECT_EQ(1, write(FakeDataSrcClientsBuilder::wakeup_fd, "w", 1));
+ mgr.run_one();
+ EXPECT_FALSE(called);
+
+ // When we terminate, it should process whatever is left
+ // of the callbacks. So push and terminate (and don't directly
+ // wake).
+ FakeDataSrcClientsBuilder::callback_queue->
+ push_back(boost::bind(callback, &called, &tag, 2));
+ }
+ EXPECT_TRUE(called);
+ EXPECT_EQ(2, tag);
+ EXPECT_TRUE(FakeDataSrcClientsBuilder::callback_queue->empty());
+}
+
TEST(DataSrcClientsMgrTest, realThread) {
// Using the non-test definition with a real thread. Just checking
// no disruption happens.
- DataSrcClientsMgr mgr;
+ isc::asiolink::IOService service;
+ DataSrcClientsMgr mgr(service);
}
} // unnamed namespace
diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.cc b/src/bin/auth/tests/test_datasrc_clients_mgr.cc
index 82937c0..80bc97c 100644
--- a/src/bin/auth/tests/test_datasrc_clients_mgr.cc
+++ b/src/bin/auth/tests/test_datasrc_clients_mgr.cc
@@ -26,7 +26,9 @@ namespace datasrc_clientmgr_internal {
// Define static DataSrcClientsBuilder member variables.
bool FakeDataSrcClientsBuilder::started = false;
std::list<Command>* FakeDataSrcClientsBuilder::command_queue = NULL;
+std::list<FinishedCallback>* FakeDataSrcClientsBuilder::callback_queue = NULL;
std::list<Command> FakeDataSrcClientsBuilder::command_queue_copy;
+std::list<FinishedCallback> FakeDataSrcClientsBuilder::callback_queue_copy;
TestCondVar* FakeDataSrcClientsBuilder::cond = NULL;
TestCondVar FakeDataSrcClientsBuilder::cond_copy;
TestMutex* FakeDataSrcClientsBuilder::queue_mutex = NULL;
@@ -38,6 +40,7 @@ bool FakeDataSrcClientsBuilder::thread_waited = false;
FakeDataSrcClientsBuilder::ExceptionFromWait
FakeDataSrcClientsBuilder::thread_throw_on_wait =
FakeDataSrcClientsBuilder::NOTHROW;
+int FakeDataSrcClientsBuilder::wakeup_fd = -1;
template<>
void
@@ -58,7 +61,7 @@ TestDataSrcClientsBuilder::doNoop() {
template<>
void
-TestDataSrcClientsMgr::cleanup() {
+TestDataSrcClientsMgrBase::cleanup() {
using namespace datasrc_clientmgr_internal;
// Make copy of some of the manager's member variables and reset the
// corresponding pointers. The currently pointed objects are in the
@@ -73,17 +76,21 @@ TestDataSrcClientsMgr::cleanup() {
FakeDataSrcClientsBuilder::cond_copy = cond_;
FakeDataSrcClientsBuilder::cond =
&FakeDataSrcClientsBuilder::cond_copy;
+ FakeDataSrcClientsBuilder::callback_queue_copy =
+ *FakeDataSrcClientsBuilder::callback_queue;
+ FakeDataSrcClientsBuilder::callback_queue =
+ &FakeDataSrcClientsBuilder::callback_queue_copy;
}
template<>
void
-TestDataSrcClientsMgr::reconfigureHook() {
+TestDataSrcClientsMgrBase::reconfigureHook() {
using namespace datasrc_clientmgr_internal;
// Simply replace the local map, ignoring bogus config value.
- assert(command_queue_.front().first == RECONFIGURE);
+ assert(command_queue_.front().id == RECONFIGURE);
try {
- clients_map_ = configureDataSource(command_queue_.front().second);
+ clients_map_ = configureDataSource(command_queue_.front().params);
} catch (...) {}
}
diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.h b/src/bin/auth/tests/test_datasrc_clients_mgr.h
index 9b1a367..34097da 100644
--- a/src/bin/auth/tests/test_datasrc_clients_mgr.h
+++ b/src/bin/auth/tests/test_datasrc_clients_mgr.h
@@ -20,6 +20,8 @@
#include <auth/datasrc_clients_mgr.h>
#include <datasrc/datasrc_config.h>
+#include <asiolink/io_service.h>
+
#include <boost/function.hpp>
#include <list>
@@ -131,15 +133,18 @@ public:
// true iff a builder has started.
static bool started;
- // These three correspond to the resource shared with the manager.
+ // These five correspond to the resource shared with the manager.
// xxx_copy will be set in the manager's destructor to record the
// final state of the manager.
static std::list<Command>* command_queue;
+ static std::list<FinishedCallback>* callback_queue;
static TestCondVar* cond;
static TestMutex* queue_mutex;
+ static int wakeup_fd;
static isc::datasrc::ClientListMapPtr* clients_map;
static TestMutex* map_mutex;
static std::list<Command> command_queue_copy;
+ static std::list<FinishedCallback> callback_queue_copy;
static TestCondVar cond_copy;
static TestMutex queue_mutex_copy;
@@ -153,15 +158,18 @@ public:
FakeDataSrcClientsBuilder(
std::list<Command>* command_queue,
+ std::list<FinishedCallback>* callback_queue,
TestCondVar* cond,
TestMutex* queue_mutex,
isc::datasrc::ClientListMapPtr* clients_map,
- TestMutex* map_mutex)
+ TestMutex* map_mutex, int wakeup_fd)
{
FakeDataSrcClientsBuilder::started = false;
FakeDataSrcClientsBuilder::command_queue = command_queue;
+ FakeDataSrcClientsBuilder::callback_queue = callback_queue;
FakeDataSrcClientsBuilder::cond = cond;
FakeDataSrcClientsBuilder::queue_mutex = queue_mutex;
+ FakeDataSrcClientsBuilder::wakeup_fd = wakeup_fd;
FakeDataSrcClientsBuilder::clients_map = clients_map;
FakeDataSrcClientsBuilder::map_mutex = map_mutex;
FakeDataSrcClientsBuilder::thread_waited = false;
@@ -201,18 +209,30 @@ typedef DataSrcClientsMgrBase<
datasrc_clientmgr_internal::TestThread,
datasrc_clientmgr_internal::FakeDataSrcClientsBuilder,
datasrc_clientmgr_internal::TestMutex,
- datasrc_clientmgr_internal::TestCondVar> TestDataSrcClientsMgr;
+ datasrc_clientmgr_internal::TestCondVar> TestDataSrcClientsMgrBase;
// A specialization of manager's "cleanup" called at the end of the
// destructor. We use this to record the final values of some of the class
// member variables.
template<>
void
-TestDataSrcClientsMgr::cleanup();
+TestDataSrcClientsMgrBase::cleanup();
template<>
void
-TestDataSrcClientsMgr::reconfigureHook();
+TestDataSrcClientsMgrBase::reconfigureHook();
+
+// A (hackish) trick how to not require the IOService to be passed from the
+// tests. We can't create the io service as a member, because it would
+// get initialized too late.
+class TestDataSrcClientsMgr :
+ public asiolink::IOService,
+ public TestDataSrcClientsMgrBase {
+public:
+ TestDataSrcClientsMgr() :
+ TestDataSrcClientsMgrBase(*static_cast<asiolink::IOService*>(this))
+ {}
+};
} // namespace auth
} // namespace isc
More information about the bind10-changes
mailing list