- use flock to lock data dir rather than a lock file.

 - removed troublesome global constructor in Mutex initialization.


git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk/qpid@671604 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 78 insertions, 64 deletions

diff --git a/cpp/src/Makefile.am b/cpp/src/Makefile.am
index 26f6dc7964..12bca2396b 100644
--- a/cpp/src/Makefile.am
+++ b/cpp/src/Makefile.am
@@ -76,7 +76,8 @@ posix_plat_src = \
   qpid/sys/posix/AsynchIO.cpp \
   qpid/sys/posix/Time.cpp \
   qpid/sys/posix/Thread.cpp \
-  qpid/sys/posix/Shlib.cpp
+  qpid/sys/posix/Shlib.cpp \
+  qpid/sys/posix/Mutex.cpp
 
 posix_plat_hdr = \
   qpid/sys/posix/check.h \
diff --git a/cpp/src/qpid/DataDir.cpp b/cpp/src/qpid/DataDir.cpp
index e9c6aaad53..879f4de202 100644
--- a/cpp/src/qpid/DataDir.cpp
+++ b/cpp/src/qpid/DataDir.cpp
@@ -23,6 +23,7 @@
 #include "qpid/log/Statement.h"
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/file.h>
 #include <fcntl.h>
 #include <cerrno>
 
