BIND 10 master, updated. 2252c5776f5447f47e567262f8a5a05753fc6ce9 Revert "Merge branch 'master' into trac2198_2"
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 16 19:54:01 UTC 2012
The branch, master has been updated
via 2252c5776f5447f47e567262f8a5a05753fc6ce9 (commit)
via f559b99dcccd35703989fce985ff200cd664ccc0 (commit)
via c91151db16e215692830bf9b6211c22aae8619dc (commit)
via 7ffd1e0cad3bd125b490953dbea9f22c67e40fae (commit)
via 34701adba7f7390640c70ba769cdb0b20f01f82c (commit)
from 42c638e07a45b69d902a51b35838ae21ac19d116 (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 2252c5776f5447f47e567262f8a5a05753fc6ce9
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 17 01:01:14 2012 +0530
Revert "Merge branch 'master' into trac2198_2"
This reverts commit f8181bb72251fd3d5f33e157d1f2a2a75a58d765, reversing
changes made to 453b58784344390cbc5375624e6e92a86b4ff3f1.
commit f559b99dcccd35703989fce985ff200cd664ccc0
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 17 00:59:03 2012 +0530
Revert "[master] Link -lpthread in libb10-threads target"
This reverts commit 8d321e3ea9f85227296cfac94a9ce79c7db70fdf.
commit c91151db16e215692830bf9b6211c22aae8619dc
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 17 00:58:45 2012 +0530
Revert "[master] Check for EDEADLK too when using Mutex::tryLock()"
This reverts commit 42c638e07a45b69d902a51b35838ae21ac19d116.
commit 7ffd1e0cad3bd125b490953dbea9f22c67e40fae
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 17 00:58:31 2012 +0530
Revert "[master] Avoid static destruction fiasco with InterprocessSyncFile and logger"
This reverts commit 34701adba7f7390640c70ba769cdb0b20f01f82c.
commit 34701adba7f7390640c70ba769cdb0b20f01f82c
Author: Mukund Sivaraman <muks at isc.org>
Date: Wed Oct 17 00:17:08 2012 +0530
[master] Avoid static destruction fiasco with InterprocessSyncFile and logger
-----------------------------------------------------------------------
Summary of changes:
src/lib/util/Makefile.am | 6 +-
src/lib/util/interprocess_sync_file.cc | 74 ++------------------
src/lib/util/interprocess_sync_file.h | 12 +---
src/lib/util/tests/Makefile.am | 1 -
.../util/tests/interprocess_sync_file_unittest.cc | 47 +------------
src/lib/util/threads/Makefile.am | 3 +-
src/lib/util/threads/lock.cc | 19 -----
src/lib/util/threads/lock.h | 55 ++-------------
src/lib/util/threads/tests/lock_unittest.cc | 38 ----------
src/lib/util/unittests/Makefile.am | 3 +-
10 files changed, 18 insertions(+), 240 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am
index 99b54eb..13f8f7b 100644
--- a/src/lib/util/Makefile.am
+++ b/src/lib/util/Makefile.am
@@ -1,5 +1,4 @@
-# InterprocessSyncFile has a dependency on isc::util::thread::Mutex
-SUBDIRS = io unittests threads . tests pyunittests python
+SUBDIRS = . io unittests tests pyunittests python threads
AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
AM_CPPFLAGS += -I$(top_srcdir)/src/lib/util -I$(top_builddir)/src/lib/util
@@ -32,8 +31,7 @@ libb10_util_la_SOURCES += random/random_number_generator.h
EXTRA_DIST = python/pycppwrapper_util.h
-libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-libb10_util_la_LIBADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
+libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
CLEANFILES = *.gcno *.gcda
libb10_util_includedir = $(includedir)/$(PACKAGE_NAME)/util
diff --git a/src/lib/util/interprocess_sync_file.cc b/src/lib/util/interprocess_sync_file.cc
index 6da3f14..d045449 100644
--- a/src/lib/util/interprocess_sync_file.cc
+++ b/src/lib/util/interprocess_sync_file.cc
@@ -14,9 +14,6 @@
#include "interprocess_sync_file.h"
-#include <boost/weak_ptr.hpp>
-
-#include <map>
#include <string>
#include <stdlib.h>
@@ -26,38 +23,9 @@
#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;
-
-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.
@@ -65,23 +33,6 @@ InterprocessSyncFile::~InterprocessSyncFile() {
// The lockfile will continue to exist, and we must not delete
// it.
}
-
- Mutex::Locker locker(sync_map_mutex);
-
- // 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());
-
- if (it->second.expired()) {
- sync_map.erase(it);
- }
-
- // Lock on sync_map_mutex is automatically unlocked during
- // destruction when basic block is exited.
}
bool
@@ -139,13 +90,8 @@ InterprocessSyncFile::lock() {
return (true);
}
- // First grab the thread lock...
- LockerPtr locker(new Mutex::Locker(*mutex_));
-
- // ... then the file lock.
if (do_lock(F_SETLKW, F_WRLCK)) {
is_locked_ = true;
- locker_ = locker;
return (true);
}
@@ -158,18 +104,8 @@ InterprocessSyncFile::tryLock() {
return (true);
}
- // First grab the thread lock...
- LockerPtr locker;
- try {
- locker.reset(new Mutex::Locker(*mutex_, false));
- } catch (const Mutex::Locker::AlreadyLocked&) {
- return (false);
- }
-
- // ... then the file lock.
if (do_lock(F_SETLK, F_WRLCK)) {
is_locked_ = true;
- locker_ = locker;
return (true);
}
@@ -182,14 +118,12 @@ InterprocessSyncFile::unlock() {
return (true);
}
- // First release the file lock...
- if (do_lock(F_SETLKW, F_UNLCK) == 0) {
- return (false);
+ if (do_lock(F_SETLKW, F_UNLCK)) {
+ is_locked_ = false;
+ return (true);
}
- locker_.reset();
- is_locked_ = false;
- return (true);
+ return (false);
}
} // namespace util
diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h
index 5cb64c1..fd8da1b 100644
--- a/src/lib/util/interprocess_sync_file.h
+++ b/src/lib/util/interprocess_sync_file.h
@@ -16,11 +16,8 @@
#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 {
@@ -58,7 +55,9 @@ 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);
+ InterprocessSyncFile(const std::string& task_name) :
+ InterprocessSync(task_name), fd_(-1)
+ {}
/// \brief Destructor
virtual ~InterprocessSyncFile();
@@ -84,11 +83,6 @@ 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;
- 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/tests/Makefile.am b/src/lib/util/tests/Makefile.am
index 1a9c96a..105322f 100644
--- a/src/lib/util/tests/Makefile.am
+++ b/src/lib/util/tests/Makefile.am
@@ -46,7 +46,6 @@ run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
run_unittests_LDADD = $(top_builddir)/src/lib/util/libb10-util.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/threads/libb10-threads.la
run_unittests_LDADD += $(top_builddir)/src/lib/util/io/libb10-util-io.la
run_unittests_LDADD += \
$(top_builddir)/src/lib/util/unittests/libutil_unittests.la
diff --git a/src/lib/util/tests/interprocess_sync_file_unittest.cc b/src/lib/util/tests/interprocess_sync_file_unittest.cc
index 242cc5b..9a1b025 100644
--- a/src/lib/util/tests/interprocess_sync_file_unittest.cc
+++ b/src/lib/util/tests/interprocess_sync_file_unittest.cc
@@ -12,15 +12,10 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-#include <util/interprocess_sync_file.h>
-#include <util/threads/thread.h>
-
+#include "util/interprocess_sync_file.h"
#include <gtest/gtest.h>
-#include <boost/bind.hpp>
-
#include <unistd.h>
-using namespace isc::util::thread;
using namespace std;
namespace isc {
@@ -113,46 +108,6 @@ TEST(InterprocessSyncFileTest, TestLock) {
EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
}
-void
-threadTest(bool* locked)
-{
- InterprocessSyncFile sync2("test");
- InterprocessSyncLocker locker2(sync2);
-
- *locked = !locker2.tryLock();
-}
-
-TEST(InterprocessSyncFileTest, TestLockThreaded) {
- InterprocessSyncFile sync("test");
- InterprocessSyncLocker locker(sync);
-
- EXPECT_FALSE(locker.isLocked());
- EXPECT_TRUE(locker.lock());
- EXPECT_TRUE(locker.isLocked());
-
- bool locked_in_other_thread = false;
-
- // Here, we check that a lock has been taken by creating a new
- // thread and checking from the new thread that a lock exists. The
- // lock attempt must fail to pass our check.
- Thread thread(boost::bind(&threadTest, &locked_in_other_thread));
- thread.wait();
-
- EXPECT_TRUE(locked_in_other_thread);
-
- // Release the lock and try again. This time, the attempt must pass.
-
- EXPECT_TRUE(locker.unlock());
- EXPECT_FALSE(locker.isLocked());
-
- Thread thread2(boost::bind(&threadTest, &locked_in_other_thread));
- thread2.wait();
-
- EXPECT_FALSE(locked_in_other_thread);
-
- EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
-}
-
TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
InterprocessSyncFile sync("test1");
InterprocessSyncLocker locker(sync);
diff --git a/src/lib/util/threads/Makefile.am b/src/lib/util/threads/Makefile.am
index e318023..7b9fb8f 100644
--- a/src/lib/util/threads/Makefile.am
+++ b/src/lib/util/threads/Makefile.am
@@ -7,7 +7,6 @@ AM_CPPFLAGS += $(BOOST_INCLUDES) $(MULTITHREADING_FLAG)
lib_LTLIBRARIES = libb10-threads.la
libb10_threads_la_SOURCES = lock.h lock.cc
libb10_threads_la_SOURCES += thread.h thread.cc
-libb10_threads_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-libb10_threads_la_LIBADD += $(PTHREAD_LDFLAGS)
+libb10_threads_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
CLEANFILES = *.gcno *.gcda
diff --git a/src/lib/util/threads/lock.cc b/src/lib/util/threads/lock.cc
index c3c818e..6a165aa 100644
--- a/src/lib/util/threads/lock.cc
+++ b/src/lib/util/threads/lock.cc
@@ -119,25 +119,6 @@ Mutex::lock() {
++impl_->locked_count; // Only in debug mode
}
-bool
-Mutex::tryLock() {
- assert(impl_ != NULL);
- const int result = pthread_mutex_trylock(&impl_->mutex);
-
- // In the case of pthread_mutex_trylock(), if it is called on a
- // locked mutex from the same thread, some platforms (such as fedora
- // and debian) return EBUSY whereas others (such as centos 5) return
- // EDEADLK. We return false and don't pass the lock attempt in both
- // cases.
- if (result == EBUSY || result == EDEADLK) {
- return (false);
- } else if (result != 0) {
- isc_throw(isc::InvalidOperation, std::strerror(result));
- }
- ++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 4992bdb..fef537b 100644
--- a/src/lib/util/threads/lock.h
+++ b/src/lib/util/threads/lock.h
@@ -15,7 +15,6 @@
#ifndef B10_THREAD_LOCK_H
#define B10_THREAD_LOCK_H
-#include <exceptions/exceptions.h>
#include <boost/noncopyable.hpp>
#include <cstdlib> // for NULL.
@@ -77,39 +76,21 @@ 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 if
- /// \c block is true.
+ /// Locks the mutex. May block for extended period of time.
///
/// \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).
- /// \throw AlreadyLocked if \c block is false and the mutex is
- /// already locked.
- Locker(Mutex& mutex, bool block = true) :
+ Locker(Mutex& mutex) :
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.
- if (block) {
- mutex.lock();
- } else {
- if (!mutex.tryLock()) {
- isc_throw(AlreadyLocked, "The mutex is already locked");
- }
- }
-
+ mutex.lock();
mutex_ = &mutex;
}
@@ -124,7 +105,6 @@ public:
private:
Mutex* mutex_;
};
-
/// \brief If the mutex is currently locked
///
/// This is debug aiding method only. And it might be unavailable in
@@ -133,36 +113,11 @@ public:
///
/// \todo Disable in non-debug build
bool locked() const;
-
-private:
- /// \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 8d337d5..0b4d3ce 100644
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -37,44 +37,6 @@ TEST(MutexTest, lockMultiple) {
Mutex::Locker l2(mutex); // Attempt to lock again.
}, isc::InvalidOperation);
EXPECT_TRUE(mutex.locked()); // Debug-only build
-
- // block=true explicitly.
- Mutex mutex2;
- EXPECT_FALSE(mutex2.locked()); // Debug-only build
- Mutex::Locker l12(mutex2, true);
- EXPECT_TRUE(mutex2.locked()); // Debug-only build
-}
-
-void
-testThread(Mutex* mutex)
-{
- // block=false (tryLock). This should not block indefinitely, but
- // throw AlreadyLocked. If block were true, this would block
- // indefinitely here.
- EXPECT_THROW({
- Mutex::Locker l3(*mutex, false);
- }, Mutex::Locker::AlreadyLocked);
-
- EXPECT_TRUE(mutex->locked()); // Debug-only build
-}
-
-// Test the non-blocking variant using a second thread.
-TEST(MutexTest, lockNonBlocking) {
- // block=false (tryLock).
- Mutex mutex;
- Mutex::Locker l1(mutex, false);
- EXPECT_TRUE(mutex.locked()); // Debug-only build
-
- // First, try another locker from the same thread.
- EXPECT_THROW({
- Mutex::Locker l2(mutex, false);
- }, Mutex::Locker::AlreadyLocked);
-
- EXPECT_TRUE(mutex.locked()); // Debug-only build
-
- // Now try another locker from a different thread.
- Thread thread(boost::bind(&testThread, &mutex));
- thread.wait();
}
// Destroying a locked mutex is a bad idea as well
diff --git a/src/lib/util/unittests/Makefile.am b/src/lib/util/unittests/Makefile.am
index 6caa6bf..451ab4e 100644
--- a/src/lib/util/unittests/Makefile.am
+++ b/src/lib/util/unittests/Makefile.am
@@ -20,7 +20,8 @@ if HAVE_GTEST
libutil_unittests_la_CPPFLAGS += $(GTEST_INCLUDES)
endif
-libutil_unittests_la_LIBADD = $(top_builddir)/src/lib/util/io/libb10-util-io.la
+libutil_unittests_la_LIBADD = $(top_builddir)/src/lib/util/libb10-util.la
+libutil_unittests_la_LIBADD += $(top_builddir)/src/lib/util/io/libb10-util-io.la
libutil_unittests_la_LIBADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
CLEANFILES = *.gcno *.gcda
More information about the bind10-changes
mailing list