Fixed IceGrid locking issue, fixes #503

9 files changed, 209 insertions, 20 deletions

diff --git a/CHANGELOG-3.7.md b/CHANGELOG-3.7.md
index 227669a1e15..4014a153961 100644
--- a/CHANGELOG-3.7.md
+++ b/CHANGELOG-3.7.md
@@ -67,6 +67,10 @@ These are the changes since Ice 3.7.2.
 
 ## C++ Changes
 
+- Fixed IceGrid issue which could cause hangs if an IceGrid node became
+  unreachable and a client either tried to get adapter endpoints with
+  `IceGrid::Admin::getAdapterInfo` or called `IceGrid::Query::findAllReplicas`.
+
 - Fixed IceGrid issue where gracefully interrupted IceGrid node wouldn't notify
   observers of the deactivation of its servers.
 
diff --git a/cpp/src/IceGrid/Activator.cpp b/cpp/src/IceGrid/Activator.cpp
index 9a9fb38af2b..6f4ce63d1ea 100644
--- a/cpp/src/IceGrid/Activator.cpp
+++ b/cpp/src/IceGrid/Activator.cpp
@@ -156,6 +156,14 @@ signalToString(int signal)
         {
             return ICE_STRING(SIGALRM);
         }
+        case SIGCONT:
+        {
+            return ICE_STRING(SIGCONT);
+        }
+        case SIGSTOP:
+        {
+            return ICE_STRING(SIGSTOP);
+        }
 #endif
         case SIGKILL:
         {
@@ -230,6 +238,14 @@ stringToSignal(const string& str)
     {
         return SIGALRM;
     }
+    else if(str == ICE_STRING(SIGCONT))
+    {
+        return SIGCONT;
+    }
+    else if(str == ICE_STRING(SIGSTOP))
+    {
+        return SIGSTOP;
+    }
     else
 #endif
         if(str == ICE_STRING(SIGKILL))
diff --git a/cpp/src/IceGrid/AdapterCache.cpp b/cpp/src/IceGrid/AdapterCache.cpp
index de67ae58543..941a653b5f3 100644
--- a/cpp/src/IceGrid/AdapterCache.cpp
+++ b/cpp/src/IceGrid/AdapterCache.cpp
@@ -170,6 +170,47 @@ typedef IceUtil::Handle<ReplicaGroupSyncCallback> ReplicaGroupSyncCallbackPtr;
 
 }
 
+void
+GetAdapterInfoResult::add(const ServerAdapterEntryPtr& adapter)
+{
+    AdapterInfo info;
+    info.id = adapter->getId();
+    info.replicaGroupId = adapter->getReplicaGroupId();
+    _adapters.push_back(info);
+    try
+    {
+        _results.push_back(adapter->getProxy("", true)->begin_getDirectProxy());
+    }
+    catch(const SynchronizationException&)
+    {
+        _results.push_back(0);
+    }
+    catch(const Ice::Exception&)
+    {
+        _results.push_back(0);
+    }
+}
+
+AdapterInfoSeq
+GetAdapterInfoResult::get()
+{
+    vector<AdapterInfo>::iterator q = _adapters.begin();
+    for(vector<Ice::AsyncResultPtr>::const_iterator p = _results.begin(); p != _results.end(); ++p, ++q)
+    {
+        try
+        {
+            if(*p)
+            {
+                q->proxy = AdapterPrx::uncheckedCast((*p)->getProxy())->end_getDirectProxy(*p);
+            }
+        }
+        catch(const Ice::Exception&)
+        {
+        }
+    }
+    return _adapters;
+}
+
 AdapterCache::AdapterCache(const Ice::CommunicatorPtr& communicator) : _communicator(communicator)
 {
 }
@@ -405,26 +446,24 @@ ServerAdapterEntry::getLeastLoadedNodeLoad(LoadSample loadSample) const
 }
 
 AdapterInfoSeq
-ServerAdapterEntry::getAdapterInfo() const
+ServerAdapterEntry::getAdapterInfoNoEndpoints() const
 {
     AdapterInfo info;
     info.id = _id;
     info.replicaGroupId = _replicaGroupId;
-    try
-    {
-        info.proxy = _server->getAdapter(_id, true)->getDirectProxy();
-    }
-    catch(const SynchronizationException&)
-    {
-    }
-    catch(const Ice::Exception&)
-    {
-    }
     AdapterInfoSeq infos;
     infos.push_back(info);
     return infos;
 }
 
