[master] Merge branch 'trac2332'

commit.
 commit.

src/bin/auth/auth_srv.cc

@@ -26,7 +26,7 @@
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 
 #include <dns/edns.h>
 #include <dns/exceptions.h>

src/bin/auth/benchmarks/query_bench.cc

@@ -18,7 +18,7 @@
 #include <bench/benchmark_util.h>
 
 #include <util/buffer.h>
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 
 #include <dns/message.h>
 #include <dns/name.h>

src/bin/auth/command.cc

@@ -21,7 +21,7 @@
 #include <config/ccsession.h>
 #include <exceptions/exceptions.h>
 #include <dns/rrclass.h>
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 
 #include <string>
 

src/bin/auth/main.cc

@@ -18,7 +18,7 @@
 
 #include <util/buffer.h>
 #include <util/io/socketsession.h>
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>

src/bin/auth/tests/auth_srv_unittest.cc

@@ -39,7 +39,7 @@
 #include <auth/datasrc_config.h>
 
 #include <util/unittests/mock_socketsession.h>
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>

src/bin/auth/tests/command_unittest.cc

@@ -16,7 +16,7 @@
 
 #include "datasrc_util.h"
 
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>

src/bin/auth/tests/datasrc_config_unittest.cc

@@ -16,7 +16,7 @@
 
 #include <config/tests/fake_session.h>
 #include <config/ccsession.h>
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 
 #include <gtest/gtest.h>
 

src/lib/util/threads/Makefile.am

@@ -5,7 +5,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES) $(MULTITHREADING_FLAG)
 
 lib_LTLIBRARIES = libb10-threads.la
-libb10_threads_la_SOURCES  = lock.h lock.cc
+libb10_threads_la_SOURCES  = sync.h sync.cc
 libb10_threads_la_SOURCES += thread.h thread.cc
 libb10_threads_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 

src/lib/util/threads/lock.h

@@ -1,128 +0,0 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-#ifndef B10_THREAD_LOCK_H
-#define B10_THREAD_LOCK_H
-
-#include <boost/noncopyable.hpp>
-
-#include <cstdlib> // for NULL.
-
-namespace isc {
-namespace util {
-namespace thread {
-
-/// \brief Mutex with very simple interface
-///
-/// Since mutexes are very system dependant, we create our own wrapper around
-/// whatever is available on the system and hide it.
-///
-/// To use this mutex, create it and then lock and unlock it by creating the
-/// Mutex::Locker object.
-///
-/// Also, as mutex is a low-level system object, an error might happen at any
-/// operation with it. We convert many errors to the isc::InvalidOperation,
-/// since the errors usually happen only when used in a wrong way. Any methods
-/// or constructors in this class can throw. Allocation errors are converted
-/// to std::bad_alloc (for example when OS-dependant limit of mutexes is
-/// exceeded). Some errors which usually mean a programmer error abort the
-/// program, since there could be no safe way to recover from them.
-///
-/// The current interface is somewhat minimalistic. If we ever need more, we
-/// can add it later.
-class Mutex : public boost::noncopyable {
-public:
-    /// \brief Constructor.
-    ///
-    /// Creates a mutex. It is a non-recursive mutex (can be locked just once,
-    /// if the same threads tries to lock it again, Bad Things Happen).
-    ///
-    /// Depending on compilation parameters and OS, the mutex may or may not
-    /// do some error and sanity checking. However, such checking is meant
-    /// only to aid development, not rely on it as a feature.
-    ///
-    /// \throw std::bad_alloc In case allocation of something (memory, the
-    ///     OS mutex) fails.
-    /// \throw isc::InvalidOperation Other unspecified errors around the mutex.
-    ///     This should be rare.
-    Mutex();
-
-    /// \brief Destructor.
-    ///
-    /// Destroys the mutex. It is not allowed to destroy a mutex which is
-    /// currently locked. This means a Locker created with this Mutex must
-    /// never live longer than the Mutex itself.
-    ~Mutex();
-
-    /// \brief This holds a lock on a Mutex.
-    ///
-    /// To lock a mutex, create a locker. It'll get unlocked when the locker
-    /// is destroyed.
-    ///
-    /// If you create the locker on the stack or using some other "garbage
-    /// collecting" mechanism (auto_ptr, for example), it ensures exception
-    /// safety with regards to the mutex - it'll get released on the exit
-    /// of function no matter by what means.
-    class Locker : public boost::noncopyable {
-    public:
-        /// \brief Constructor.
-        ///
-        /// Locks the mutex. May block for extended period of time.
-        ///
-        /// \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) :
-            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();
-            mutex_ = &mutex;
-        }
-
-        /// \brief Destructor.
-        ///
-        /// Unlocks the mutex.
-        ~Locker() {
-            if (mutex_ != NULL) {
-                mutex_->unlock();
-            }
-        }
-    private:
-        Mutex* mutex_;
-    };
-    /// \brief If the mutex is currently locked
-    ///
-    /// This is debug aiding method only. And it might be unavailable in
-    /// non-debug build (because keeping the state might be needlesly
-    /// slow).
-    ///
-    /// \todo Disable in non-debug build
-    bool locked() const;
-private:
-    class Impl;
-    Impl* impl_;
-    void lock();
-    void unlock();
-};
-
-
-}
-}
-}
-
-#endif

