BIND 10 trac2236, updated. 2eaf01995b4c0d6c007afd51c94b8453cbb797ca [2236] Don't use PTHREAD_MUTEX_ERRORCHECK in non-debug builds

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Oct 23 04:23:25 UTC 2012


The branch, trac2236 has been updated
       via  2eaf01995b4c0d6c007afd51c94b8453cbb797ca (commit)
       via  8b19f3a80a57b4a0c41e58460920924d5613e606 (commit)
       via  98db15a31fea52ec2b1952846c61d20f9f373305 (commit)
       via  4f4b3724e1e06933646dcbee14f31eb2713a514d (commit)
      from  fa78c4ba3aff851955576ec7e98f8fe98dacd889 (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 2eaf01995b4c0d6c007afd51c94b8453cbb797ca
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Oct 23 09:36:26 2012 +0530

    [2236] Don't use PTHREAD_MUTEX_ERRORCHECK in non-debug builds
    
    Also disable tests in non-debug builds that depend on
    PTHREAD_MUTEX_ERRORCHECK.

commit 8b19f3a80a57b4a0c41e58460920924d5613e606
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Oct 23 09:28:43 2012 +0530

    [2236] Remove config.h include from sync.h
    
    ... and move it to sync.cc. This is so that sync.h can be dist-ed.
    
    Also add comments to some methods that may not be available in
    non-debug builds.

commit 98db15a31fea52ec2b1952846c61d20f9f373305
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Oct 23 09:25:20 2012 +0530

    [2236] Add comments to configure.ac about --enable-debug and performance

commit 4f4b3724e1e06933646dcbee14f31eb2713a514d
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Oct 23 09:24:50 2012 +0530

    [2236] Access Mutex::locked() only when ENABLE_DEBUG is defined

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

Summary of changes:
 configure.ac                                   |    6 ++++--
 src/bin/auth/auth_srv.cc                       |    8 ++++++--
 src/bin/auth/tests/auth_srv_unittest.cc        |    4 ++++
 src/lib/util/threads/sync.cc                   |    7 ++++++-
 src/lib/util/threads/sync.h                    |   12 ++++++------
 src/lib/util/threads/tests/condvar_unittest.cc |    8 ++++----
 src/lib/util/threads/tests/lock_unittest.cc    |   14 +++++++-------
 7 files changed, 37 insertions(+), 22 deletions(-)

-----------------------------------------------------------------------
diff --git a/configure.ac b/configure.ac
index 83b9c02..922631b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -12,7 +12,9 @@ AC_CONFIG_MACRO_DIR([m4macros])
 # Checks for programs.
 AC_PROG_CXX
 
-# Enable debugging facilities?
+# Enable low-performing debugging facilities? This option optionally
+# enables some debugging aids that perform slowly and hence aren't built
+# by default.
 AC_ARG_ENABLE([debug],
   AS_HELP_STRING([--enable-debug],
     [enable debugging (default is no)]),
@@ -22,7 +24,7 @@ AC_ARG_ENABLE([debug],
     *)   AC_MSG_ERROR([bad value ${enableval} for --enable-debug]) ;;
   esac],[debug_enabled=no])
 AM_CONDITIONAL([DEBUG_ENABLED], [test x$debug_enabled = xyes])