+GetAdapterInfoResultPtr
+ServerAdapterEntry::getAdapterInfoAsync() const
+{
+    GetAdapterInfoResultPtr result = new GetAdapterInfoResult();
+    result->add(const_cast<ServerAdapterEntry*>(this));
+    return result;
+}
+
 AdapterPrx
 ServerAdapterEntry::getProxy(const string& replicaGroupId, bool upToDate) const
 {
@@ -763,7 +802,7 @@ ReplicaGroupEntry::getLeastLoadedNodeLoad(LoadSample loadSample) const
 }
 
 AdapterInfoSeq
-ReplicaGroupEntry::getAdapterInfo() const
+ReplicaGroupEntry::getAdapterInfoNoEndpoints() const
 {
     //
     // This method is called with the database locked so we're sure
@@ -778,13 +817,29 @@ ReplicaGroupEntry::getAdapterInfo() const
     AdapterInfoSeq infos;
     for(vector<ServerAdapterEntryPtr>::const_iterator p = replicas.begin(); p != replicas.end(); ++p)
     {
-        AdapterInfoSeq infs = (*p)->getAdapterInfo();
+        AdapterInfoSeq infs = (*p)->getAdapterInfoNoEndpoints();
         assert(infs.size() == 1);
         infos.push_back(infs[0]);
     }
     return infos;
 }
 