src/lib/util/threads/sync.cc

@@ -12,7 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include "lock.h"
+#include "sync.h"
 
 #include <exceptions/exceptions.h>
 
@@ -109,6 +109,15 @@ Mutex::~Mutex() {
     }
 }
 
+void
+Mutex::postLockAction() {
+    // This assertion would fail only in non-debugging mode, in which case
+    // this method wouldn't be called either, so we simply assert the
+    // condition.
+    assert(impl_->locked_count == 0);
+    ++impl_->locked_count;
+}
+
 void
 Mutex::lock() {
     assert(impl_ != NULL);
@@ -116,13 +125,26 @@ Mutex::lock() {
     if (result != 0) {
         isc_throw(isc::InvalidOperation, std::strerror(result));
     }
-    ++impl_->locked_count; // Only in debug mode
+    postLockAction();           // Only in debug mode
+}
+
+void
+Mutex::preUnlockAction(bool throw_ok) {
+    if (impl_->locked_count == 0) {
+        if (throw_ok) {
+            isc_throw(isc::InvalidOperation,
+                      "Unlock attempt for unlocked mutex");
+        } else {
+            assert(false);
+        }
+    }
+    --impl_->locked_count;
 }
 
 void
 Mutex::unlock() {
     assert(impl_ != NULL);
-    --impl_->locked_count; // Only in debug mode
+    preUnlockAction(false);     // Only in debug mode.  Ensure no throw.
     const int result = pthread_mutex_unlock(&impl_->mutex);
     assert(result == 0); // This should never be possible
 }
@@ -133,6 +155,59 @@ Mutex::locked() const {
     return (impl_->locked_count != 0);
 }
 
+class CondVar::Impl {
+public:
+    Impl() {
+        const int result = pthread_cond_init(&cond_, NULL);
+        if (result != 0) {
+            isc_throw(isc::Unexpected, "pthread_cond_init failed: "
+                      << std::strerror(result));
+        }
+    }
+    ~Impl() {
+        const int result = pthread_cond_destroy(&cond_);
+
+        // This can happen if we try to destroy cond_ while some other thread
+        // is waiting on it.  assert() may be too strong for such a case,
+        // but we cannot safely destroy cond_ anyway.  In order to avoid
+        // throwing from a destructor we simply let the process die.
+        assert(result == 0);
+    }
+
+    // For convenience allow the main class to access this directly.
+    pthread_cond_t cond_;
+};
+
+CondVar::CondVar() : impl_(new Impl)
+{}
+
+CondVar::~CondVar() {
+    delete impl_;
+}
+
+void
+CondVar::wait(Mutex& mutex) {
+    mutex.preUnlockAction(true);    // Only in debug mode
+    const int result = pthread_cond_wait(&impl_->cond_, &mutex.impl_->mutex);
+    mutex.postLockAction();     // Only in debug mode
+
+    // pthread_cond_wait should normally succeed unless mutex is completely
+    // broken.
+    if (result != 0) {
+        isc_throw(isc::BadValue, "pthread_cond_wait failed unexpectedly: " <<
+                  std::strerror(result));
+    }
+}
+
+void
+CondVar::signal() {
+    const int result = pthread_cond_signal(&impl_->cond_);
+
+    // pthread_cond_signal() can only fail when if cond_ is invalid.  It
+    //should be impossible as long as this is a valid CondVar object.
+    assert(result == 0);
+}
+
 }
 }
 }

src/lib/util/threads/sync.h