-AM_COND_IF([DEBUG_ENABLED], [AC_DEFINE([ENABLE_DEBUG], [1], [Enable debugging facilities?])])
+AM_COND_IF([DEBUG_ENABLED], [AC_DEFINE([ENABLE_DEBUG], [1], [Enable low-performing debugging facilities?])])
 
 # Libtool configuration
 #
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 315a752..bf653a4 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -274,10 +274,12 @@ public:
     shared_ptr<ConfigurableClientList> getDataSrcClientList(
         const RRClass& rrclass)
     {
-        // TODO: Debug-build only check
+#ifdef ENABLE_DEBUG
+        // Debug-build only check
         if (!mutex_.locked()) {
             isc_throw(isc::Unexpected, "Not locked!");
         }
+#endif
         const std::map<RRClass, shared_ptr<ConfigurableClientList> >::
             const_iterator it(datasrc_client_lists_->find(rrclass));
         if (it == datasrc_client_lists_->end()) {
@@ -935,10 +937,12 @@ AuthSrv::destroyDDNSForwarder() {
 
 AuthSrv::DataSrcClientListsPtr
 AuthSrv::swapDataSrcClientLists(DataSrcClientListsPtr new_lists) {
-    // TODO: Debug-build only check
+#ifdef ENABLE_DEBUG
+    // Debug-build only check
     if (!impl_->mutex_.locked()) {
         isc_throw(isc::Unexpected, "Not locked!");
     }
+#endif
     std::swap(new_lists, impl_->datasrc_client_lists_);
     return (new_lists);
 }
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index 396b247..cde4c81 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -1824,6 +1824,8 @@ TEST_F(AuthSrvTest, clientList) {
     EXPECT_FALSE(server.getDataSrcClientList(RRClass::CH()));
 }
 
+#ifdef ENABLE_DEBUG
+
 // We just test the mutex can be locked (exactly once).
 TEST_F(AuthSrvTest, mutex) {
     isc::util::thread::Mutex::Locker l1(server.getDataSrcClientListMutex());
@@ -1837,4 +1839,6 @@ TEST_F(AuthSrvTest, mutex) {
     }, isc::InvalidOperation);
 }
 
+#endif // ENABLE_DEBUG
+
 }
diff --git a/src/lib/util/threads/sync.cc b/src/lib/util/threads/sync.cc
index 9c15f03..ca467b6 100644
--- a/src/lib/util/threads/sync.cc
+++ b/src/lib/util/threads/sync.cc
@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include "config.h"
+
 #include "sync.h"
 
 #include <exceptions/exceptions.h>
@@ -74,12 +76,15 @@ Mutex::Mutex() :
             isc_throw(isc::InvalidOperation, std::strerror(result));
     }
     Deinitializer deinitializer(attributes);
+
+#ifdef ENABLE_DEBUG
     // TODO: Distinguish if debug mode is enabled in compilation.
-    // If so, it should be PTHREAD_MUTEX_NORMAL or NULL
     result = pthread_mutexattr_settype(&attributes, PTHREAD_MUTEX_ERRORCHECK);
     if (result != 0) {
         isc_throw(isc::InvalidOperation, std::strerror(result));
     }
+#endif // ENABLE_DEBUG
+
     auto_ptr<Impl> impl(new Impl);
     result = pthread_mutex_init(&impl->mutex, &attributes);
     switch (result) {
diff --git a/src/lib/util/threads/sync.h b/src/lib/util/threads/sync.h
index 6a968df..87c78be 100644
--- a/src/lib/util/threads/sync.h
+++ b/src/lib/util/threads/sync.h
@@ -15,8 +15,6 @@
 #ifndef B10_THREAD_SYNC_H
 #define B10_THREAD_SYNC_H
 
-#include "config.h"
-
 #include <boost/noncopyable.hpp>
 
 #include <cstdlib> // for NULL.
@@ -112,10 +110,11 @@ public:
 private:
     friend class CondVar;
 
-#ifdef ENABLE_DEBUG
-
     // Commonly called after acquiring the lock, checking and updating
     // internal state for debug.
+    //
+    // Note that this method is only available when the build is
+    // configured with debugging support.
     void postLockAction();
 
     // Commonly called before releasing the lock, checking and updating
@@ -125,10 +124,11 @@ private:
     // 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).
+    //
+    // Note that this method is only available when the build is
+    // configured with debugging support.
     void preUnlockAction(bool throw_ok);
 
-#endif // ENABLE_DEBUG
-
     class Impl;
     Impl* impl_;
     void lock();
diff --git a/src/lib/util/threads/tests/condvar_unittest.cc b/src/lib/util/threads/tests/condvar_unittest.cc
index 6cec0dc..8b1fe54 100644
--- a/src/lib/util/threads/tests/condvar_unittest.cc
+++ b/src/lib/util/threads/tests/condvar_unittest.cc
@@ -149,15 +149,15 @@ TEST_F(CondVarTest, DISABLED_destroyWhileWait) {
         }, "");
 }
 
+#ifdef ENABLE_DEBUG
+
 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
 }
 
+#endif // ENABLE_DEBUG
+
 TEST_F(CondVarTest, emptySignal) {
     // It's okay to call signal when no one waits.
     EXPECT_NO_THROW(condvar_.signal());
diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
index abd0f60..40af7ee 100644
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -26,28 +26,28 @@ using namespace isc::util::thread;
 
 namespace {
 
-// If we try to lock the debug mutex multiple times, it should throw.
+#ifdef ENABLE_DEBUG
+
+// If we try to lock the debug mutex multiple times, it should
+// throw. This test will complete properly only when pthread debugging
+// facilities are enabled by configuring the code for debug build.
 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
 }
 
+#endif // ENABLE_DEBUG
+
 // Destroying a locked mutex is a bad idea as well
 #ifdef EXPECT_DEATH
 TEST(MutexTest, destroyLocked) {



More information about the bind10-changes mailing list