Browse Source

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

JINMEI Tatuya 12 years ago
parent
commit
aaf061f2c3

+ 8 - 2
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:

+ 6 - 2
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);
 }