@@ -0,0 +1,223 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef B10_THREAD_SYNC_H
+#define B10_THREAD_SYNC_H
+
+#include <boost/noncopyable.hpp>
+
+#include <cstdlib> // for NULL.
+
+namespace isc {
+namespace util {
+namespace thread {
+class CondVar;
+
+/// \brief Mutex with very simple interface
+///
+/// Since mutexes are very system dependant, we create our own wrapper around
+/// whatever is available on the system and hide it.
+///
+/// To use this mutex, create it and then lock and unlock it by creating the
+/// Mutex::Locker object.
+///
+/// Also, as mutex is a low-level system object, an error might happen at any
+/// operation with it. We convert many errors to the isc::InvalidOperation,
+/// since the errors usually happen only when used in a wrong way. Any methods
+/// or constructors in this class can throw. Allocation errors are converted
+/// to std::bad_alloc (for example when OS-dependant limit of mutexes is
+/// exceeded). Some errors which usually mean a programmer error abort the
+/// program, since there could be no safe way to recover from them.
+///
+/// The current interface is somewhat minimalistic. If we ever need more, we
+/// can add it later.
+class Mutex : boost::noncopyable {
+public:
+    /// \brief Constructor.
+    ///
+    /// Creates a mutex. It is a non-recursive mutex (can be locked just once,
+    /// if the same threads tries to lock it again, Bad Things Happen).
+    ///
+    /// Depending on compilation parameters and OS, the mutex may or may not
+    /// do some error and sanity checking. However, such checking is meant
+    /// only to aid development, not rely on it as a feature.
+    ///
+    /// \throw std::bad_alloc In case allocation of something (memory, the
+    ///     OS mutex) fails.
+    /// \throw isc::InvalidOperation Other unspecified errors around the mutex.
+    ///     This should be rare.
+    Mutex();
+
+    /// \brief Destructor.
+    ///
+    /// Destroys the mutex. It is not allowed to destroy a mutex which is
+    /// currently locked. This means a Locker created with this Mutex must
+    /// never live longer than the Mutex itself.
+    ~Mutex();
+
+    /// \brief This holds a lock on a Mutex.
+    ///
+    /// To lock a mutex, create a locker. It'll get unlocked when the locker
+    /// is destroyed.
+    ///
+    /// If you create the locker on the stack or using some other "garbage
+    /// collecting" mechanism (auto_ptr, for example), it ensures exception
+    /// safety with regards to the mutex - it'll get released on the exit
+    /// of function no matter by what means.
+    class Locker : boost::noncopyable {
+    public:
+        /// \brief Constructor.
+        ///
+        /// Locks the mutex. May block for extended period of time.
+        ///
+        /// \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) :
+            mutex_(mutex)
+        {
+            mutex.lock();
+        }
+
+        /// \brief Destructor.
+        ///
+        /// Unlocks the mutex.
+        ~Locker() {
+            mutex_.unlock();
+        }
+    private:
+        Mutex& mutex_;
+    };
+    /// \brief If the mutex is currently locked
+    ///
+    /// This is debug aiding method only. And it might be unavailable in
+    /// non-debug build (because keeping the state might be needlesly
+    /// slow).
+    ///
+    /// \todo Disable in non-debug build
+    bool locked() const;
+private:
+    friend class CondVar;
+
+    // Commonly called after acquiring the lock, checking and updating
+    // internal state for debug.
+    void postLockAction();
+
+    // Commonly called before releasing the lock, checking and updating
+    // internal state for debug.
+    //
+    // If throw_ok is true, it throws \c isc::InvalidOperation when the check
+    // fails; otherwise it aborts the process.  This parameter must be set
+    // to false if the call to this shouldn't result in an exception (e.g.
+    // when called from a destructor).
+    void preUnlockAction(bool throw_ok);
+
+    class Impl;
+    Impl* impl_;
+    void lock();
+    void unlock();
+};
+
+/// \brief Encapsulation for a condition variable.
+///
+/// This class provides a simple encapsulation of condition variable for
+/// inter-thread synchronization.  It has similar but simplified interface as
+/// that for \c pthread_cond_ variants.
+///
+/// It uses the \c Mutex class object for the mutex used with the condition
+/// variable.  Since for normal applications the internal \c Mutex::Locker
+/// class is the only available interface to acquire a lock, sample code
+/// for waiting on a condition variable would look like this:
+/// \code
+/// CondVar cond;
+/// Mutex mutex;
+/// {
+///     Mutex::Locker locker(mutex);
+///     while (some_condition) {
+///         cond.wait(mutex);
+///     }
+///     // do something under the protection of locker
+/// }   // lock is released here
+/// \endcode
+/// Note that \c mutex passed to the \c wait() method must be the same one
+/// used to construct the \c locker.
+///
+/// Right now there is no equivalent to pthread_cond_broadcast() or
+/// pthread_cond_timedwait() in this class, because this class is meant
+/// for internal development of BIND 10 and we don't need these at the
+/// moment.  If and when we need these interfaces they can be added at that
+/// point.
+///
+/// \note This class is defined as a friend class of \c Mutex and directly
+/// refers to and modifies private internals of the \c Mutex class.  It breaks
+/// the assumption that the lock is only acquired or released via the
+/// \c Locker class and breaks other integrity assumption on \c Mutex,
+/// thereby making it more fragile, but we couldn't find other way to
+/// implement a safe and still simple realization of condition variables.
+/// So, this is a kind of compromise.  If this class is needed to be
+/// extended, first consider a way to use public interfaces of \c Mutex;
+/// do not easily rely on the fact that this class is a friend of it.
+class CondVar : boost::noncopyable {
+public:
+    /// \brief Constructor.
+    ///
+    /// \throw std::bad_alloc memory allocation failure
+    /// \throw isc::Unexpected other unexpected shortage of system resource
+    CondVar();
+
+    /// \brief Destructor.
+    ///
+    /// An object of this class must not be destroyed while some thread
+    /// is waiting on it.  If this condition isn't met the destructor will
+    /// terminate the program.
+    ~CondVar();
+
+    /// \brief Wait on the condition variable.
+    ///
+    /// This method works like \c pthread_cond_wait().  For mutex it takes
+    /// an \c Mutex class object.  A lock for the mutex must have been
+    /// acquired.  If this condition isn't met, it can throw an exception
+    /// (in the debug mode build) or result in undefined behavior.
+    ///
+    /// The lock will be automatically released within this method, and
+    /// will be re-acquired on the exit of this method.
+    ///
+    /// \throw isc::InvalidOperation mutex isn't locked
+    /// \throw isc::BadValue mutex is not a valid \c Mutex object
+    ///
+    /// \param mutex A \c Mutex object to be released on wait().
+    void wait(Mutex& mutex);
+
+    /// \brief Unblock a thread waiting for the condition variable.
+    ///
+    /// This method wakes one of other threads (if any) waiting on this object
+    /// via the \c wait() call.
+    ///
+    /// This method never throws; if some unexpected low level error happens
+    /// it terminates the program.
+    void signal();
+private:
+    class Impl;
+    Impl* impl_;
+};
+
+} // namespace thread
+} // namespace util
+} // namespace isc
+
+#endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/util/threads/tests/Makefile.am

