From: JINMEI Tatuya <jinmei@isc.org>
Date: Wed, 7 Nov 2012 05:20:15 +0000 (-0800)
Subject: [2459] protect the call to getCachedZoneWriter() by mutex. it can cause race.
X-Git-Tag: trac2487_base~1^2~19^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aaf061f2c3f18d7e1cdc09c71ecb8820a107e64c;p=thirdparty%2Fkea.git

[2459] protect the call to getCachedZoneWriter() by mutex. it can cause race.
---

diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h
index 38bf162539..fc33180375 100644
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -614,8 +614,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:
diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
index 585e7c30a4..04dd0889a2 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);
 }