+GetAdapterInfoResultPtr
+ReplicaGroupEntry::getAdapterInfoAsync() const
+{
+    GetAdapterInfoResultPtr result = new GetAdapterInfoResult();
+    vector<ServerAdapterEntryPtr> replicas;
+    {
+        Lock sync(*this);
+        replicas = _replicas;
+    }
+    for(vector<ServerAdapterEntryPtr>::const_iterator p = replicas.begin(); p != replicas.end(); ++p)
+    {
+        result->add(*p);
+    }
+    return result;
+}
+
 bool
 ReplicaGroupEntry::hasAdaptersFromOtherApplications() const
 {
diff --git a/cpp/src/IceGrid/AdapterCache.h b/cpp/src/IceGrid/AdapterCache.h
index f0f8dd4f771..bbe0bfeb794 100644
--- a/cpp/src/IceGrid/AdapterCache.h
+++ b/cpp/src/IceGrid/AdapterCache.h
@@ -28,6 +28,9 @@ typedef std::vector<ServerEntryPtr> ServerEntrySeq;
 class AdapterEntry;
 typedef IceUtil::Handle<AdapterEntry> AdapterEntryPtr;
 
+class ServerAdapterEntry;
+typedef IceUtil::Handle<ServerAdapterEntry> ServerAdapterEntryPtr;
+
 struct LocatorAdapterInfo
 {
     std::string id;
@@ -37,6 +40,20 @@ struct LocatorAdapterInfo
 };
 typedef std::vector<LocatorAdapterInfo> LocatorAdapterInfoSeq;
 
+class GetAdapterInfoResult : public IceUtil::Shared
+{
+public:
+
+    void add(const ServerAdapterEntryPtr&);
+    AdapterInfoSeq get();
+
+private:
+
+    AdapterInfoSeq _adapters;
+    std::vector<Ice::AsyncResultPtr> _results;
+};
+typedef IceUtil::Handle<GetAdapterInfoResult> GetAdapterInfoResultPtr;
+
 class AdapterEntry : public virtual IceUtil::Shared
 {
 public:
@@ -48,7 +65,8 @@ public:
     virtual void getLocatorAdapterInfo(LocatorAdapterInfoSeq&, int&, bool&, bool&, std::string&,
                                        const std::set<std::string>&) = 0;
     virtual float getLeastLoadedNodeLoad(LoadSample) const = 0;
-    virtual AdapterInfoSeq getAdapterInfo() const = 0;
+    virtual AdapterInfoSeq getAdapterInfoNoEndpoints() const = 0;
+    virtual GetAdapterInfoResultPtr getAdapterInfoAsync() const = 0;
     virtual AdapterPrx getProxy(const std::string&, bool) const = 0;
 
     virtual bool canRemove();
@@ -77,7 +95,8 @@ public:
                                        const std::set<std::string>&);
 
     virtual float getLeastLoadedNodeLoad(LoadSample) const;
-    virtual AdapterInfoSeq getAdapterInfo() const;
+    virtual AdapterInfoSeq getAdapterInfoNoEndpoints() const;
+    virtual GetAdapterInfoResultPtr getAdapterInfoAsync() const;
     virtual AdapterPrx getProxy(const std::string&, bool) const;
 
     void getLocatorAdapterInfo(LocatorAdapterInfoSeq&) const;
@@ -107,7 +126,8 @@ public:
     virtual void getLocatorAdapterInfo(LocatorAdapterInfoSeq&, int&, bool&, bool&, std::string&,
                                        const std::set<std::string>&);
     virtual float getLeastLoadedNodeLoad(LoadSample) const;
-    virtual AdapterInfoSeq getAdapterInfo() const;
+    virtual AdapterInfoSeq getAdapterInfoNoEndpoints() const;
+    virtual GetAdapterInfoResultPtr getAdapterInfoAsync() const;
     virtual AdapterPrx getProxy(const std::string&, bool) const { return 0; }
 
     void addReplica(const std::string&, const ServerAdapterEntryPtr&);
diff --git a/cpp/src/IceGrid/Database.cpp b/cpp/src/IceGrid/Database.cpp
index 300865cf469..4abfe9e436d 100644
--- a/cpp/src/IceGrid/Database.cpp
+++ b/cpp/src/IceGrid/Database.cpp
@@ -1262,14 +1262,19 @@ Database::getAdapterInfo(const string& id)
     // server, if that's the case we get the adapter proxy from the
     // server.
     //
+    GetAdapterInfoResultPtr result;
     try
     {
         Lock sync(*this); // Make sure this isn't call during an update.
-        return _adapterCache.get(id)->getAdapterInfo();
+        result = _adapterCache.get(id)->getAdapterInfoAsync();
     }
     catch(const AdapterNotExistException&)
     {
     }
+    if(result)
+    {
+        return result->get(); // Don't hold the database lock while waiting for the endpoints
+    }
 
     //
     // Otherwise, we check the adapter endpoint table -- if there's an
@@ -1314,7 +1319,7 @@ Database::getFilteredAdapterInfo(const string& id, const Ice::ConnectionPtr& con
             Lock sync(*this); // Make sure this isn't call during an update.
 
             AdapterEntryPtr entry = _adapterCache.get(id);
-            infos = entry->getAdapterInfo();
+            infos = entry->getAdapterInfoNoEndpoints();
             replicaGroup = ReplicaGroupEntryPtr::dynamicCast(entry);
         }
         if(replicaGroup)
diff --git a/cpp/test/IceGrid/replication/AllTests.cpp b/cpp/test/IceGrid/replication/AllTests.cpp
index a4374fb53fe..b477913949c 100644
--- a/cpp/test/IceGrid/replication/AllTests.cpp
+++ b/cpp/test/IceGrid/replication/AllTests.cpp
@@ -1365,8 +1365,94 @@ allTests(Test::TestHelper* helper)
         comm->stringToProxy("test -e 1.0")->ice_locator(slave3Locator)->ice_locatorCacheTimeout(0)->ice_ping();
         masterAdmin->stopServer("Server");
 
+        masterAdmin->removeApplication("TestApp");
+    }
+    cout << "ok" << endl;
+
+#if !defined(_WIN32) // This test relies on SIGSTOP/SITCONT
+    cout << "testing unreachable nodes... " << flush;
+    {
+        ApplicationDescriptor app;
+        app.name = "TestApp";
+        app.description = "added application";
+
+        app.replicaGroups.resize(1);
+        app.replicaGroups[0].id = "TestReplicaGroup";
+        app.replicaGroups[0].loadBalancing = new IceGrid::RandomLoadBalancingPolicy("2");
+
+        ObjectDescriptor object;
+        object.id = Ice::stringToIdentity("test");
+        object.type = "::Test::TestIntf";
+        app.replicaGroups[0].objects.push_back(object);
+
+        ServerDescriptorPtr server = new ServerDescriptor();
+        server->id = "Server1";
+        server->exe = comm->getProperties()->getProperty("ServerDir") + "/server";
+        server->pwd = ".";
+        server->applicationDistrib = false;
+        server->allocatable = false;
+        addProperty(server, "Ice.Admin.Endpoints", "tcp -h 127.0.0.1");
+        server->activation = "on-demand";
+        AdapterDescriptor adapter;
+        adapter.name = "TestAdapter";
+        adapter.id = "TestAdapter.${server}";
+        adapter.replicaGroupId = "TestReplicaGroup";
+        adapter.serverLifetime = true;
+        adapter.registerProcess = false;
+        PropertyDescriptor property;
+        property.name = "TestAdapter.Endpoints";
+        property.value = "default";
+        server->propertySet.properties.push_back(property);
+        property.name = "Identity";
+        property.value = "test";
+        server->propertySet.properties.push_back(property);
+
+        server->adapters.push_back(adapter);
+        app.nodes["Node1"].servers.push_back(server);
+
+        ServerDescriptorPtr server2 = ServerDescriptorPtr::dynamicCast(server->ice_clone());
+        server2->id = "Server2";
+        app.nodes["Node2"].servers.push_back(server2);
+
+        try
+        {
+            masterAdmin->addApplication(app);
+        }
+        catch(const DeploymentException& ex)
+        {
+            cerr << ex.reason << endl;
+        }
+
+        comm->stringToProxy("test -e 1.0@TestReplicaGroup")->ice_locator(
+             masterLocator->ice_encodingVersion(Ice::Encoding_1_0))->ice_locatorCacheTimeout(0)->ice_ping();
+
+        QueryPrx query = QueryPrx::uncheckedCast(
+             comm->stringToProxy("RepTestIceGrid/Query")->ice_locator(masterLocator));
+        test(query->findAllReplicas(comm->stringToProxy("test")).size() == 2);
+        masterAdmin->getAdapterInfo("TestReplicaGroup");
+
+        admin->sendSignal("Node2", "SIGSTOP");
+
+        test(query->findAllReplicas(comm->stringToProxy("test")).size() == 2);
+        try
+        {
+            masterAdmin->ice_invocationTimeout(1000)->getAdapterInfo("TestReplicaGroup");
+            test(false);
+        }
+        catch(const Ice::InvocationTimeoutException&)
+        {
+        }
+
+        admin->sendSignal("Node2", "SIGCONT");
+
+        test(query->findAllReplicas(comm->stringToProxy("test")).size() == 2);
+        masterAdmin->ice_invocationTimeout(1000)->getAdapterInfo("TestReplicaGroup");
+
+        masterAdmin->removeApplication("TestApp");
+
     }
     cout << "ok" << endl;
