BIND 10 trac2202, updated. e25a0b2202650ef749340a1993b427bf99d962e5 [2202] Check the mutex is locked when it should
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Sep 5 08:44:43 UTC 2012
The branch, trac2202 has been updated
via e25a0b2202650ef749340a1993b427bf99d962e5 (commit)
from 7cd33029c63a0cd0889dba94709cf6cadaea48ab (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 e25a0b2202650ef749340a1993b427bf99d962e5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 5 10:43:51 2012 +0200
[2202] Check the mutex is locked when it should
And update tests so they lock.
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth_srv.cc | 14 ++++++
src/bin/auth/tests/auth_srv_unittest.cc | 16 +++++--
src/bin/auth/tests/command_unittest.cc | 76 ++++++++++++++++++-------------
3 files changed, 70 insertions(+), 36 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 07fd5e7..9f6becf 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -264,6 +264,10 @@ public:
boost::shared_ptr<ConfigurableClientList> getClientList(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()) {
@@ -913,6 +917,11 @@ AuthSrv::destroyDDNSForwarder() {
void
AuthSrv::setClientList(const RRClass& rrclass,
const boost::shared_ptr<ConfigurableClientList>& list) {
+ // TODO: Debug-build only check
+ if (!impl_->mutex_.locked()) {
+ isc_throw(isc::Unexpected, "Not locked");
+ }
+
if (list) {
impl_->client_lists_[rrclass] = list;
} else {
@@ -926,6 +935,11 @@ AuthSrv::getClientList(const RRClass& rrclass) {
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());
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index a464f62..de1a9ec 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -1423,10 +1423,13 @@ TEST_F(AuthSrvTest,
{
// Set real inmem client to proxy
updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
- boost::shared_ptr<isc::datasrc::ConfigurableClientList>
- list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
- false));
- server.setClientList(RRClass::IN(), list);
+ {
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+ boost::shared_ptr<isc::datasrc::ConfigurableClientList>
+ list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
+ false));
+ server.setClientList(RRClass::IN(), list);
+ }
createDataFromFile("nsec3query_nodnssec_fromWire.wire");
server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1449,6 +1452,7 @@ setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception,
{
updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
+ isc::util::thread::Mutex::Locker locker(server->getClientListMutex());
boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list(new FakeList(server->getClientList(RRClass::IN()), throw_when,
isc_exception, rrset));
@@ -1761,6 +1765,10 @@ TEST_F(AuthSrvTest, DDNSForwardCreateDestroy) {
// 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.getClientListMutex());
// The lists don't exist. Therefore, the list of RRClasses is empty.
// We also have no IN list.
EXPECT_TRUE(server.getClientListClasses().empty());
diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc
index a22725e..e57de93 100644
--- a/src/bin/auth/tests/command_unittest.cc
+++ b/src/bin/auth/tests/command_unittest.cc
@@ -174,6 +174,7 @@ 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())->
find(Name("ns.test1.example")).finder_->
find(Name("ns.test1.example"), RRType::A())->code);
@@ -214,6 +215,7 @@ configureZones(AuthSrv& server) {
void
newZoneChecks(AuthSrv& server) {
+ isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
find(Name("ns.test1.example")).finder_->
find(Name("ns.test1.example"), RRType::A())->code);
@@ -271,30 +273,33 @@ TEST_F(AuthCommandTest,
"}]}"));
DataSourceConfigurator::testReconfigure(&server_, config);
- // Check that the A record at www.example.org does not exist
- EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("www.example.org"), RRType::A())->code);
-
- // Add the record to the underlying sqlite database, by loading
- // it as a separate datasource, and updating it
- ConstElementPtr sql_cfg = Element::fromJSON("{ \"type\": \"sqlite3\","
- "\"database_file\": \""
- + test_db + "\"}");
- DataSourceClientContainer sql_ds("sqlite3", sql_cfg);
- ZoneUpdaterPtr sql_updater =
- sql_ds.getInstance().getUpdater(Name("example.org"), false);
- RRsetPtr rrset(new RRset(Name("www.example.org."), RRClass::IN(),
- RRType::A(), RRTTL(60)));
- rrset->addRdata(rdata::createRdata(rrset->getType(),
- rrset->getClass(),
- "192.0.2.1"));
- sql_updater->addRRset(*rrset);
- sql_updater->commit();
-
- EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("www.example.org"), RRType::A())->code);
+ {
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+ // Check that the A record at www.example.org does not exist
+ EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("www.example.org"), RRType::A())->code);
+
+ // Add the record to the underlying sqlite database, by loading
+ // it as a separate datasource, and updating it
+ ConstElementPtr sql_cfg = Element::fromJSON("{ \"type\": \"sqlite3\","
+ "\"database_file\": \""
+ + test_db + "\"}");
+ DataSourceClientContainer sql_ds("sqlite3", sql_cfg);
+ ZoneUpdaterPtr sql_updater =
+ sql_ds.getInstance().getUpdater(Name("example.org"), false);
+ RRsetPtr rrset(new RRset(Name("www.example.org."), RRClass::IN(),
+ RRType::A(), RRTTL(60)));
+ rrset->addRdata(rdata::createRdata(rrset->getType(),
+ rrset->getClass(),
+ "192.0.2.1"));
+ sql_updater->addRRset(*rrset);
+ sql_updater->commit();
+
+ EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("www.example.org"), RRType::A())->code);
+ }
// Now send the command to reload it
result_ = execAuthServerCommand(server_, "loadzone",
@@ -302,20 +307,26 @@ TEST_F(AuthCommandTest,
"{\"origin\": \"example.org\"}"));
checkAnswer(0, "Successful load");
- // And now it should be present too.
- EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("www.example.org"), RRType::A())->code);
+ {
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+ // And now it should be present too.
+ EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("www.example.org"), RRType::A())->code);
+ }
// Some error cases. First, the zone has no configuration. (note .com here)
result_ = execAuthServerCommand(server_, "loadzone",
Element::fromJSON("{\"origin\": \"example.com\"}"));
checkAnswer(1, "example.com");
- // The previous zone is not hurt in any way
- EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
- find(Name("example.org")).finder_->
- find(Name("example.org"), RRType::SOA())->code);
+ {
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+ // The previous zone is not hurt in any way
+ EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+ find(Name("example.org")).finder_->
+ find(Name("example.org"), RRType::SOA())->code);
+ }
const ConstElementPtr config2(Element::fromJSON("{"
"\"IN\": [{"
@@ -331,6 +342,7 @@ TEST_F(AuthCommandTest,
Element::fromJSON("{\"origin\": \"example.com\"}"));
checkAnswer(1, "Unreadable");
+ isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
// The previous zone is not hurt in any way
EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
find(Name("example.org")).finder_->
More information about the bind10-changes
mailing list