BIND 10 trac2198_2, updated. c0d25c429a4f2026bb97b9e013730b8f265862e6 [2198] First use a mutex for mutual exclusion among threads, then a lock file
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 9 21:36:05 UTC 2012
The branch, trac2198_2 has been updated
via c0d25c429a4f2026bb97b9e013730b8f265862e6 (commit)
via e71cd13c5cb261635f7115e0d30fbd7dcc5fe151 (commit)
from 66424553f3fa05e46e558f3b4e16677463e0e412 (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 c0d25c429a4f2026bb97b9e013730b8f265862e6
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 10 03:05:46 2012 +0530
[2198] First use a mutex for mutual exclusion among threads, then a lock file
commit e71cd13c5cb261635f7115e0d30fbd7dcc5fe151
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 10 03:03:09 2012 +0530
[2198] Add Mutex::tryLock() and make lock methods public
tryLock() is necessary inside InterprocessSyncFile. It also doesn't
make sense afterwards to make a dynamically constructed Mutex::Locker
that lives after the basic block is exited. So lock() and unlock()
have also been made public.
I've added comments to discourage the use of these methods directly
and use the Mutex::Locker where applicable.
-----------------------------------------------------------------------
Summary of changes:
src/lib/util/interprocess_sync_file.cc | 95 +++++++++++++++++++++++----
src/lib/util/interprocess_sync_file.h | 10 ++-
src/lib/util/threads/lock.cc | 11 ++++
src/lib/util/threads/lock.h | 29 +++++++-
src/lib/util/threads/tests/lock_unittest.cc | 18 +++++
5 files changed, 147 insertions(+), 16 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/util/interprocess_sync_file.cc b/src/lib/util/interprocess_sync_file.cc
index d045449..71d3287 100644
--- a/src/lib/util/interprocess_sync_file.cc
+++ b/src/lib/util/interprocess_sync_file.cc
@@ -12,8 +12,11 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-#include "interprocess_sync_file.h"
+#include <util/interprocess_sync_file.h>
+#include <boost/weak_ptr.hpp>
+
+#include <map>
#include <string>
#include <stdlib.h>
@@ -23,9 +26,39 @@
#include <sys/types.h>
#include <sys/stat.h>
+using namespace isc::util::thread;
+
namespace isc {
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;
+
+} // end of unnamed namespace
+
+InterprocessSyncFile::InterprocessSyncFile(const std::string& task_name) :
+ InterprocessSync(task_name),
+ fd_(-1)
+{
+ Mutex::Locker locker(sync_map_mutex);
+
+ SyncMap::iterator it = sync_map.find(task_name);
+ if (it != sync_map.end()) {
+ mutex_ = it->second.lock();
+ } else {
+ mutex_.reset(new Mutex());
+ sync_map[task_name] = mutex_;
+ }
+
+ // Lock on sync_map_mutex is automatically unlocked during
+ // destruction when basic block is exited.
+}
+
InterprocessSyncFile::~InterprocessSyncFile() {
if (fd_ != -1) {
// This will also release any applied locks.
@@ -33,6 +66,21 @@ InterprocessSyncFile::~InterprocessSyncFile() {
// The lockfile will continue to exist, and we must not delete
// it.
}
+
+ Mutex::Locker locker(sync_map_mutex);
+
+ // Unref the shared mutex first.
+ mutex_.reset();
+
+ SyncMap::iterator it = sync_map.find(task_name_);
+ assert(it != sync_map.end());
+
+ if (it->second.expired()) {
+ sync_map.erase(it);
+ }
+
+ // Lock on sync_map_mutex is automatically unlocked during
+ // destruction when basic block is exited.
}
bool
@@ -90,11 +138,21 @@ InterprocessSyncFile::lock() {
return (true);
}
- if (do_lock(F_SETLKW, F_WRLCK)) {
- is_locked_ = true;
- return (true);
+ // First grab the thread lock...
+ mutex_->lock();
+
+ // ... then the file lock.
+ try {
+ if (do_lock(F_SETLKW, F_WRLCK)) {
+ is_locked_ = true;
+ return (true);
+ }
+ } catch (...) {
+ mutex_->unlock();
+ throw;
}
+ mutex_->unlock();
return (false);
}
@@ -104,11 +162,24 @@ InterprocessSyncFile::tryLock() {
return (true);
}
- if (do_lock(F_SETLK, F_WRLCK)) {
- is_locked_ = true;
- return (true);
+ // First grab the thread lock...
+ if (!mutex_->tryLock()) {
+ 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;
}
+ mutex_->unlock();
return (false);
}
@@ -118,12 +189,14 @@ InterprocessSyncFile::unlock() {
return (true);
}
- if (do_lock(F_SETLKW, F_UNLCK)) {
- is_locked_ = false;
- return (true);
+ // First release the file lock...
+ if (do_lock(F_SETLKW, F_UNLCK) == 0) {
+ return (false);
}
- return (false);
+ mutex_->unlock();
+ is_locked_ = false;
+ return (true);
}
} // namespace util
diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h
index fd8da1b..95f0c28 100644
--- a/src/lib/util/interprocess_sync_file.h
+++ b/src/lib/util/interprocess_sync_file.h
@@ -16,8 +16,11 @@
#define __INTERPROCESS_SYNC_FILE_H__
#include <util/interprocess_sync.h>
+#include <util/threads/lock.h>
#include <exceptions/exceptions.h>
+#include <boost/shared_ptr.hpp>
+
namespace isc {
namespace util {
@@ -55,9 +58,7 @@ public:
/// \param name Name of the synchronization task. This has to be
/// identical among the various processes that need to be
/// synchronized for the same task.
- InterprocessSyncFile(const std::string& task_name) :
- InterprocessSync(task_name), fd_(-1)
- {}
+ InterprocessSyncFile(const std::string& task_name);
/// \brief Destructor
virtual ~InterprocessSyncFile();
@@ -83,6 +84,9 @@ private:
bool do_lock(int cmd, short l_type);
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
};
} // namespace util
diff --git a/src/lib/util/threads/lock.cc b/src/lib/util/threads/lock.cc
index 6a165aa..ab6bd79 100644
--- a/src/lib/util/threads/lock.cc
+++ b/src/lib/util/threads/lock.cc
@@ -119,6 +119,17 @@ Mutex::lock() {
++impl_->locked_count; // Only in debug mode
}
+bool
+Mutex::tryLock() {
+ assert(impl_ != NULL);
+ const int result = pthread_mutex_trylock(&impl_->mutex);
+ if (result != 0) {
+ return (false);
+ }
+ ++impl_->locked_count; // Only in debug mode
+ return (true);
+}
+
void
Mutex::unlock() {
assert(impl_ != NULL);
diff --git a/src/lib/util/threads/lock.h b/src/lib/util/threads/lock.h
index fef537b..8426a1a 100644
--- a/src/lib/util/threads/lock.h
+++ b/src/lib/util/threads/lock.h
@@ -105,6 +105,7 @@ public:
private:
Mutex* mutex_;
};
+
/// \brief If the mutex is currently locked
///
/// This is debug aiding method only. And it might be unavailable in
@@ -113,11 +114,35 @@ public:
///
/// \todo Disable in non-debug build
bool locked() const;
+
+ /// \brief Lock the mutex
+ ///
+ /// This method blocks until the mutex can be locked.
+ ///
+ /// Please consider not using this method directly and instead using
+ /// a Mutex::Locker object instead.
+ void lock();
+
+ /// \brief Try to lock the mutex
+ ///
+ /// This method doesn't block and returns immediately with a status
+ /// on whether the lock operation was successful.
+ ///
+ /// Please consider not using this method directly and instead using
+ /// a Mutex::Locker object instead.
+ ///
+ /// \return true if the lock was successful, false otherwise.
+ bool tryLock();
+
+ /// \brief Unlock the mutex
+ ///
+ /// Please consider not using this method directly and instead using
+ /// a Mutex::Locker object instead.
+ void unlock();
+
private:
class Impl;
Impl* impl_;
- void lock();
- void unlock();
};
diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
index 0b4d3ce..7e15a20 100644
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -25,6 +25,24 @@ 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