+#endif
 
     slave1Admin->shutdownNode("Node1");
     removeServer(admin, "Node1");
diff --git a/cpp/test/IceGrid/replication/Client.cpp b/cpp/test/IceGrid/replication/Client.cpp
index 0d88f5c27be..842f20329d4 100644
--- a/cpp/test/IceGrid/replication/Client.cpp
+++ b/cpp/test/IceGrid/replication/Client.cpp
@@ -18,7 +18,9 @@ public:
 void
 Client::run(int argc, char** argv)
 {
-    Ice::CommunicatorHolder communicator = initialize(argc, argv);
+    Ice::PropertiesPtr properties = createTestProperties(argc, argv);
+    properties->setProperty("Ice.Warn.Connections", "0");
+    Ice::CommunicatorHolder communicator = initialize(argc, argv, properties);
     communicator->getProperties()->parseCommandLineOptions("", Ice::argsToStringSeq(argc, argv));
     void allTests(Test::TestHelper*);
     allTests(this);
diff --git a/cpp/test/IceGrid/replication/application.xml b/cpp/test/IceGrid/replication/application.xml
index 6d96bdd9758..7e1c6a770fb 100644
--- a/cpp/test/IceGrid/replication/application.xml
+++ b/cpp/test/IceGrid/replication/application.xml
@@ -57,6 +57,7 @@
         <property name="Ice.Admin.Enabled" value="0"/>
 
         <property name="Ice.Default.EncodingVersion" value="${encoding}"/>
+        <property name="Ice.StdErr" value="${server.data}/stderr.txt"/>
       </server>
     </server-template>
 
diff --git a/cpp/test/IceGrid/session/Client.cpp b/cpp/test/IceGrid/session/Client.cpp
index efe250ee543..882ba448080 100644
--- a/cpp/test/IceGrid/session/Client.cpp
+++ b/cpp/test/IceGrid/session/Client.cpp
@@ -18,9 +18,9 @@ void
 Client::run(int argc, char** argv)
 {
     Ice::PropertiesPtr properties = createTestProperties(argc, argv);
+    properties->parseCommandLineOptions("", Ice::argsToStringSeq(argc, argv));
     properties->setProperty("Ice.Warn.Connections", "0");
     Ice::CommunicatorHolder communicator = initialize(argc, argv, properties);
-    communicator->getProperties()->parseCommandLineOptions("", Ice::argsToStringSeq(argc, argv));
     void allTests(Test::TestHelper*);
     allTests(this);
 }