Browse Source

[2202] Add a mutex to the AuthSrv

To be used when accessing the client lists.
Michal 'vorner' Vaner 12 years ago
parent
commit
10a4833ef7

+ 1 - 0
src/bin/auth/Makefile.am

@@ -73,6 +73,7 @@ b10_auth_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
 b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libb10-xfr.la
 b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libb10-xfr.la
 b10_auth_LDADD += $(top_builddir)/src/lib/server_common/libb10-server-common.la
 b10_auth_LDADD += $(top_builddir)/src/lib/server_common/libb10-server-common.la
 b10_auth_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
 b10_auth_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
+b10_auth_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
 b10_auth_LDADD += $(SQLITE_LIBS)
 b10_auth_LDADD += $(SQLITE_LIBS)
 
 
 # TODO: config.h.in is wrong because doesn't honor pkgdatadir
 # TODO: config.h.in is wrong because doesn't honor pkgdatadir

+ 8 - 0
src/bin/auth/auth_srv.cc

@@ -26,6 +26,7 @@
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
 #include <util/buffer.h>
 #include <util/buffer.h>
+#include <util/threads/lock.h>
 
 
 #include <dns/edns.h>
 #include <dns/edns.h>
 #include <dns/exceptions.h>
 #include <dns/exceptions.h>
@@ -300,6 +301,8 @@ public:
                       isc::dns::Message& message,
                       isc::dns::Message& message,
                       bool done);
                       bool done);
 
 
+    mutable util::thread::Mutex mutex_;
+
 private:
 private:
     bool xfrout_connected_;
     bool xfrout_connected_;
     AbstractXfroutClient& xfrout_client_;
     AbstractXfroutClient& xfrout_client_;
@@ -927,3 +930,8 @@ AuthSrv::getClientListClasses() const {
     }
     }
     return (result);
     return (result);
 }
 }
+
+util::thread::Mutex&
+AuthSrv::getClientListMutex() const {
+    return (impl_->mutex_);
+}

+ 38 - 0
src/bin/auth/auth_srv.h

@@ -40,6 +40,9 @@ namespace util {
 namespace io {
 namespace io {
 class BaseSocketSessionForwarder;
 class BaseSocketSessionForwarder;
 }
 }
+namespace thread {
+class Mutex;
+}
 }
 }
 namespace datasrc {
 namespace datasrc {
 class ConfigurableClientList;
 class ConfigurableClientList;
@@ -319,6 +322,41 @@ public:
     ///     has been set by setClientList.
     ///     has been set by setClientList.
     std::vector<isc::dns::RRClass> getClientListClasses() const;
     std::vector<isc::dns::RRClass> getClientListClasses() const;
 
 
+    /**
+     * \brief Return a mutex for the client lists.
+     *
+     * Background loading of data uses threads. Therefore we need to protect
+     * the client lists by a mutex, so they don't change (or get destroyed)
+     * during query processing. Get (and lock) this mutex whenever you do
+     * something with the lists and keep it locked until you finish. This
+     * is correct:
+     * \code
+    {
+      Mutex::Locker locker(auth->getClientListMutex());
+      boost::shared_ptr<isc::datasrc::ConfigurableClientList>
+        list(auth->getClientList(RRClass::IN()));
+      // Do some processing here
+    }
+    \endcode
+     *
+     * But this is not (it releases the mutex too soon):
+     * \code
+    boost::shared_ptr<isc::datasrc::ConfigurableClientList>
+      list;
+    {
+      Mutex::Locker locker(auth->getClientListMutex());
+      list = auth->getClientList(RRClass::IN()));
+    }
+    // Do some processing here
+     * \endcode
+     *
+     * \note This method is const even if you are allowed to modify
+     *    (lock) the mutex. It's because locking of the mutex is not really
+     *    a modification of the server object and it is needed to protect the
+     *    lists even on read-only operations.
+     */
+    isc::util::thread::Mutex& getClientListMutex() const;
+
 private:
 private:
     AuthSrvImpl* impl_;
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;
     isc::asiolink::SimpleCallback* checkin_;

+ 1 - 0
src/bin/auth/tests/Makefile.am

@@ -80,6 +80,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libb10-nsas.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
 run_unittests_LDADD += $(top_builddir)/src/lib/statistics/libb10-statistics.la
 run_unittests_LDADD += $(top_builddir)/src/lib/config/tests/libfake_session.la
 run_unittests_LDADD += $(top_builddir)/src/lib/config/tests/libfake_session.la
+run_unittests_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
 run_unittests_LDADD += $(GTEST_LDADD)
 run_unittests_LDADD += $(GTEST_LDADD)
 run_unittests_LDADD += $(SQLITE_LIBS)
 run_unittests_LDADD += $(SQLITE_LIBS)
 
 

+ 11 - 0
src/bin/auth/tests/auth_srv_unittest.cc

@@ -38,6 +38,7 @@
 #include <auth/datasrc_configurator.h>
 #include <auth/datasrc_configurator.h>
 
 
 #include <util/unittests/mock_socketsession.h>
 #include <util/unittests/mock_socketsession.h>
+#include <util/threads/lock.h>
 #include <dns/tests/unittest_util.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>
 #include <testutils/srv_test.h>
@@ -1790,4 +1791,14 @@ TEST_F(AuthSrvTest, clientList) {
     EXPECT_EQ(list, server.getClientList(RRClass::IN()));
     EXPECT_EQ(list, server.getClientList(RRClass::IN()));
 }
 }
 
 
+// We just test the mutex can be locked (exactly once).
+TEST_F(AuthSrvTest, mutex) {
+    isc::util::thread::Mutex::Locker l1(server.getClientListMutex());
+    // TODO: Once we have non-debug build, this one will not work.
+    // Skip then.
+    EXPECT_THROW({
+        isc::util::thread::Mutex::Locker l2(server.getClientListMutex());
+    }, isc::InvalidOperation);
+}
+
 }
 }