[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 38bf162539e7dc1a6406f3762f03c0b1c470a6a2..fc33180375243f37f47538d5fffa9b53511bbbb8 100644 (file)
--- 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 585e7c30a49498172c18d0b28cec4a88595b3a0f..04dd0889a2182b60c16ba61bba7e329c18accdad 100644 (file)
--- 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);
 }