BIND 10 master, updated. f8181bb72251fd3d5f33e157d1f2a2a75a58d765 Merge branch 'master' into trac2198_2

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Oct 16 01:01:55 UTC 2012


The branch, master has been updated
       via  f8181bb72251fd3d5f33e157d1f2a2a75a58d765 (commit)
       via  6adfc244341c07a70eb0984536390486649316ae (commit)
       via  5b781744ac6f909958754f7e5947115ec799ac18 (commit)
       via  4ed683ce42d061905a4e5b06fcc1a38c64136e7a (commit)
       via  e6ac5ef47d9885e20e247697c8c3d1fe55fd2500 (commit)
       via  48e80a5c7505df708999ef817911d1eb505deaad (commit)
       via  fff8929b13d8c2cd725b861c7e930c0060b54d83 (commit)
       via  ecef170dd0af4f282d4426ef663b667c76873737 (commit)
       via  350c775f7bd5c7777f0345754dac4ecaa0a9af18 (commit)
       via  666bfa9c7b6de4ee31e210d7f3e0f2f68704f2de (commit)
       via  d042cd04c0784641738a0f06e5a6b193b433d39f (commit)
       via  c0d25c429a4f2026bb97b9e013730b8f265862e6 (commit)
       via  e71cd13c5cb261635f7115e0d30fbd7dcc5fe151 (commit)
       via  66424553f3fa05e46e558f3b4e16677463e0e412 (commit)
       via  e562ac798865d18ece6a7ddd28ed79d8b4abe0ae (commit)
      from  453b58784344390cbc5375624e6e92a86b4ff3f1 (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 f8181bb72251fd3d5f33e157d1f2a2a75a58d765
Merge: 6adfc24 453b587
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Oct 16 06:01:22 2012 +0530

    Merge branch 'master' into trac2198_2

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

Summary of changes:
 src/lib/util/Makefile.am                           |    6 +-
 src/lib/util/interprocess_sync_file.cc             |   74 ++++++++++++++++++--
 src/lib/util/interprocess_sync_file.h              |   12 +++-
 src/lib/util/tests/Makefile.am                     |    1 +
 .../util/tests/interprocess_sync_file_unittest.cc  |   47 ++++++++++++-
 src/lib/util/threads/lock.cc                       |   13 ++++
 src/lib/util/threads/lock.h                        |   55 +++++++++++++--
 src/lib/util/threads/tests/lock_unittest.cc        |   38 ++++++++++
 src/lib/util/unittests/Makefile.am                 |    3 +-
 9 files changed, 232 insertions(+), 17 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am
index 13f8f7b..99b54eb 100644
--- a/src/lib/util/Makefile.am
+++ b/src/lib/util/Makefile.am
@@ -1,4 +1,5 @@
-SUBDIRS = . io unittests tests pyunittests python threads
+# InterprocessSyncFile has a dependency on isc::util::thread::Mutex
+SUBDIRS = io unittests threads . tests pyunittests python
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/util -I$(top_builddir)/src/lib/util
@@ -31,7 +32,8 @@ libb10_util_la_SOURCES += random/random_number_generator.h
 
 EXTRA_DIST = python/pycppwrapper_util.h
 
-libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+libb10_util_la_LIBADD =  $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+libb10_util_la_LIBADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
 CLEANFILES = *.gcno *.gcda
 
 libb10_util_includedir = $(includedir)/$(PACKAGE_NAME)/util
diff --git a/src/lib/util/interprocess_sync_file.cc b/src/lib/util/interprocess_sync_file.cc
index d045449..6da3f14 100644
--- a/src/lib/util/interprocess_sync_file.cc
+++ b/src/lib/util/interprocess_sync_file.cc
@@ -14,6 +14,9 @@
 
 #include "interprocess_sync_file.h"
 
+#include <boost/weak_ptr.hpp>
+
+#include <map>
 #include <string>
 
 #include <stdlib.h>
@@ -23,9 +26,38 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+using namespace isc::util::thread;
+
 namespace isc {
 namespace util {
 
+namespace { // unnamed namespace
+
+typedef std::map<std::string, boost::weak_ptr<Mutex> > SyncMap;
+
+Mutex sync_map_mutex;
+SyncMap sync_map;
+
+} // end of unnamed namespace
+
+InterprocessSyncFile::InterprocessSyncFile(const std::string& task_name) :
+    InterprocessSync(task_name),
+    fd_(-1)
+{
+    Mutex::Locker locker(sync_map_mutex);
+
+    SyncMap::iterator it = sync_map.find(task_name);
+    if (it != sync_map.end()) {
+        mutex_ = it->second.lock();
+    } else {
+        mutex_.reset(new Mutex());
+        sync_map[task_name] = mutex_;
+    }
+
+    // Lock on sync_map_mutex is automatically unlocked during
+    // destruction when basic block is exited.
+}
+
 InterprocessSyncFile::~InterprocessSyncFile() {
     if (fd_ != -1) {
         // This will also release any applied locks.
@@ -33,6 +65,23 @@ InterprocessSyncFile::~InterprocessSyncFile() {
         // The lockfile will continue to exist, and we must not delete
         // it.
     }
+
+    Mutex::Locker locker(sync_map_mutex);
+
+    // Unref the shared mutex.
+    locker_.reset();
+    mutex_.reset();
+
+    // Remove name from the map if it is unused anymore.
+    SyncMap::iterator it = sync_map.find(task_name_);
+    assert(it != sync_map.end());
+
+    if (it->second.expired()) {
+        sync_map.erase(it);
+    }
+
+    // Lock on sync_map_mutex is automatically unlocked during
+    // destruction when basic block is exited.
 }
 
 bool
@@ -90,8 +139,13 @@ InterprocessSyncFile::lock() {
         return (true);
     }
 
+    // First grab the thread lock...
+    LockerPtr locker(new Mutex::Locker(*mutex_));
+
+    // ... then the file lock.
     if (do_lock(F_SETLKW, F_WRLCK)) {
         is_locked_ = true;
+        locker_ = locker;
         return (true);
     }
 
@@ -104,8 +158,18 @@ InterprocessSyncFile::tryLock() {
         return (true);
     }
 
+    // First grab the thread lock...
+    LockerPtr locker;
+    try {
+        locker.reset(new Mutex::Locker(*mutex_, false));
+    } catch (const Mutex::Locker::AlreadyLocked&) {
+        return (false);
+    }
+
+    // ... then the file lock.
     if (do_lock(F_SETLK, F_WRLCK)) {
         is_locked_ = true;
+        locker_ = locker;
         return (true);
     }
 
@@ -118,12 +182,14 @@ InterprocessSyncFile::unlock() {
         return (true);
     }
 
-    if (do_lock(F_SETLKW, F_UNLCK)) {
-        is_locked_ = false;
-        return (true);
+    // First release the file lock...
+    if (do_lock(F_SETLKW, F_UNLCK) == 0) {
+        return (false);
     }
 
-    return (false);
+    locker_.reset();
+    is_locked_ = false;
+    return (true);
 }
 
 } // namespace util
diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h
index fd8da1b..5cb64c1 100644
--- a/src/lib/util/interprocess_sync_file.h
+++ b/src/lib/util/interprocess_sync_file.h
@@ -16,8 +16,11 @@
 #define __INTERPROCESS_SYNC_FILE_H__
 
 #include <util/interprocess_sync.h>
+#include <util/threads/lock.h>
 #include <exceptions/exceptions.h>
 
+#include <boost/shared_ptr.hpp>
+
 namespace isc {
 namespace util {
 
@@ -55,9 +58,7 @@ public:
     /// \param name Name of the synchronization task. This has to be
     /// identical among the various processes that need to be
     /// synchronized for the same task.
-    InterprocessSyncFile(const std::string& task_name) :
-        InterprocessSync(task_name), fd_(-1)
-    {}
+    InterprocessSyncFile(const std::string& task_name);
 
     /// \brief Destructor
     virtual ~InterprocessSyncFile();
@@ -83,6 +84,11 @@ private:
     bool do_lock(int cmd, short l_type);
 
     int fd_; ///< The descriptor for the open file
+
+    typedef boost::shared_ptr<isc::util::thread::Mutex> MutexPtr;
+    typedef boost::shared_ptr<isc::util::thread::Mutex::Locker> LockerPtr;
+    MutexPtr mutex_;   ///< A mutex for mutual exclusion among threads
+    LockerPtr locker_; ///< A locker on mutex_
 };
 
 } // namespace util
diff --git a/src/lib/util/tests/Makefile.am b/src/lib/util/tests/Makefile.am
index 105322f..1a9c96a 100644
--- a/src/lib/util/tests/Makefile.am
+++ b/src/lib/util/tests/Makefile.am
@@ -46,6 +46,7 @@ run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 
 run_unittests_LDADD = $(top_builddir)/src/lib/util/libb10-util.la
+run_unittests_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/io/libb10-util-io.la
 run_unittests_LDADD += \
 	$(top_builddir)/src/lib/util/unittests/libutil_unittests.la
diff --git a/src/lib/util/tests/interprocess_sync_file_unittest.cc b/src/lib/util/tests/interprocess_sync_file_unittest.cc
index 9a1b025..242cc5b 100644
--- a/src/lib/util/tests/interprocess_sync_file_unittest.cc
+++ b/src/lib/util/tests/interprocess_sync_file_unittest.cc
@@ -12,10 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include "util/interprocess_sync_file.h"
+#include <util/interprocess_sync_file.h>
+#include <util/threads/thread.h>
+
 #include <gtest/gtest.h>
+#include <boost/bind.hpp>
+
 #include <unistd.h>
 
+using namespace isc::util::thread;
 using namespace std;
 
 namespace isc {
@@ -108,6 +113,46 @@ TEST(InterprocessSyncFileTest, TestLock) {
   EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
 }
 
+void
+threadTest(bool* locked)
+{
+    InterprocessSyncFile sync2("test");
+    InterprocessSyncLocker locker2(sync2);
+
+    *locked = !locker2.tryLock();
+}
+
+TEST(InterprocessSyncFileTest, TestLockThreaded) {
+    InterprocessSyncFile sync("test");
+    InterprocessSyncLocker locker(sync);
+
+    EXPECT_FALSE(locker.isLocked());
+    EXPECT_TRUE(locker.lock());
+    EXPECT_TRUE(locker.isLocked());
+
+    bool locked_in_other_thread = false;
+
+    // Here, we check that a lock has been taken by creating a new
+    // thread and checking from the new thread that a lock exists. The
+    // lock attempt must fail to pass our check.
+    Thread thread(boost::bind(&threadTest, &locked_in_other_thread));
+    thread.wait();
+
+    EXPECT_TRUE(locked_in_other_thread);
+
+    // Release the lock and try again. This time, the attempt must pass.
+
+    EXPECT_TRUE(locker.unlock());
+    EXPECT_FALSE(locker.isLocked());
+
+    Thread thread2(boost::bind(&threadTest, &locked_in_other_thread));
+    thread2.wait();
+
+    EXPECT_FALSE(locked_in_other_thread);
+
+    EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
+}
+
 TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
   InterprocessSyncFile sync("test1");
   InterprocessSyncLocker locker(sync);
diff --git a/src/lib/util/threads/lock.cc b/src/lib/util/threads/lock.cc
index 6a165aa..acc73d6 100644
--- a/src/lib/util/threads/lock.cc
+++ b/src/lib/util/threads/lock.cc
@@ -119,6 +119,19 @@ Mutex::lock() {
     ++impl_->locked_count; // Only in debug mode
 }
 
+bool
+Mutex::tryLock() {
+    assert(impl_ != NULL);
+    const int result = pthread_mutex_trylock(&impl_->mutex);
+    if (result == EBUSY) {
+        return (false);
+    } else if (result != 0) {
+        isc_throw(isc::InvalidOperation, std::strerror(result));
+    }
+    ++impl_->locked_count; // Only in debug mode
+    return (true);
+}
+
 void
 Mutex::unlock() {
     assert(impl_ != NULL);
diff --git a/src/lib/util/threads/lock.h b/src/lib/util/threads/lock.h
index fef537b..4992bdb 100644
--- a/src/lib/util/threads/lock.h
+++ b/src/lib/util/threads/lock.h
@@ -15,6 +15,7 @@
 #ifndef B10_THREAD_LOCK_H
 #define B10_THREAD_LOCK_H
 
+#include <exceptions/exceptions.h>
 #include <boost/noncopyable.hpp>
 
 #include <cstdlib> // for NULL.
@@ -76,21 +77,39 @@ public:
     /// of function no matter by what means.
     class Locker : public boost::noncopyable {
     public:
+        /// \brief Exception thrown when the mutex is already locked and
+        ///     a non-blocking locker is attempted around it.
+        struct AlreadyLocked : public isc::InvalidParameter {
+            AlreadyLocked(const char* file, size_t line, const char* what) :
+                isc::InvalidParameter(file, line, what)
+            {}
+        };
+
         /// \brief Constructor.
         ///
-        /// Locks the mutex. May block for extended period of time.
+        /// Locks the mutex. May block for extended period of time if
+        /// \c block is true.
         ///
         /// \throw isc::InvalidOperation when OS reports error. This usually
         ///     means an attempt to use the mutex in a wrong way (locking
         ///     a mutex second time from the same thread, for example).
-        Locker(Mutex& mutex) :
+        /// \throw AlreadyLocked if \c block is false and the mutex is
+        ///     already locked.
+        Locker(Mutex& mutex, bool block = true) :
             mutex_(NULL)
         {
             // Set the mutex_ after we acquire the lock. This is because of
             // exception safety. If lock() throws, it didn't work, so we must
             // not unlock when we are destroyed. In such case, mutex_ is
             // NULL and checked in the destructor.
-            mutex.lock();
+            if (block) {
+                mutex.lock();
+            } else {
+                if (!mutex.tryLock()) {
+                    isc_throw(AlreadyLocked, "The mutex is already locked");
+                }
+            }
+
             mutex_ = &mutex;
         }
 
@@ -105,6 +124,7 @@ public:
     private:
         Mutex* mutex_;
     };
+
     /// \brief If the mutex is currently locked
     ///
     /// This is debug aiding method only. And it might be unavailable in
@@ -113,11 +133,36 @@ public:
     ///
     /// \todo Disable in non-debug build
     bool locked() const;
+
 private:
-    class Impl;
-    Impl* impl_;
+    /// \brief Lock the mutex
+    ///
+    /// This method blocks until the mutex can be locked.
+    ///
+    /// Please consider not using this method directly and instead using
+    /// a Mutex::Locker object instead.
     void lock();
+
+    /// \brief Try to lock the mutex
+    ///
+    /// This method doesn't block and returns immediately with a status
+    /// on whether the lock operation was successful.
+    ///
+    /// Please consider not using this method directly and instead using
+    /// a Mutex::Locker object instead.
+    ///
+    /// \return true if the lock was successful, false otherwise.
+    bool tryLock();
+
+    /// \brief Unlock the mutex
+    ///
+    /// Please consider not using this method directly and instead using
+    /// a Mutex::Locker object instead.
     void unlock();
+
+private:
+    class Impl;
+    Impl* impl_;
 };
 
 
diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
index 0b4d3ce..8d337d5 100644
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -37,6 +37,44 @@ TEST(MutexTest, lockMultiple) {
         Mutex::Locker l2(mutex); // Attempt to lock again.
     }, isc::InvalidOperation);
     EXPECT_TRUE(mutex.locked()); // Debug-only build
+
+    // block=true explicitly.
+    Mutex mutex2;
+    EXPECT_FALSE(mutex2.locked()); // Debug-only build
+    Mutex::Locker l12(mutex2, true);
+    EXPECT_TRUE(mutex2.locked()); // Debug-only build
+}
+
+void
+testThread(Mutex* mutex)
+{
+    // block=false (tryLock).  This should not block indefinitely, but
+    // throw AlreadyLocked. If block were true, this would block
+    // indefinitely here.
+    EXPECT_THROW({
+        Mutex::Locker l3(*mutex, false);
+    }, Mutex::Locker::AlreadyLocked);
+
+    EXPECT_TRUE(mutex->locked()); // Debug-only build
+}
+
+// Test the non-blocking variant using a second thread.
+TEST(MutexTest, lockNonBlocking) {
+    // block=false (tryLock).
+    Mutex mutex;
+    Mutex::Locker l1(mutex, false);
+    EXPECT_TRUE(mutex.locked()); // Debug-only build
+
+    // First, try another locker from the same thread.
+    EXPECT_THROW({
+        Mutex::Locker l2(mutex, false);
+    }, Mutex::Locker::AlreadyLocked);
+
+    EXPECT_TRUE(mutex.locked()); // Debug-only build
+
+    // Now try another locker from a different thread.
+    Thread thread(boost::bind(&testThread, &mutex));
+    thread.wait();
 }
 
 // Destroying a locked mutex is a bad idea as well
diff --git a/src/lib/util/unittests/Makefile.am b/src/lib/util/unittests/Makefile.am
index 451ab4e..6caa6bf 100644
--- a/src/lib/util/unittests/Makefile.am
+++ b/src/lib/util/unittests/Makefile.am
@@ -20,8 +20,7 @@ if HAVE_GTEST
 libutil_unittests_la_CPPFLAGS += $(GTEST_INCLUDES)
 endif
 
-libutil_unittests_la_LIBADD  = $(top_builddir)/src/lib/util/libb10-util.la
-libutil_unittests_la_LIBADD += $(top_builddir)/src/lib/util/io/libb10-util-io.la
+libutil_unittests_la_LIBADD  = $(top_builddir)/src/lib/util/io/libb10-util-io.la
 libutil_unittests_la_LIBADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 
 CLEANFILES = *.gcno *.gcda



More information about the bind10-changes mailing list