[2202] Check the mutex is locked when it should

And update tests so they lock.

diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 07fd5e7bfe80a907d99cf1f6ad17c19c90776dc8..9f6becff20614c7383a15bb170a2906732699c64 100644 (file)
--- 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 a464f6204792cbbf65c0ddd91f49dd892db11aa2..de1a9ec29f8f78449444bf0bcbf26c60365894a9 100644 (file)
--- 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 a22725eb1a1a00db4f68f4022f27ea02838825f4..e57de93d67575e928341d92366ddb6feae43e996 100644 (file)
--- 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_->