@@ -23,6 +23,7 @@ TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += thread_unittest.cc
 run_unittests_SOURCES += lock_unittest.cc
+run_unittests_SOURCES += condvar_unittest.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) $(PTHREAD_LDFLAGS)

src/lib/util/threads/tests/condvar_unittest.cc

@@ -0,0 +1,161 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <exceptions/exceptions.h>
+
+#include <util/threads/sync.h>
+#include <util/threads/thread.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/bind.hpp>
+#include <boost/scoped_ptr.hpp>
+
+#include <cstring>
+
+#include <unistd.h>
+#include <signal.h>
+
+using namespace isc::util::thread;
+
+namespace {
+// Used as a signal handler below.
+volatile bool do_exit;          // use for emergency escape
+void
+alarmHandler(int) {
+    do_exit = true;
+}
+
+class CondVarTest : public ::testing::Test {
+protected:
+    // We use a signal in case some of the thread synchronization tests
+    // unexpectedly cause a deadlock.
+    void SetUp() {
+        do_exit = false;
+
+        std::memset(&handler_, 0, sizeof(handler_));
+        handler_.sa_handler = alarmHandler;
+        if (sigaction(SIGALRM, &handler_, &original_) != 0) {
+            FAIL() << "Couldn't set alarm";
+        }
+        alarm(10);                  // 10sec duration: arbitrary choice
+    }
+    void TearDown() {
+        // Cancel the alarm and return the original handler
+        alarm(0);
+        if (sigaction(SIGALRM, &original_, NULL)) {
+            FAIL() << "Couldn't restore alarm";
+        }
+    }
+
+    CondVar condvar_;
+    Mutex mutex_;
+private:
+    struct sigaction handler_, original_;
+};
+
+TEST(CondVarTest0, create) {
+    // Just construct and destruct it.  Nothing unusual should happen.
+    EXPECT_NO_THROW(CondVar condvar);
+}
+
+// Running on a separate thread, just updating the argument and waking up
+// the other thread via the condition variable passed.
+void
+ringSignal(CondVar* condvar, Mutex* mutex, int* arg) {
+    assert(*arg == 0);
+    Mutex::Locker locker(*mutex);
+    ++*arg;
+    condvar->signal();
+}
+
+// A simple wait-signal operation on a condition variable.
+TEST_F(CondVarTest, waitAndSignal) {
+    Mutex::Locker locker(mutex_);
+    int shared_var = 0; // let the other thread increment this
+    Thread t(boost::bind(&ringSignal, &condvar_, &mutex_, &shared_var));
+    condvar_.wait(mutex_);
+    t.wait();
+    EXPECT_EQ(1, shared_var);
+}
+
+// Thread's main code for the next test
+void
+signalAndWait(CondVar* condvar1, CondVar* condvar2, Mutex* mutex, int* arg) {
+    Mutex::Locker locker(*mutex);
+    ++*arg;
+    condvar2->signal();  // let the main thread know this one is ready
+    condvar1->wait(*mutex);
+    ++*arg;
+}
+
+// Similar to the previous test, but having two threads wait for a condvar.
+TEST_F(CondVarTest, multiWaits) {
+    boost::scoped_ptr<Mutex::Locker> locker(new Mutex::Locker(mutex_));
+    CondVar condvar2; // separate cond var for initial synchronization
+    int shared_var = 0; // let the other thread increment this
+    Thread t1(boost::bind(&signalAndWait, &condvar_, &condvar2, &mutex_,
+                          &shared_var));
+    Thread t2(boost::bind(&signalAndWait, &condvar_, &condvar2, &mutex_,
+                          &shared_var));
+
+    // Wait until both threads are waiting on condvar_.
+    while (shared_var < 2 && !do_exit) {
+        condvar2.wait(mutex_);
+    }
+    // Check we exited from the loop successfully.
+    ASSERT_FALSE(do_exit);
+    ASSERT_EQ(2, shared_var);
+
+    // release the lock, wake up both threads, wait for them to die, and
+    // confirm they successfully woke up.
+    locker.reset();
+    condvar_.signal();
+    condvar_.signal();
+    t1.wait();
+    t2.wait();
+    EXPECT_EQ(4, shared_var);
+}
+
+// Similar to the previous version of the same function, but just do
+// condvar operations.  It will never wake up.
+void
+signalAndWait(CondVar* condvar, Mutex* mutex) {
+    Mutex::Locker locker(*mutex);
+    condvar->signal();
+    condvar->wait(*mutex);
+}
+
+TEST_F(CondVarTest, destroyWhileWait) {
+    // We'll destroy a CondVar object while the thread is still waiting
+    // on it.  This will trigger an assertion failure.
+    EXPECT_DEATH_IF_SUPPORTED({
+            CondVar cond;
+            Mutex::Locker locker(mutex_);
+            Thread t(boost::bind(&signalAndWait, &cond, &mutex_));
+            cond.wait(mutex_);
+        }, "");
+}
+
+TEST_F(CondVarTest, badWait) {
+    // In our implementation, wait() requires acquiring the lock beforehand.
+    EXPECT_THROW(condvar_.wait(mutex_), isc::InvalidOperation);
+}
+
+TEST_F(CondVarTest, emptySignal) {
+    // It's okay to call signal when no one waits.
+    EXPECT_NO_THROW(condvar_.signal());
+}
+
+}

src/lib/util/threads/tests/lock_unittest.cc

@@ -12,7 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/threads/lock.h>
+#include <util/threads/sync.h>
 #include <util/threads/thread.h>
 
 #include <gtest/gtest.h>
@@ -80,13 +80,13 @@ performIncrement(volatile double* canary, volatile bool* ready_me,
 }
 
 void
-no_handler(int) {}
+noHandler(int) {}
 
 TEST(MutexTest, swarm) {
     // Create a timeout in case something got stuck here
     struct sigaction ignored, original;
-    memset(&ignored, 0, sizeof ignored);
+    memset(&ignored, 0, sizeof(ignored));
-    ignored.sa_handler = no_handler;
+    ignored.sa_handler = noHandler;
     if (sigaction(SIGALRM, &ignored, &original)) {
         FAIL() << "Couldn't set alarm";
     }

src/lib/util/threads/thread.cc

@@ -13,7 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include "thread.h"
-#include "lock.h"
+#include "sync.h"
 
 #include <memory>
 #include <string>