[2198] Add Mutex::tryLock() and non-blocking variant of Mutex::Locker

This is necessary for testing that a mutex was used in code.  I've
also added comments to discourage the use of the lock methods
directly and use the Mutex::Locker where applicable. The Mutex lock
methods are currently private.

diff --git a/src/lib/util/threads/sync.cc b/src/lib/util/threads/sync.cc
index 46a56465a4e21049ce63612ad94ed464371d1500..0e0ec08b867c1f1b839c2e9f508e470c9edb0010 100644 (file)
--- a/src/lib/util/threads/sync.cc
+++ b/src/lib/util/threads/sync.cc
@@ -169,6 +169,26 @@ Mutex::lock() {
 #endif // ENABLE_DEBUG
 }
 
+bool
+Mutex::tryLock() {
+    assert(impl_ != NULL);
+    const int result = pthread_mutex_trylock(&impl_->mutex);
+    // In the case of pthread_mutex_trylock(), if it is called on a
+    // locked mutex from the same thread, some platforms (such as fedora
+    // and debian) return EBUSY whereas others (such as centos 5) return
+    // EDEADLK. We return false and don't pass the lock attempt in both
+    // cases.
+    if (result == EBUSY || result == EDEADLK) {
+        return (false);
+    } else if (result != 0) {
+        isc_throw(isc::InvalidOperation, std::strerror(result));
+    }
+#ifdef ENABLE_DEBUG
+    postLockAction();           // Only in debug mode
+#endif // ENABLE_DEBUG
+    return (true);
+}
+
 void
 Mutex::unlock() {
     assert(impl_ != NULL);

diff --git a/src/lib/util/threads/sync.h b/src/lib/util/threads/sync.h
index 87c78be94d5f505b7802fadb1edf89e0581407f6..4da5ef139c2c1f89b5a2669561932c41f037dee8 100644 (file)
--- a/src/lib/util/threads/sync.h
+++ b/src/lib/util/threads/sync.h
@@ -15,6 +15,8 @@
 #ifndef B10_THREAD_SYNC_H
 #define B10_THREAD_SYNC_H
 
+#include <exceptions/exceptions.h>
+
 #include <boost/noncopyable.hpp>
 
 #include <cstdlib> // for NULL.
@@ -77,17 +79,34 @@ public:
     /// of function no matter by what means.
     class Locker : 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_(mutex)
         {
-            mutex.lock();
+            if (block) {
+                mutex.lock();
+            } else {
+                if (!mutex.tryLock()) {
+                    isc_throw(AlreadyLocked, "The mutex is already locked");
+                }
+            }
         }
 
         /// \brief Destructor.
@@ -107,6 +126,33 @@ public:
     ///
     /// \todo Disable in non-debug build
     bool locked() const;
+
+private:
+    /// \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:
     friend class CondVar;
 
@@ -131,8 +177,6 @@ private:
 
     class Impl;
     Impl* impl_;
-    void lock();
-    void unlock();
 };
 
 /// \brief Encapsulation for a condition variable.

diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
index 4c4f831876024d46ff141416301aae78985ba8a8..c17999ebdbfdb0ea3376f157ba3ab4dfe015a342 100644 (file)
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -44,6 +44,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();
 }
 
 #endif // ENABLE_DEBUG