From: Mukund Sivaraman Date: Tue, 16 Oct 2012 19:31:14 +0000 (+0530) Subject: Revert "Merge branch 'master' into trac2198_2" X-Git-Tag: trac2402_base~21 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2252c5776f5447f47e567262f8a5a05753fc6ce9;p=thirdparty%2Fkea.git Revert "Merge branch 'master' into trac2198_2" This reverts commit f8181bb72251fd3d5f33e157d1f2a2a75a58d765, reversing changes made to 453b58784344390cbc5375624e6e92a86b4ff3f1. --- diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am index 99b54eb4fa..13f8f7b595 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 6da3f146d6..d045449025 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 - -#include #include #include @@ -26,38 +23,9 @@ #include #include -using namespace isc::util::thread; - namespace isc { namespace util { -namespace { // unnamed namespace - -typedef std::map > 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 5cb64c1c05..fd8da1b438 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 -#include #include -#include - 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 MutexPtr; - typedef boost::shared_ptr 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 1a9c96a6d1..105322ff2f 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 242cc5b7cf..9a1b02511a 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 -#include - +#include "util/interprocess_sync_file.h" #include -#include - #include -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/lock.cc b/src/lib/util/threads/lock.cc index acc73d6f69..6a165aa531 100644 --- a/src/lib/util/threads/lock.cc +++ b/src/lib/util/threads/lock.cc @@ -119,19 +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); - if (result == EBUSY) { - 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 4992bdb766..fef537b9a5 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 #include #include // 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 8d337d59be..0b4d3ce4ec 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 6caa6bf26c..451ab4e34c 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