[2236] Use ENABLE_DEBUG within the lock implementation

src/lib/util/threads/sync.cc

@@ -31,12 +31,16 @@ namespace thread {
 
 class Mutex::Impl {
 public:
-    Impl() :
-        locked_count(0)
+    Impl()
+#ifdef ENABLE_DEBUG
+        : locked_count(0)
+#endif // ENABLE_DEBUG
     {}
+
     pthread_mutex_t mutex;
-    // Only in debug mode
+#ifdef ENABLE_DEBUG
     size_t locked_count;
+#endif // ENABLE_DEBUG
 };
 
 namespace {
@@ -93,12 +97,17 @@ Mutex::Mutex() :
 Mutex::~Mutex() {
     if (impl_ != NULL) {
         const int result = pthread_mutex_destroy(&impl_->mutex);
+
+#ifdef ENABLE_DEBUG
         const bool locked = impl_->locked_count != 0;
+#endif // ENABLE_DEBUG
+
         delete impl_;
         // We don't want to throw from the destructor. Also, if this ever
         // fails, something is really screwed up a lot.
         assert(result == 0);
 
+#ifdef ENABLE_DEBUG
         // We should not try to destroy a locked mutex, bad threaded monsters
         // could get loose if we ever do and it is also forbidden by pthreads.
 
@@ -106,9 +115,12 @@ Mutex::~Mutex() {
         // pthread_mutex_destroy should check for it already. But it seems
         // there are systems that don't check it.
         assert(!locked);
+#endif // ENABLE_DEBUG
     }
 }
 
+#ifdef ENABLE_DEBUG
+
 void
 Mutex::postLockAction() {
     // This assertion would fail only in non-debugging mode, in which case
@@ -119,16 +131,6 @@ Mutex::postLockAction() {
 }
 
 void
-Mutex::lock() {
-    assert(impl_ != NULL);
-    const int result = pthread_mutex_lock(&impl_->mutex);
-    if (result != 0) {
-        isc_throw(isc::InvalidOperation, std::strerror(result));
-    }
-    postLockAction();           // Only in debug mode
-}
-
-void
 Mutex::preUnlockAction(bool throw_ok) {
     if (impl_->locked_count == 0) {
         if (throw_ok) {
@@ -141,20 +143,35 @@ Mutex::preUnlockAction(bool throw_ok) {
     --impl_->locked_count;
 }
 
+bool
+Mutex::locked() const {
+    return (impl_->locked_count != 0);
+}
+
+#endif // ENABLE_DEBUG
+
+void
+Mutex::lock() {
+    assert(impl_ != NULL);
+    const int result = pthread_mutex_lock(&impl_->mutex);
+    if (result != 0) {
+        isc_throw(isc::InvalidOperation, std::strerror(result));
+    }
+#ifdef ENABLE_DEBUG
+    postLockAction();           // Only in debug mode
+#endif // ENABLE_DEBUG
+}
+
 void
 Mutex::unlock() {
     assert(impl_ != NULL);
+#ifdef ENABLE_DEBUG
     preUnlockAction(false);     // Only in debug mode.  Ensure no throw.
+#endif // ENABLE_DEBUG
     const int result = pthread_mutex_unlock(&impl_->mutex);
     assert(result == 0); // This should never be possible
 }
 
-// TODO: Disable in non-debug build
-bool
-Mutex::locked() const {
-    return (impl_->locked_count != 0);
-}
-
 class CondVar::Impl {
 public:
     Impl() {
@@ -187,10 +204,13 @@ CondVar::~CondVar() {
 
 void
 CondVar::wait(Mutex& mutex) {
+#ifdef ENABLE_DEBUG
     mutex.preUnlockAction(true);    // Only in debug mode
     const int result = pthread_cond_wait(&impl_->cond_, &mutex.impl_->mutex);
     mutex.postLockAction();     // Only in debug mode
-
+#else
+    const int result = pthread_cond_wait(&impl_->cond_, &mutex.impl_->mutex);
+#endif
     // pthread_cond_wait should normally succeed unless mutex is completely
     // broken.
     if (result != 0) {

src/lib/util/threads/sync.h

@@ -110,6 +110,8 @@ public:
 private:
     friend class CondVar;
 
+#ifdef ENABLE_DEBUG
+
     // Commonly called after acquiring the lock, checking and updating
     // internal state for debug.
     void postLockAction();
@@ -123,6 +125,8 @@ private:
     // when called from a destructor).
     void preUnlockAction(bool throw_ok);
 
+#endif // ENABLE_DEBUG
+
     class Impl;
     Impl* impl_;
     void lock();

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

@@ -151,7 +151,11 @@ TEST_F(CondVarTest, DISABLED_destroyWhileWait) {
 
 TEST_F(CondVarTest, badWait) {
     // In our implementation, wait() requires acquiring the lock beforehand.
+#ifdef ENABLE_DEBUG
     EXPECT_THROW(condvar_.wait(mutex_), isc::InvalidOperation);
+#else
+    EXPECT_THROW(condvar_.wait(mutex_), isc::BadValue);
+#endif // ENABLE_DEBUG
 }
 
 TEST_F(CondVarTest, emptySignal) {

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

@@ -31,13 +31,21 @@ TEST(MutexTest, lockMultiple) {
     // TODO: Once we support non-debug mutexes, disable the test if we compile
     // with them.
     Mutex mutex;
+#ifdef ENABLE_DEBUG
     EXPECT_FALSE(mutex.locked()); // Debug-only build
+#endif // ENABLE_DEBUG
+
     Mutex::Locker l1(mutex);
+#ifdef ENABLE_DEBUG
     EXPECT_TRUE(mutex.locked()); // Debug-only build
+#endif // ENABLE_DEBUG
+
     EXPECT_THROW({
         Mutex::Locker l2(mutex); // Attempt to lock again.
     }, isc::InvalidOperation);
+#ifdef ENABLE_DEBUG
     EXPECT_TRUE(mutex.locked()); // Debug-only build
+#endif // ENABLE_DEBUG
 }
 
 // Destroying a locked mutex is a bad idea as well