QPID-1725: Revised some lock scopes in management and link registry to avoid deadlocks.

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

3 files changed, 32 insertions, 27 deletions

diff --git a/qpid/cpp/src/qpid/broker/LinkRegistry.cpp b/qpid/cpp/src/qpid/broker/LinkRegistry.cpp
index d4ae0aedd8..7d4ab7548e 100644
--- a/qpid/cpp/src/qpid/broker/LinkRegistry.cpp
+++ b/qpid/cpp/src/qpid/broker/LinkRegistry.cpp
@@ -238,67 +238,70 @@ MessageStore* LinkRegistry::getStore() const {
     return store;
 }
 
-void LinkRegistry::notifyConnection(const std::string& key, Connection* c)
+Link::shared_ptr LinkRegistry::findLink(const std::string& key)
 {
     Mutex::ScopedLock locker(lock);
     LinkMap::iterator l = links.find(key);
-    if (l != links.end())
-    {
-        l->second->established();
-        l->second->setConnection(c);
-        c->setUserId(str(format("%1%@%2%") % l->second->getUsername() % realm));
+    if (l != links.end()) return l->second;
+    else return Link::shared_ptr();
+}
+
+void LinkRegistry::notifyConnection(const std::string& key, Connection* c)
+{
+    Link::shared_ptr link = findLink(key);
+    if (link) {
+        link->established();
+        link->setConnection(c);
+        c->setUserId(str(format("%1%@%2%") % link->getUsername() % realm));
     }
 }
 
 void LinkRegistry::notifyClosed(const std::string& key)
 {
-    Mutex::ScopedLock locker(lock);
-    LinkMap::iterator l = links.find(key);
-    if (l != links.end())
-        l->second->closed(0, "Closed by peer");
+    Link::shared_ptr link = findLink(key);
+    if (link) {
+        link->closed(0, "Closed by peer");
+    }
 }
 
 void LinkRegistry::notifyConnectionForced(const std::string& key, const std::string& text)
 {
-    Mutex::ScopedLock locker(lock);
-    LinkMap::iterator l = links.find(key);
-    if (l != links.end())
-        l->second->notifyConnectionForced(text);
+    Link::shared_ptr link = findLink(key);
+    if (link) {
+        link->notifyConnectionForced(text);
+    }
 }
 
 std::string LinkRegistry::getAuthMechanism(const std::string& key)
 {
-    Mutex::ScopedLock locker(lock);
-    LinkMap::iterator l = links.find(key);
-    if (l != links.end())
-        return l->second->getAuthMechanism();
+    Link::shared_ptr link = findLink(key);
+    if (link)
+        return link->getAuthMechanism();
     return string("ANONYMOUS");
 }
 
 std::string LinkRegistry::getAuthCredentials(const std::string& key)
 {
-    Mutex::ScopedLock locker(lock);
-    LinkMap::iterator l = links.find(key);
-    if (l == links.end())
+    Link::shared_ptr link = findLink(key);
+    if (!link)
         return string();
 
     string result;
     result += '\0';
-    result += l->second->getUsername();
+    result += link->getUsername();
     result += '\0';
-    result += l->second->getPassword();
+    result += link->getPassword();
 
     return result;
 }
 
 std::string LinkRegistry::getAuthIdentity(const std::string& key)
 {
-    Mutex::ScopedLock locker(lock);
-    LinkMap::iterator l = links.find(key);
-    if (l == links.end())
+    Link::shared_ptr link = findLink(key);
+    if (!link)
         return string();
 
-    return l->second->getUsername();
+    return link->getUsername();
 }
 
 
diff --git a/qpid/cpp/src/qpid/broker/LinkRegistry.h b/qpid/cpp/src/qpid/broker/LinkRegistry.h
index 92064692f5..2397dbc6f3 100644
--- a/qpid/cpp/src/qpid/broker/LinkRegistry.h
+++ b/qpid/cpp/src/qpid/broker/LinkRegistry.h
@@ -70,6 +70,7 @@ namespace broker {
 
         void periodicMaintenance ();
         bool updateAddress(const std::string& oldKey, const TcpAddress& newAddress);
+        Link::shared_ptr findLink(const std::string& key);
         static std::string createKey(const TcpAddress& address);
 
     public:
diff --git a/qpid/cpp/src/qpid/management/ManagementBroker.cpp b/qpid/cpp/src/qpid/management/ManagementBroker.cpp
index 729826e0cd..19300ef1af 100644
--- a/qpid/cpp/src/qpid/management/ManagementBroker.cpp
+++ b/qpid/cpp/src/qpid/management/ManagementBroker.cpp
@@ -518,6 +518,7 @@ void ManagementBroker::handleMethodRequestLH (Buffer& inBuffer, string replyToKe
         else
             try {
                 outBuffer.record();
+                Mutex::ScopedUnlock u(userLock);
                 iter->second->doMethod(methodName, inBuffer, outBuffer);
             } catch(exception& e) {
                 outBuffer.restore();