From: Michal 'vorner' Vaner Date: Wed, 5 Sep 2012 08:43:51 +0000 (+0200) Subject: [2202] Check the mutex is locked when it should X-Git-Tag: trac2402_base~79^2~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e25a0b2202650ef749340a1993b427bf99d962e5;p=thirdparty%2Fkea.git [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 07fd5e7bfe..9f6becff20 100644 --- a/src/bin/auth/auth_srv.cc +++ b/src/bin/auth/auth_srv.cc @@ -264,6 +264,10 @@ public: boost::shared_ptr getClientList(const RRClass& rrclass) { + // TODO: Debug-build only check + if (!mutex_.locked()) { + isc_throw(isc::Unexpected, "Not locked!"); + } const std::map >:: 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& 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 AuthSrv::getClientListClasses() const { + // TODO: Debug-build only check + if (!impl_->mutex_.locked()) { + isc_throw(isc::Unexpected, "Not locked"); + } + vector result; for (std::map >:: 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 a464f62047..de1a9ec29f 100644 --- 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 - 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 + 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 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 a22725eb1a..e57de93d67 100644 --- 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_->