Bug 3966 - deadlock in LocatorInfo

4 files changed, 104 insertions, 26 deletions

diff --git a/cpp/src/Ice/LocatorInfo.cpp b/cpp/src/Ice/LocatorInfo.cpp
index 4271e3ceaab..4cd06442f3b 100644
--- a/cpp/src/Ice/LocatorInfo.cpp
+++ b/cpp/src/Ice/LocatorInfo.cpp
@@ -514,7 +514,8 @@ IceInternal::LocatorInfo::Request::exception(const Ice::Exception& ex)
 IceInternal::LocatorInfo::LocatorInfo(const LocatorPrx& locator, const LocatorTablePtr& table, bool background) :
     _locator(locator),
     _table(table),
-    _background(background)
+    _background(background),
+    _waitForRegistry(false)
 {
     assert(_locator);
     assert(_table);
@@ -523,7 +524,7 @@ IceInternal::LocatorInfo::LocatorInfo(const LocatorPrx& locator, const LocatorTa
 void
 IceInternal::LocatorInfo::destroy()
 {
-    IceUtil::Mutex::Lock sync(*this);
+    IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
 
     _locatorRegistry = 0;
     _table->clear();
@@ -559,19 +560,42 @@ IceInternal::LocatorInfo::getLocator() const
 LocatorRegistryPrx
 IceInternal::LocatorInfo::getLocatorRegistry()
 {
-    IceUtil::Mutex::Lock sync(*this);
+    LocatorPrx locator;
+    {
+        IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
+
+        while(_waitForRegistry)
+        {
+            wait();
+        }
+
+        if(_locatorRegistry)
+        {
+            return _locatorRegistry;
+        }
+
+        _waitForRegistry = true;
+        locator = _locator;
+    }
+
+    //
+    // Do not make locator calls from within sync.
+    //
+    LocatorRegistryPrx locatorRegistry = locator->getRegistry();
     
-    if(!_locatorRegistry) // Lazy initialization.
     {
-        _locatorRegistry = _locator->getRegistry();
+        IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
 
         //
         // The locator registry can't be located.
         //
-        _locatorRegistry = LocatorRegistryPrx::uncheckedCast(_locatorRegistry->ice_locator(0));
-    }
+        _locatorRegistry = LocatorRegistryPrx::uncheckedCast(locatorRegistry->ice_locator(0));
+
+        _waitForRegistry = false;
+        notifyAll();
     
-    return _locatorRegistry;
+        return _locatorRegistry;
+    }
 }
 
 vector<EndpointIPtr>
@@ -845,7 +869,7 @@ IceInternal::LocatorInfo::trace(const string& msg, const ReferencePtr& ref, cons
 IceInternal::LocatorInfo::RequestPtr
 IceInternal::LocatorInfo::getAdapterRequest(const ReferencePtr& ref)
 {
-    IceUtil::Mutex::Lock sync(*this);
+    IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
     if(ref->getInstance()->traceLevels()->location >= 1)
     {
         Trace out(ref->getInstance()->initializationData().logger, ref->getInstance()->traceLevels()->locationCat);
@@ -866,7 +890,7 @@ IceInternal::LocatorInfo::getAdapterRequest(const ReferencePtr& ref)
 IceInternal::LocatorInfo::RequestPtr
 IceInternal::LocatorInfo::getObjectRequest(const ReferencePtr& ref)
 {
-    IceUtil::Mutex::Lock sync(*this);
+    IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
     if(ref->getInstance()->traceLevels()->location >= 1)
     {
         Trace out(ref->getInstance()->initializationData().logger, ref->getInstance()->traceLevels()->locationCat);
@@ -912,7 +936,7 @@ IceInternal::LocatorInfo::finishRequest(const ReferencePtr& ref,
             _table->removeAdapterEndpoints(ref->getAdapterId());
         }
 
-        IceUtil::Mutex::Lock sync(*this);
+        IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
         assert(_adapterRequests.find(ref->getAdapterId()) != _adapterRequests.end());
         _adapterRequests.erase(ref->getAdapterId());
     }
@@ -927,7 +951,7 @@ IceInternal::LocatorInfo::finishRequest(const ReferencePtr& ref,
             _table->removeObjectReference(ref->getIdentity());
         }
 
-        IceUtil::Mutex::Lock sync(*this);
+        IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
         assert(_objectRequests.find(ref->getIdentity()) != _objectRequests.end());
         _objectRequests.erase(ref->getIdentity());
     }
diff --git a/cpp/src/Ice/LocatorInfo.h b/cpp/src/Ice/LocatorInfo.h
index 2d44d050a4a..cfc8e2c22ba 100644
--- a/cpp/src/Ice/LocatorInfo.h
+++ b/cpp/src/Ice/LocatorInfo.h
@@ -73,7 +73,7 @@ private:
     std::map<Ice::Identity, std::pair<IceUtil::Time, ReferencePtr> > _objectMap;
 };
 
-class LocatorInfo : public IceUtil::Shared, public IceUtil::Mutex
+class LocatorInfo : public IceUtil::Shared, public IceUtil::Monitor<IceUtil::Mutex>
 {
 public:
 
@@ -175,6 +175,7 @@ private:
     Ice::LocatorRegistryPrx _locatorRegistry;
     const LocatorTablePtr _table;
     const bool _background;
+    bool _waitForRegistry;
 
     std::map<std::string, RequestPtr> _adapterRequests;
     std::map<Ice::Identity, RequestPtr> _objectRequests;
diff --git a/cs/src/Ice/LocatorInfo.cs b/cs/src/Ice/LocatorInfo.cs
index ed89f032a76..033588c3ded 100644
--- a/cs/src/Ice/LocatorInfo.cs
+++ b/cs/src/Ice/LocatorInfo.cs
@@ -338,6 +338,7 @@ namespace IceInternal
             _locator = locator;
             _table = table;
             _background = background;
+            _waitForRegistry = false;
         }
         
         public void destroy()
@@ -375,17 +376,37 @@ namespace IceInternal
 
         public Ice.LocatorRegistryPrx getLocatorRegistry()
         {
+            Ice.LocatorPrx locator = null;
             lock(this)
             {
-                if(_locatorRegistry == null) // Lazy initialization
+                while(_waitForRegistry)
                 {
-                    _locatorRegistry = _locator.getRegistry();
-                    
-                    //
-                    // The locator registry can't be located.
-                    //
-                    _locatorRegistry = Ice.LocatorRegistryPrxHelper.uncheckedCast(_locatorRegistry.ice_locator(null));
+                    Monitor.Wait(this);
                 }
+
+                if(_locatorRegistry != null)
+                {
+                    return _locatorRegistry;
+                }
+
+                _waitForRegistry = true;
+                locator = _locator;
+            }
+
+            //
+            // Do not make locator calls from within sync.
+            //
+            Ice.LocatorRegistryPrx locatorRegistry = _locator.getRegistry();
+
+            lock(this)
+            {
+                //
+                // The locator registry can't be located.
+                //
+                _locatorRegistry = Ice.LocatorRegistryPrxHelper.uncheckedCast(locatorRegistry.ice_locator(null));
+
+                _waitForRegistry = false;
+                Monitor.PulseAll(this);
                 
                 return _locatorRegistry;
             }
@@ -787,6 +808,7 @@ namespace IceInternal
         private Ice.LocatorRegistryPrx _locatorRegistry;
         private readonly LocatorTable _table;
         private readonly bool _background;
+        private bool _waitForRegistry;
         
         private Dictionary<string, Request> _adapterRequests = new Dictionary<string, Request>();
         private Dictionary<Ice.Identity, Request> _objectRequests = new Dictionary<Ice.Identity, Request>();
diff --git a/java/src/IceInternal/LocatorInfo.java b/java/src/IceInternal/LocatorInfo.java
index cfb06e9d0e7..c4d977fb223 100644
--- a/java/src/IceInternal/LocatorInfo.java
+++ b/java/src/IceInternal/LocatorInfo.java
@@ -329,6 +329,7 @@ public final class LocatorInfo
         _locator = locator;
         _table = table;
         _background = background;
+        _waitForRegistry = false;
     }
 
     synchronized public void
@@ -363,20 +364,49 @@ public final class LocatorInfo
         return _locator;
     }
 
-    public synchronized Ice.LocatorRegistryPrx
+    public Ice.LocatorRegistryPrx
     getLocatorRegistry()
     {
-        if(_locatorRegistry == null) // Lazy initialization
+        Ice.LocatorPrx locator = null;
+        synchronized(this)
         {
-            _locatorRegistry = _locator.getRegistry();
+            while(_waitForRegistry)
+            {
+                try
+                {
+                    wait();
+                }
+                catch(InterruptedException ex)
+                {
+                }
+            }
 
+            if(_locatorRegistry != null)
+            {
+                return _locatorRegistry;
+            }
+
+            _waitForRegistry = true;
+            locator = _locator;
+        }
+
+        //
+        // Do not make locator calls from within sync.
+        //
+        Ice.LocatorRegistryPrx locatorRegistry = locator.getRegistry();
+
+        synchronized(this)
+        {
             //
             // The locator registry can't be located.
             //
-            _locatorRegistry = Ice.LocatorRegistryPrxHelper.uncheckedCast(_locatorRegistry.ice_locator(null));
+            _locatorRegistry = Ice.LocatorRegistryPrxHelper.uncheckedCast(locatorRegistry.ice_locator(null));
+
+            _waitForRegistry = false;
+            notifyAll();
+
+            return _locatorRegistry;
         }
-        
-        return _locatorRegistry;
     }
 
     public EndpointI[]
@@ -793,6 +823,7 @@ public final class LocatorInfo
     private Ice.LocatorRegistryPrx _locatorRegistry;
     private final LocatorTable _table;
     private final boolean _background;
+    private boolean _waitForRegistry;
 
     private java.util.Map<String, Request> _adapterRequests = new java.util.HashMap<String, Request>();
     private java.util.Map<Ice.Identity, Request> _objectRequests = new java.util.HashMap<Ice.Identity, Request>();