BIND 10 master, updated. 36378804a28850cb882bbc087c819fcabb5ada16 [master] Merge branch 'trac2459'
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Nov 14 06:25:48 UTC 2012
The branch, master has been updated
via 36378804a28850cb882bbc087c819fcabb5ada16 (commit)
via 8f4e6ef30b3078b8720b618efdacfb4fb186ad9f (commit)
via 38880a0a12721118c423dee2396ccfd165b103d0 (commit)
via aaf061f2c3f18d7e1cdc09c71ecb8820a107e64c (commit)
from 3f1afa7c181c802ba2dab143cc9b5f72e8e1de69 (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 36378804a28850cb882bbc087c819fcabb5ada16
Merge: 3f1afa7 8f4e6ef
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 13 22:14:41 2012 -0800
[master] Merge branch 'trac2459'
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth_messages.mes | 10 ++++++++++
src/bin/auth/auth_srv.cc | 10 +++++++---
src/bin/auth/datasrc_clients_mgr.h | 21 ++++++++++++++------
.../auth/tests/datasrc_clients_builder_unittest.cc | 21 ++++++++++++++------
4 files changed, 47 insertions(+), 15 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_messages.mes b/src/bin/auth/auth_messages.mes
index 3780499..ac9ffb9 100644
--- a/src/bin/auth/auth_messages.mes
+++ b/src/bin/auth/auth_messages.mes
@@ -98,6 +98,16 @@ This debug message is issued when the separate thread for maintaining data
source clients successfully loaded the named zone of the named class as a
result of the 'loadzone' command.
+% AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE_NOCACHE skipped loading zone %1/%2 due to no in-memory cache
+This debug message is issued when the separate thread for maintaining data
+source clients received a command to reload a zone but skipped it because
+the specified zone is not loaded in-memory (but served from an underlying
+data source). This could happen if the loadzone command is manually issued
+by a user but the zone name is misspelled, but a more likely cause is
+that the command is sent from another BIND 10 module (such as xfrin or DDNS).
+In the latter case it can be simply ignored because there is no need
+for explicit reloading.
+
% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_CONFIG_ERROR Error in data source configuration: %1
The thread for maintaining data source clients has received a command to
reconfigure, but the parameter data (the new configuration) contains an
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index dca8fd0..26a8489 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -651,9 +651,10 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
local_edns->setUDPSize(AuthSrvImpl::DEFAULT_LOCAL_UDPSIZE);
message.setEDNS(local_edns);
}
- // 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.
+
+ // Get access to data source client list through the holder and keep
+ // the holder until the processing and rendering is done to avoid
+ // race with any other thread(s) such as the background loader.
auth::DataSrcClientsMgr::Holder datasrc_holder(datasrc_clients_mgr_);
try {
@@ -688,6 +689,9 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
return (true);
// The message can contain some data from the locked resource. But outside
// this method, we touch only the RCode of it, so it should be safe.
+
+ // Lock on datasrc_clients_mgr_ acquired by datasrc_holder is
+ // released here upon its deletion.
}
bool
diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h
index 38bf162..5bbdb99 100644
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -581,6 +581,9 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::doLoadZone(
try {
boost::shared_ptr<datasrc::memory::ZoneWriter> zwriter =
getZoneWriter(*client_list, rrclass, origin);
+ if (!zwriter) {
+ return;
+ }
zwriter->load(); // this can take time but doesn't cause a race
{ // install() can cause a race and must be in a critical section
@@ -614,8 +617,14 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::getZoneWriter(
datasrc::ConfigurableClientList& client_list,
const dns::RRClass& rrclass, const dns::Name& origin)
{
- const datasrc::ConfigurableClientList::ZoneWriterPair writerpair =
- client_list.getCachedZoneWriter(origin);
+ // getCachedZoneWriter() could get access to an underlying data source
+ // that can cause a race condition with the main thread using that data
+ // source for lookup. So we need to protect the access here.
+ datasrc::ConfigurableClientList::ZoneWriterPair writerpair;
+ {
+ typename MutexType::Locker locker(*map_mutex_);
+ writerpair = client_list.getCachedZoneWriter(origin);
+ }
switch (writerpair.first) {
case datasrc::ConfigurableClientList::ZONE_SUCCESS:
@@ -626,8 +635,10 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::getZoneWriter(
<< "/" << rrclass << ": not found in any configured "
"data source.");
case datasrc::ConfigurableClientList::ZONE_NOT_CACHED:
- isc_throw(InternalCommandError, "failed to load zone " << origin
- << "/" << rrclass << ": not served from memory");
+ LOG_DEBUG(auth_logger, DBG_AUTH_OPS,
+ AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE_NOCACHE)
+ .arg(origin).arg(rrclass);
+ break; // return NULL below
case datasrc::ConfigurableClientList::CACHE_DISABLED:
// This is an internal error. Auth server must have the cache
// enabled.
@@ -636,8 +647,6 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::getZoneWriter(
"is somehow disabled");
}
- // all cases above should either return or throw, but some compilers
- // still need a return statement
return (boost::shared_ptr<datasrc::memory::ZoneWriter>());
}
} // namespace datasrc_clientmgr_internal
diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
index 585e7c3..838e032 100644
--- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
@@ -308,8 +308,12 @@ TEST_F(DataSrcClientsBuilderTest, loadZone) {
"{\"class\": \"IN\","
" \"origin\": \"test1.example\"}"));
EXPECT_TRUE(builder.handleCommand(loadzone_cmd));
- EXPECT_EQ(1, map_mutex.lock_count); // we should have acquired the lock
- EXPECT_EQ(1, map_mutex.unlock_count); // and released it.
+
+ // loadZone involves two critical sections: one for getting the zone
+ // writer, and one for actually updating the zone data. So the lock/unlock
+ // count should be incremented by 2.
+ EXPECT_EQ(2, map_mutex.lock_count);
+ EXPECT_EQ(2, map_mutex.unlock_count);
newZoneChecks(clients_map, rrclass);
}
@@ -381,7 +385,10 @@ TEST_F(DataSrcClientsBuilderTest,
find(Name("example.org")).finder_->
find(Name("example.org"), RRType::SOA())->code);
- // attempt of reloading a zone but in-memory cache is disabled.
+ // attempt of reloading a zone but in-memory cache is disabled. In this
+ // case the command is simply ignored.
+ const size_t orig_lock_count = map_mutex.lock_count;
+ const size_t orig_unlock_count = map_mutex.unlock_count;
const ConstElementPtr config2(Element::fromJSON("{"
"\"IN\": [{"
" \"type\": \"sqlite3\","
@@ -390,11 +397,13 @@ TEST_F(DataSrcClientsBuilderTest,
" \"cache-zones\": [\"example.org\"]"
"}]}"));
clients_map = configureDataSource(config2);
- EXPECT_THROW(builder.handleCommand(
+ builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}"))),
- TestDataSrcClientsBuilder::InternalCommandError);
+ " \"origin\": \"example.org\"}")));
+ // 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);
// basically impossible case: in-memory cache is completely disabled.
// In this implementation of manager-builder, this should never happen,
More information about the bind10-changes
mailing list