BIND 10 trac2332, updated. 8c02f8a83f011816c3fb19f655a1e57edf0c1048 [2332] added comments about missing pthread_cond_xxx variants
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Oct 15 21:02:22 UTC 2012
The branch, trac2332 has been updated
via 8c02f8a83f011816c3fb19f655a1e57edf0c1048 (commit)
via a439004524335feb296ee7477a5b450a2a7d7953 (commit)
via 410987bf3c9299053c07aee46591193c97af6ea4 (commit)
via 7f94f671e0da9c98cabcc38433852732294bee4a (commit)
via 671005eea2c18ad34bb0bad435b538447cdc7a50 (commit)
via b6d2a03ff5dfc0b1269e69e44defb2999e8a5d33 (commit)
via d504c06cbe387363d11ae091eeaa980444cc5190 (commit)
from d3983e1e68f66bbf6c69838aebe01a8f95b7bc33 (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 8c02f8a83f011816c3fb19f655a1e57edf0c1048
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 13:45:53 2012 -0700
[2332] added comments about missing pthread_cond_xxx variants
commit a439004524335feb296ee7477a5b450a2a7d7953
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 13:42:24 2012 -0700
[2332] fixing typos and wording in doc
commit 410987bf3c9299053c07aee46591193c97af6ea4
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 13:39:32 2012 -0700
[2332] updated doc for wait() on what should happen if mutex isn't locked.
mentioning pthread_cond_wait() seems to be confusing, so I'd rather
just describing what this method does.
commit 7f94f671e0da9c98cabcc38433852732294bee4a
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 13:31:04 2012 -0700
[2332] Make sure preUnlockAction() doesn't throw when it shouldn't.
adding a parameter to control that.
commit 671005eea2c18ad34bb0bad435b538447cdc7a50
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 11:21:47 2012 -0700
[2332] changed the type of Locker::mutex_ to a reference.
may not be a big deal in such a simple case, but just as suggested in review.
commit b6d2a03ff5dfc0b1269e69e44defb2999e8a5d33
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 11:17:21 2012 -0700
[2332] renamed a too-short test name a possibly better one
commit d504c06cbe387363d11ae091eeaa980444cc5190
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Oct 15 11:14:56 2012 -0700
[2332] use EXPECT_DEATH_IF_SUPPORTED instead of ifdef.
avoiding ifdef is generally better, and _IF_SUPPORTED seems to have been
supported since googletest 1.4 and should be commonly available for us.
-----------------------------------------------------------------------
Summary of changes:
src/lib/util/threads/lock.cc | 13 +++++++---
src/lib/util/threads/lock.h | 32 ++++++++++++++++--------
src/lib/util/threads/tests/condvar_unittest.cc | 6 ++---
3 files changed, 32 insertions(+), 19 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/util/threads/lock.cc b/src/lib/util/threads/lock.cc
index 13d0a0d..3f4d92a 100644
--- a/src/lib/util/threads/lock.cc
+++ b/src/lib/util/threads/lock.cc
@@ -128,9 +128,14 @@ Mutex::lock() {
}
void
-Mutex::preUnlockAction() {
+Mutex::preUnlockAction(bool throw_ok) {
if (impl_->locked_count == 0) {
- isc_throw(isc::InvalidOperation, "Unlock attempt for unlocked mutex");
+ if (throw_ok) {
+ isc_throw(isc::InvalidOperation,
+ "Unlock attempt for unlocked mutex");
+ } else {
+ assert(false);
+ }
}
--impl_->locked_count;
}
@@ -138,7 +143,7 @@ Mutex::preUnlockAction() {
void
Mutex::unlock() {
assert(impl_ != NULL);
- preUnlockAction(); // 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
}
@@ -181,7 +186,7 @@ CondVar::~CondVar() {
void
CondVar::wait(Mutex& mutex) {
- mutex.preUnlockAction(); // Only in debug mode
+ mutex.preUnlockAction(true); // Only in debug mode
const int result = pthread_cond_wait(&impl_->cond_, &mutex.impl_->mutex);
mutex.postLockAction(); // Only in debug mode
diff --git a/src/lib/util/threads/lock.h b/src/lib/util/threads/lock.h
index 8a072f1..cb3805e 100644
--- a/src/lib/util/threads/lock.h
+++ b/src/lib/util/threads/lock.h
@@ -85,7 +85,7 @@ public:
/// 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_(mutex)
{
mutex.lock();
}
@@ -94,10 +94,10 @@ public:
///
/// Unlocks the mutex.
~Locker() {
- mutex_->unlock();
+ mutex_.unlock();
}
private:
- Mutex* const mutex_;
+ Mutex& mutex_;
};
/// \brief If the mutex is currently locked
///
@@ -116,7 +116,12 @@ private:
// Commonly called before releasing the lock, checking and updating
// internal state for debug.
- void preUnlockAction();
+ //
+ // 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_;
@@ -128,7 +133,7 @@ private:
///
/// This class provides a simple encapsulation of condition variable for
/// inter-thread synchronization. It has similar but simplified interface as
-/// that for \c pthread_bond_ variants.
+/// 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
@@ -148,6 +153,12 @@ private:
/// 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
@@ -167,7 +178,7 @@ public:
/// \brief Destructor.
///
- /// An object of this class must not be destructed while some thread
+ /// 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();
@@ -175,10 +186,9 @@ public:
/// \brief Wait on the condition variable.
///
/// This method works like \c pthread_cond_wait(). For mutex it takes
- /// an \c Mutex class object. Unlike \c pthread_cond_wait(), however,
- /// this method requires a lock for the mutex has been acquired.
- /// If this condition isn't met, it can throw an exception (in the
- /// debug mode build) or result in undefined behavior.
+ /// 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.
@@ -191,7 +201,7 @@ public:
/// \brief Unblock a thread waiting for the condition variable.
///
- /// This method waits one of other threads (if any) waiting on this object
+ /// 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
diff --git a/src/lib/util/threads/tests/condvar_unittest.cc b/src/lib/util/threads/tests/condvar_unittest.cc
index 4de4f85..57cc5d6 100644
--- a/src/lib/util/threads/tests/condvar_unittest.cc
+++ b/src/lib/util/threads/tests/condvar_unittest.cc
@@ -137,18 +137,16 @@ signalAndWait(CondVar* condvar, Mutex* mutex) {
condvar->wait(*mutex);
}
-#ifdef EXPECT_DEATH
-TEST_F(CondVarTest, bad) {
+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({
+ EXPECT_DEATH_IF_SUPPORTED({
CondVar cond;
Mutex::Locker locker(mutex_);
Thread t(boost::bind(&signalAndWait, &cond, &mutex_));
cond.wait(mutex_);
}, "");
}
-#endif
TEST_F(CondVarTest, badWait) {
// In our implementation, wait() requires acquiring the lock beforehand.
More information about the bind10-changes
mailing list