BIND 10 trac2198_2, updated. fff8929b13d8c2cd725b861c7e930c0060b54d83 [2198] Stop using direct lock methods on Mutex inside InterprocessSyncFile
BIND 10 source code commits
bind10-changes at lists.isc.org
Sun Oct 14 16:57:40 UTC 2012
The branch, trac2198_2 has been updated
via fff8929b13d8c2cd725b861c7e930c0060b54d83 (commit)
via ecef170dd0af4f282d4426ef663b667c76873737 (commit)
via 350c775f7bd5c7777f0345754dac4ecaa0a9af18 (commit)
via 666bfa9c7b6de4ee31e210d7f3e0f2f68704f2de (commit)
from d042cd04c0784641738a0f06e5a6b193b433d39f (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 fff8929b13d8c2cd725b861c7e930c0060b54d83
Author: Mukund Sivaraman <muks at isc.org>
Date: Sun Oct 14 22:22:35 2012 +0530
[2198] Stop using direct lock methods on Mutex inside InterprocessSyncFile
commit ecef170dd0af4f282d4426ef663b667c76873737
Author: Mukund Sivaraman <muks at isc.org>
Date: Sun Oct 14 22:19:47 2012 +0530
[2198] Make direct methods on Mutex private
commit 350c775f7bd5c7777f0345754dac4ecaa0a9af18
Author: Mukund Sivaraman <muks at isc.org>
Date: Sun Oct 14 22:17:29 2012 +0530
[2198] Add a block argument to Mutex::Locker constructor
commit 666bfa9c7b6de4ee31e210d7f3e0f2f68704f2de
Author: Mukund Sivaraman <muks at isc.org>
Date: Sun Oct 14 22:16:41 2012 +0530
[2198] Add AlreadyLocked exception to Mutex::Locker
-----------------------------------------------------------------------
Summary of changes:
src/lib/util/interprocess_sync_file.cc | 41 +++++++++++----------------
src/lib/util/interprocess_sync_file.h | 4 ++-
src/lib/util/threads/lock.h | 26 +++++++++++++++--
src/lib/util/threads/tests/lock_unittest.cc | 18 ------------
4 files changed, 43 insertions(+), 46 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/util/interprocess_sync_file.cc b/src/lib/util/interprocess_sync_file.cc
index 71d3287..b406e6e 100644
--- a/src/lib/util/interprocess_sync_file.cc
+++ b/src/lib/util/interprocess_sync_file.cc
@@ -34,7 +34,6 @@ namespace util {
namespace { // unnamed namespace
typedef std::map<std::string, boost::weak_ptr<Mutex> > SyncMap;
-typedef boost::shared_ptr<Mutex> MutexPtr;
Mutex sync_map_mutex;
SyncMap sync_map;
@@ -69,9 +68,11 @@ InterprocessSyncFile::~InterprocessSyncFile() {
Mutex::Locker locker(sync_map_mutex);
- // Unref the shared mutex first.
+ // Unref the shared mutex.
+ locker_.reset();
mutex_.reset();
+ // Remove name from the map if it is unused anymore.
SyncMap::iterator it = sync_map.find(task_name_);
assert(it != sync_map.end());
@@ -139,20 +140,15 @@ InterprocessSyncFile::lock() {
}
// First grab the thread lock...
- mutex_->lock();
+ LockerPtr locker(new Mutex::Locker(*mutex_));
// ... then the file lock.
- try {
- if (do_lock(F_SETLKW, F_WRLCK)) {
- is_locked_ = true;
- return (true);
- }
- } catch (...) {
- mutex_->unlock();
- throw;
+ if (do_lock(F_SETLKW, F_WRLCK)) {
+ is_locked_ = true;
+ locker_ = locker;
+ return (true);
}
- mutex_->unlock();
return (false);
}
@@ -163,23 +159,20 @@ InterprocessSyncFile::tryLock() {
}
// First grab the thread lock...
- if (!mutex_->tryLock()) {
+ LockerPtr locker;
+ try {
+ locker.reset(new Mutex::Locker(*mutex_, false));
+ } catch (const Mutex::Locker::AlreadyLocked&) {
return (false);
}
// ... then the file lock.
- try {
- // ... then the file lock.
- if (do_lock(F_SETLK, F_WRLCK)) {
- is_locked_ = true;
- return (true);
- }
- } catch (...) {
- mutex_->unlock();
- throw;
+ if (do_lock(F_SETLK, F_WRLCK)) {
+ is_locked_ = true;
+ locker_ = locker;
+ return (true);
}
- mutex_->unlock();
return (false);
}
@@ -194,7 +187,7 @@ InterprocessSyncFile::unlock() {
return (false);
}
- mutex_->unlock();
+ locker_.reset();
is_locked_ = false;
return (true);
}
diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h
index 95f0c28..5cb64c1 100644
--- a/src/lib/util/interprocess_sync_file.h
+++ b/src/lib/util/interprocess_sync_file.h
@@ -86,7 +86,9 @@ private:
int fd_; ///< The descriptor for the open file
typedef boost::shared_ptr<isc::util::thread::Mutex> MutexPtr;
- MutexPtr mutex_; ///< A mutex for mutual exclusion among threads
+ typedef boost::shared_ptr<isc::util::thread::Mutex::Locker> LockerPtr;
+ MutexPtr mutex_; ///< A mutex for mutual exclusion among threads
+ LockerPtr locker_; ///< A locker on mutex_
};
} // namespace util
diff --git a/src/lib/util/threads/lock.h b/src/lib/util/threads/lock.h
index 8426a1a..4992bdb 100644
--- a/src/lib/util/threads/lock.h
+++ b/src/lib/util/threads/lock.h
@@ -15,6 +15,7 @@
#ifndef B10_THREAD_LOCK_H
#define B10_THREAD_LOCK_H
+#include <exceptions/exceptions.h>
#include <boost/noncopyable.hpp>
#include <cstdlib> // for NULL.
@@ -76,21 +77,39 @@ public:
/// of function no matter by what means.
class Locker : public 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_(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();
+ if (block) {
+ mutex.lock();
+ } else {
+ if (!mutex.tryLock()) {
+ isc_throw(AlreadyLocked, "The mutex is already locked");
+ }
+ }
+
mutex_ = &mutex;
}
@@ -115,6 +134,7 @@ public:
/// \todo Disable in non-debug build
bool locked() const;
+private:
/// \brief Lock the mutex
///
/// This method blocks until the mutex can be locked.
diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
index 7e15a20..0b4d3ce 100644
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -25,24 +25,6 @@ using namespace isc::util::thread;
namespace {
-TEST(MutexTest, direct) {
- Mutex mutex;
- EXPECT_FALSE(mutex.locked()); // Debug-only build
-
- mutex.lock();
- EXPECT_TRUE(mutex.locked()); // Debug-only build
-
- EXPECT_FALSE(mutex.tryLock());
-
- mutex.unlock();
- EXPECT_FALSE(mutex.locked()); // Debug-only build
-
- EXPECT_TRUE(mutex.tryLock());
-
- mutex.unlock();
- EXPECT_FALSE(mutex.locked()); // Debug-only build
-}
-
// If we try to lock the debug mutex multiple times, it should throw.
TEST(MutexTest, lockMultiple) {
// TODO: Once we support non-debug mutexes, disable the test if we compile
More information about the bind10-changes
mailing list