Browse Source

Revert "Merge branch 'master' into trac2198_2"

This reverts commit f8181bb72251fd3d5f33e157d1f2a2a75a58d765, reversing
changes made to 453b58784344390cbc5375624e6e92a86b4ff3f1.
Mukund Sivaraman 12 years ago
parent
commit
2252c5776f

+ 2 - 4
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

+ 4 - 70
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

+ 3 - 9
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

+ 0 - 1
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

+ 1 - 46
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);

+ 0 - 13
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);

+ 5 - 50
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();
 };
 
 

+ 0 - 38
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

+ 2 - 1
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