BIND 10 trac2202, updated. 10a4833ef74f0cf9aebc3861aa69cae6a25ce1cb [2202] Add a mutex to the AuthSrv

[BIND 10 source code commits]

The branch, trac2202 has been updated
       via  10a4833ef74f0cf9aebc3861aa69cae6a25ce1cb (commit)
      from  35d60f15a86733bb3bc9db77b3d8c7713d83d70d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 10a4833ef74f0cf9aebc3861aa69cae6a25ce1cb
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Sep 4 12:08:14 2012 +0200

    [2202] Add a mutex to the AuthSrv
    
    To be used when accessing the client lists.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/auth/Makefile.am                |    1 +
 src/bin/auth/auth_srv.cc                |    8 +++++++
 src/bin/auth/auth_srv.h                 |   38 +++++++++++++++++++++++++++++++
 src/bin/auth/tests/Makefile.am          |    1 +
 src/bin/auth/tests/auth_srv_unittest.cc |   11 +++++++++
 5 files changed, 59 insertions(+)

-----------------------------------------------------------------------
diff --git a/src/bin/auth/Makefile.am b/src/bin/auth/Makefile.am
index 47bed15..112778d 100644
--- a/src/bin/auth/Makefile.am
+++ b/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/server_common/libb10-server-common.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)
 
 # TODO: config.h.in is wrong because doesn't honor pkgdatadir
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 6e5666f..26dfada 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -26,6 +26,7 @@
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
+#include <util/threads/lock.h>
 
 #include <dns/edns.h>
 #include <dns/exceptions.h>
@@ -300,6 +301,8 @@ public:
                       isc::dns::Message& message,
                       bool done);
 
+    mutable util::thread::Mutex mutex_;
+
 private:
     bool xfrout_connected_;
     AbstractXfroutClient& xfrout_client_;
@@ -927,3 +930,8 @@ AuthSrv::getClientListClasses() const {
     }
     return (result);
 }
+
+util::thread::Mutex&
+AuthSrv::getClientListMutex() const {
+    return (impl_->mutex_);
+}
diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h
index 2c2b415..8f2fcec 100644
--- a/src/bin/auth/auth_srv.h
+++ b/src/bin/auth/auth_srv.h
@@ -40,6 +40,9 @@ namespace util {
 namespace io {
 class BaseSocketSessionForwarder;
 }
+namespace thread {
+class Mutex;
+}
 }
 namespace datasrc {
 class ConfigurableClientList;
@@ -319,6 +322,41 @@ public:
     ///     has been set by setClientList.
     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:
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;
diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am
index f87ed4c..83b3a90 100644
--- a/src/bin/auth/tests/Makefile.am
+++ b/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/statistics/libb10-statistics.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 += $(SQLITE_LIBS)
 
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index ca42473..a464f62 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -38,6 +38,7 @@
 #include <auth/datasrc_configurator.h>
 
 #include <util/unittests/mock_socketsession.h>
+#include <util/threads/lock.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>
@@ -1790,4 +1791,14 @@ TEST_F(AuthSrvTest, clientList) {
     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);
+}
+
 }