@@ -30,7 +31,8 @@ namespace qpid {
 
 DataDir::DataDir (std::string path) :
     enabled (!path.empty ()),
-    dirPath (path)
+    dirPath (path),
+    dirFd(-1)
 {
     if (!enabled)
     {
@@ -40,7 +42,6 @@ DataDir::DataDir (std::string path) :
 
     const  char *cpath = dirPath.c_str ();
     struct stat  s;
-
     if (::stat(cpath, &s)) {
         if (errno == ENOENT) {
             if (::mkdir(cpath, 0755))
@@ -49,34 +50,23 @@ DataDir::DataDir (std::string path) :
         else
             throw Exception ("Data directory not found: " + path);
     }
-
-    std::string lockFile (path);
-    lockFile = lockFile + "/lock";
-    int fd = ::open (lockFile.c_str (), O_CREAT | O_EXCL,
-                     S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-    if (fd == -1)
-    {
-        if (errno == EEXIST)
-            throw Exception ("Data directory is locked by another process: " + path);
-        if (errno == EACCES)
-            throw Exception ("Insufficient privileges for data directory: " + path);
-        throw Exception(
-            QPID_MSG("Error locking " << lockFile << ": " << strError(errno)));
+    int dirFd = ::open(path.c_str(), 0);
+    if (dirFd == -1)
+        throw Exception(QPID_MSG("Can't open data directory: " << dirPath << ": " << strError(errno)));
+    int result = ::flock(dirFd, LOCK_EX | LOCK_NB);
+    if (result != 0) {
+        if (errno == EWOULDBLOCK)
+            throw Exception(QPID_MSG("Data directory locked by another process: " << path));
+        throw  Exception(QPID_MSG("Cannot lock data directory: " << strError(errno)));
     }
-
     QPID_LOG (info, "Locked data directory: " << dirPath);
 }
 
-DataDir::~DataDir ()
-{
-    if (dirPath.empty ())
-        return;
-
-    std::string lockFile (dirPath);
-    lockFile = lockFile + "/lock";
-
-    ::unlink (lockFile.c_str ());
-    QPID_LOG (info, "Unlocked data directory: " << dirPath);
+DataDir::~DataDir () {
+    if (dirFd != -1) {
+        ::close(dirFd);         // Closing the fd unlocks the directory.
+        QPID_LOG (info, "Unlocked data directory: " << dirPath);
+    }
 }
 
 } // namespace qpid
diff --git a/cpp/src/qpid/DataDir.h b/cpp/src/qpid/DataDir.h
index 56aa4f26d7..6bd27fcda2 100644
--- a/cpp/src/qpid/DataDir.h
+++ b/cpp/src/qpid/DataDir.h
@@ -32,6 +32,7 @@ class DataDir
 {
     const bool        enabled;
     const std::string dirPath;
+    int dirFd;
 
   public:
 
diff --git a/cpp/src/qpid/broker/Daemon.h b/cpp/src/qpid/broker/Daemon.h
index 7d683232b1..98468debb7 100644
--- a/cpp/src/qpid/broker/Daemon.h
+++ b/cpp/src/qpid/broker/Daemon.h
@@ -72,6 +72,7 @@ class Daemon : private boost::noncopyable
 
     pid_t pid;
     int pipeFds[2];
+    int lockFileFd;
     std::string lockFile;
     std::string pidDir;
 };
diff --git a/cpp/src/qpid/cluster/Cpg.h b/cpp/src/qpid/cluster/Cpg.h
index 2a7ec459d3..1ed362f94e 100644
--- a/cpp/src/qpid/cluster/Cpg.h
+++ b/cpp/src/qpid/cluster/Cpg.h
@@ -149,19 +149,19 @@ class Cpg : public Dispatchable {
     }
 
     static void globalDeliver(
-        cpg_handle_t /*handle*/,
+        cpg_handle_t handle,
         struct cpg_name *group,
-        uint32_t /*nodeid*/,
-        uint32_t /*pid*/,
-        void* /*msg*/,
-        int /*msg_len*/);
+        uint32_t nodeid,
+        uint32_t pid,
+        void* msg,
+        int msg_len);
 
     static void globalConfigChange(
-        cpg_handle_t /*handle*/,
-        struct cpg_name */*group*/,
-        struct cpg_address */*members*/, int /*nMembers*/,
-        struct cpg_address */*left*/, int /*nLeft*/,
-        struct cpg_address */*joined*/, int /*nJoined*/
+        cpg_handle_t handle,
+        struct cpg_name *group,
+        struct cpg_address *members, int nMembers,
+        struct cpg_address *left, int nLeft,
+        struct cpg_address *joined, int nJoined
     );
 
     static Handles handles;
diff --git a/cpp/src/qpid/sys/posix/Mutex.cpp b/cpp/src/qpid/sys/posix/Mutex.cpp
new file mode 100644
index 0000000000..0e1f0d30c2
--- /dev/null
+++ b/cpp/src/qpid/sys/posix/Mutex.cpp
@@ -0,0 +1,46 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+#include "qpid/sys/Mutex.h"
+
+namespace qpid {
+namespace sys {
+
+/**
+ * Initialise a recursive mutex attr for use in creating mutexes later
+ * (we use pthread_once to make sure it is initialised exactly once)
+ */
+
+namespace {
+pthread_once_t  onceControl = PTHREAD_ONCE_INIT;
+pthread_mutexattr_t mutexattr;
+	
+void initMutexattr()  {
+    pthread_mutexattr_init(&mutexattr);
+    pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_RECURSIVE);
+}
+}
+
+const pthread_mutexattr_t* Mutex::getAttribute() {
+    pthread_once(&onceControl, initMutexattr);
+    return &mutexattr;
+}
+
+}} // namespace qpid::sys
diff --git a/cpp/src/qpid/sys/posix/Mutex.h b/cpp/src/qpid/sys/posix/Mutex.h
index 9414f39d8f..cd5a8affd4 100644
--- a/cpp/src/qpid/sys/posix/Mutex.h
+++ b/cpp/src/qpid/sys/posix/Mutex.h
@@ -34,6 +34,7 @@ class Condition;
  */
 class Mutex : private boost::noncopyable {
     friend class Condition;
+    static const pthread_mutexattr_t* getAttribute();
 
 public:
     typedef ::qpid::sys::ScopedLock<Mutex> ScopedLock;
@@ -74,32 +75,6 @@ protected:
 
 
 /**
- * Initialise a recursive mutex attr for use in creating mutexes later
- * (we use pthread_once to make sure it is initialised exactly once)
- */
-namespace {
-	pthread_once_t  onceControl = PTHREAD_ONCE_INIT;
-	pthread_mutexattr_t mutexattr;
-	
-	void initMutexattr()  {
-		pthread_mutexattr_init(&mutexattr);
-		pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_RECURSIVE);
-	}
-
-	struct RecursiveMutexattr {
-		RecursiveMutexattr() {
-			pthread_once(&onceControl, initMutexattr);
-		}
-		
-		operator const pthread_mutexattr_t*() const {
-			return &mutexattr;
-		}
-	};
-  
-	RecursiveMutexattr recursiveMutexattr;
-}
-
-/**
  * PODMutex is a POD, can be static-initialized with
  * PODMutex m = QPID_PODMUTEX_INITIALIZER
  */
@@ -130,7 +105,7 @@ bool PODMutex::trylock() {
 }
 
 Mutex::Mutex() {
-    QPID_POSIX_ASSERT_THROW_IF(pthread_mutex_init(&mutex, recursiveMutexattr));
+    QPID_POSIX_ASSERT_THROW_IF(pthread_mutex_init(&mutex, getAttribute()));
 }
 
 Mutex::~Mutex(){