[2198] First use a mutex for mutual exclusion among threads, then a lock file

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)) {
+    // First grab the thread lock...
-        is_locked_ = true;
+    mutex_->lock();
-        return (true);
+
+    // ... 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)) {
+    // First grab the thread lock...
-        is_locked_ = true;
+    if (!mutex_->tryLock()) {
-        return (true);
+        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)) {
+    // First release the file lock...
-        is_locked_ = false;
+    if (do_lock(F_SETLKW, F_UNLCK) == 0) {
-        return (true);
+        return (false);
     }
 
-    return (false);
+    mutex_->unlock();
+    is_locked_ = false;
+    return (true);
 }
 
 } // namespace util

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) :
+    InterprocessSyncFile(const std::string& task_name);
-        InterprocessSync(task_name), fd_(-1)
-    {}
 
     /// \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