Fixes for ICE-4961: improve IceGrid interop with old nodes and registries

10 files changed, 96 insertions, 76 deletions

diff --git a/cpp/src/IceGrid/Client.cpp b/cpp/src/IceGrid/Client.cpp
index a234e57ea83..b13eb8a2bd0 100644
--- a/cpp/src/IceGrid/Client.cpp
+++ b/cpp/src/IceGrid/Client.cpp
@@ -545,14 +545,13 @@ Client::run(StringSeq& originalArgs)
 
                 try
                 {
-                    const string strId = "\"" + communicator()->identityToString(registryId) + "\"";
-                    registry = RegistryPrx::checkedCast(communicator()->stringToProxy(strId));
+                    registry = RegistryPrx::checkedCast(locator->findObjectById(registryId));
                     if(!registry)
                     {
                         cerr << _appName << ": could not contact an IceGrid registry" << endl;
                     }
                 }
-                catch(const NotRegisteredException&)
+                catch(const ObjectNotFoundException&)
                 {
                     cerr << _appName << ": no active registry replica named `" << replica << "'" << endl;
                     return EXIT_FAILURE;            
diff --git a/cpp/src/IceGrid/Database.cpp b/cpp/src/IceGrid/Database.cpp
index 1f777cb33fe..a3e0536c002 100644
--- a/cpp/src/IceGrid/Database.cpp
+++ b/cpp/src/IceGrid/Database.cpp
@@ -826,7 +826,7 @@ Database::setAdapterDirectProxy(const string& adapterId, const string& replicaGr
 }
 
 Ice::ObjectPrx
-Database::getAdapterDirectProxy(const string& id)
+Database::getAdapterDirectProxy(const string& id, const Ice::EncodingVersion& encoding)
 {
     DatabaseConnectionPtr connection = _connectionPool->newConnection();
     AdaptersWrapperPtr adaptersWrapper = _connectionPool->getAdapters(connection);
@@ -842,8 +842,11 @@ Database::getAdapterDirectProxy(const string& id)
     vector<AdapterInfo> infos = adaptersWrapper->findByReplicaGroupId(id);
     for(unsigned int i = 0; i < infos.size(); ++i)
     {
-        Ice::EndpointSeq edpts = infos[i].proxy->ice_getEndpoints();
-        endpoints.insert(endpoints.end(), edpts.begin(), edpts.end());
+        if(infos[i].proxy->ice_getEncodingVersion() < encoding)
+        {
+            Ice::EndpointSeq edpts = infos[i].proxy->ice_getEndpoints();
+            endpoints.insert(endpoints.end(), edpts.begin(), edpts.end());
+        }
     }
     if(!endpoints.empty())
     {
diff --git a/cpp/src/IceGrid/Database.h b/cpp/src/IceGrid/Database.h
index c2eb5dc7c5f..bb35ca05652 100644
--- a/cpp/src/IceGrid/Database.h
+++ b/cpp/src/IceGrid/Database.h
@@ -95,7 +95,7 @@ public:
     AllocatableObjectEntryPtr getAllocatableObject(const Ice::Identity&) const;
 
     void setAdapterDirectProxy(const std::string&, const std::string&, const Ice::ObjectPrx&);
-    Ice::ObjectPrx getAdapterDirectProxy(const std::string&);
+    Ice::ObjectPrx getAdapterDirectProxy(const std::string&, const Ice::EncodingVersion&);
 
     void removeAdapter(const std::string&);
     AdapterPrx getAdapterProxy(const std::string&, const std::string&, bool);
diff --git a/cpp/src/IceGrid/IceGridNode.cpp b/cpp/src/IceGrid/IceGridNode.cpp
index 210c1563fed..51ed870f3de 100644
--- a/cpp/src/IceGrid/IceGridNode.cpp
+++ b/cpp/src/IceGrid/IceGridNode.cpp
@@ -325,9 +325,9 @@ NodeService::startImpl(int argc, char* argv[], int& status)
             locatorId.category = properties->getPropertyWithDefault("IceGrid.InstanceName", "IceGrid");
             locatorId.name = "Locator";
             string endpoints = properties->getProperty("IceGrid.Registry.Client.Endpoints");
-            string locatorPrx = "\"" + communicator()->identityToString(locatorId) + "\" :" + endpoints;
-            communicator()->setDefaultLocator(Ice::LocatorPrx::uncheckedCast(communicator()->stringToProxy(locatorPrx)));
-            properties->setProperty("Ice.Default.Locator", locatorPrx);
+            string locPrx = "\"" + communicator()->identityToString(locatorId) + "\" :" + endpoints;
+            communicator()->setDefaultLocator(Ice::LocatorPrx::uncheckedCast(communicator()->stringToProxy(locPrx)));
+            properties->setProperty("Ice.Default.Locator", locPrx);
         }
     }
     else if(properties->getProperty("Ice.Default.Locator").empty())
@@ -571,12 +571,11 @@ NodeService::startImpl(int argc, char* argv[], int& status)
     {
         try
         {
-            Ice::Identity registryId;
-            registryId.category = instanceName;
-            registryId.name = "Registry";
+            Ice::Identity regId;
+            regId.category = instanceName;
+            regId.name = "Registry";
             
-            RegistryPrx registry = RegistryPrx::checkedCast(
-                communicator()->stringToProxy("\"" + communicator()->identityToString(registryId) + "\""));
+            RegistryPrx registry = RegistryPrx::checkedCast(communicator()->getDefaultLocator()->findObjectById(regId));
             if(!registry)
             {
                 throw "invalid registry";
diff --git a/cpp/src/IceGrid/Internal.ice b/cpp/src/IceGrid/Internal.ice
index 2209a99dea0..fbf33ebf06c 100644
--- a/cpp/src/IceGrid/Internal.ice
+++ b/cpp/src/IceGrid/Internal.ice
@@ -575,7 +575,8 @@ interface ReplicaSession
      * will receive the database and database updates.
      *
      **/
-    idempotent void setDatabaseObserver(DatabaseObserver* dbObs);
+    idempotent void setDatabaseObserver(DatabaseObserver* dbObs)
+        throws ObserverAlreadyRegisteredException;
 
     /**
      *
diff --git a/cpp/src/IceGrid/LocatorI.cpp b/cpp/src/IceGrid/LocatorI.cpp
index 2310f3eb49a..22fd79040ed 100644
--- a/cpp/src/IceGrid/LocatorI.cpp
+++ b/cpp/src/IceGrid/LocatorI.cpp
@@ -912,7 +912,7 @@ LocatorI::findAdapterById_async(const Ice::AMD_Locator_findAdapterByIdPtr& cb,
 
     try
     {
-        cb->ice_response(_database->getAdapterDirectProxy(id));
+        cb->ice_response(_database->getAdapterDirectProxy(id, current.encoding));
     }
     catch(const AdapterNotExistException&)
     {
diff --git a/cpp/src/IceGrid/NodeSessionManager.cpp b/cpp/src/IceGrid/NodeSessionManager.cpp
index b3a498ae1fd..c696d4527df 100644
--- a/cpp/src/IceGrid/NodeSessionManager.cpp
+++ b/cpp/src/IceGrid/NodeSessionManager.cpp
@@ -154,14 +154,23 @@ NodeSessionKeepAliveThread::createSession(InternalRegistryPrx& registry, IceUtil
 NodeSessionPrx
 NodeSessionKeepAliveThread::createSessionImpl(const InternalRegistryPrx& registry, IceUtil::Time& timeout)
 {
-    NodeSessionPrx session = _node->registerWithRegistry(registry);
-    int t = session->getTimeout();
-    if(t > 0)
+    NodeSessionPrx session;
+    try
     {
-        timeout = IceUtil::Time::seconds(t / 2);
+        session = _node->registerWithRegistry(registry);
+        int t = session->getTimeout();
+        if(t > 0)
+        {
+            timeout = IceUtil::Time::seconds(t / 2);
+        }
+        _node->addObserver(session, session->getObserver());
+        return session;
+    }
+    catch(const Ice::LocalException&)
+    {
+        destroySession(session);
+        throw;
     }
-    _node->addObserver(session, session->getObserver());
-    return session;
 }
 
 void 
@@ -169,22 +178,25 @@ NodeSessionKeepAliveThread::destroySession(const NodeSessionPrx& session)
 {
     _node->removeObserver(session);
 
-    try
+    if(session)
     {
-        session->destroy();
-
-        if(_node->getTraceLevels() && _node->getTraceLevels()->replica > 0)
+        try
         {
-            Ice::Trace out(_node->getTraceLevels()->logger, _node->getTraceLevels()->replicaCat);
-            out << "destroyed replica `" << _name << "' session";
+            session->destroy();
+            
+            if(_node->getTraceLevels() && _node->getTraceLevels()->replica > 0)
+            {
+                Ice::Trace out(_node->getTraceLevels()->logger, _node->getTraceLevels()->replicaCat);
+                out << "destroyed replica `" << _name << "' session";
+            }
         }
-    }
-    catch(const Ice::LocalException& ex)
-    {
-        if(_node->getTraceLevels() && _node->getTraceLevels()->replica > 1)
+        catch(const Ice::LocalException& ex)
         {
-            Ice::Trace out(_node->getTraceLevels()->logger, _node->getTraceLevels()->replicaCat);
-            out << "couldn't destroy replica `" << _name << "' session:\n" << ex;
+            if(_node->getTraceLevels() && _node->getTraceLevels()->replica > 1)
+            {
+                Ice::Trace out(_node->getTraceLevels()->logger, _node->getTraceLevels()->replicaCat);
+                out << "couldn't destroy replica `" << _name << "' session:\n" << ex;
+            }
         }
     }
 }
@@ -233,7 +245,7 @@ NodeSessionManager::create(const NodeIPtr& node)
 
         Ice::CommunicatorPtr communicator = _node->getCommunicator();
         assert(communicator->getDefaultLocator());
-        Ice::Identity id = communicator->getDefaultLocator()->ice_getIdentity();
+        Ice::ObjectPrx prx = communicator->getDefaultLocator();
 
         //
         // Initialize the IceGrid::Query objects. The IceGrid::Query
@@ -242,9 +254,10 @@ NodeSessionManager::create(const NodeIPtr& node)
         // an up to date registry proxy, we need to query all the
         // replicas.
         //
-        Ice::EndpointSeq endpoints = communicator->getDefaultLocator()->ice_getEndpoints();
+        Ice::EndpointSeq endpoints = prx->ice_getEndpoints();
+        Ice::Identity id = prx->ice_getIdentity();
         id.name = "Query";
-        QueryPrx query = QueryPrx::uncheckedCast(communicator->stringToProxy(communicator->identityToString(id)));
+        QueryPrx query = QueryPrx::uncheckedCast(prx->ice_identity(id));
         for(Ice::EndpointSeq::const_iterator p = endpoints.begin(); p != endpoints.end(); ++p)
         {
             Ice::EndpointSeq singleEndpoint;
@@ -253,7 +266,7 @@ NodeSessionManager::create(const NodeIPtr& node)
         }
 
         id.name = "InternalRegistry-Master";
-        _master = InternalRegistryPrx::uncheckedCast(communicator->stringToProxy(communicator->identityToString(id)));
+        _master = InternalRegistryPrx::uncheckedCast(prx->ice_identity(id)->ice_endpoints(Ice::EndpointSeq()));
 
         _thread = new Thread(*this);
         _thread->start();
@@ -311,8 +324,10 @@ NodeSessionManager::activate()
             session->setReplicaObserver(_node->getProxy());
             syncServers(session);
         }
-        catch(const Ice::LocalException&)
+        catch(const Ice::LocalException& ex)
         {
+            Ice::Warning out(_node->getTraceLevels()->logger);
+            out << "failed to set replica observer:\n" << ex;
         }
     }
 }
@@ -513,8 +528,10 @@ NodeSessionManager::createdSession(const NodeSessionPrx& session)
             session->setReplicaObserver(_node->getProxy());
             syncServers(session);
         }
-        catch(const Ice::LocalException&)
+        catch(const Ice::LocalException& ex)
         {
+            Ice::Warning out(_node->getTraceLevels()->logger);
+            out << "failed to set replica observer:\n" << ex;
         }
         return;
     }
diff --git a/cpp/src/IceGrid/RegistryI.cpp b/cpp/src/IceGrid/RegistryI.cpp
index 2db5c694528..2a4ea190076 100644
--- a/cpp/src/IceGrid/RegistryI.cpp
+++ b/cpp/src/IceGrid/RegistryI.cpp
@@ -328,7 +328,7 @@ RegistryI::startImpl()
     catch(const Ice::LocalException&)
     {
     }
-    
+
     //
     // Create the reaper thread.
     //
diff --git a/cpp/src/IceGrid/ReplicaSessionManager.cpp b/cpp/src/IceGrid/ReplicaSessionManager.cpp
index 17b1a789df4..40c1b4c0058 100644
--- a/cpp/src/IceGrid/ReplicaSessionManager.cpp
+++ b/cpp/src/IceGrid/ReplicaSessionManager.cpp
@@ -271,11 +271,13 @@ ReplicaSessionManager::create(const string& name,
     {
         Lock sync(*this);
 
+        Ice::ObjectPrx prx = comm->getDefaultLocator();
+
         Ice::Identity id;
-        id.category = comm->getDefaultLocator()->ice_getIdentity().category;
+        id.category = prx->ice_getIdentity().category;
         id.name = "InternalRegistry-Master";
         
-        _master = InternalRegistryPrx::uncheckedCast(comm->stringToProxy(comm->identityToString(id)));
+        _master = InternalRegistryPrx::uncheckedCast(prx->ice_identity(id)->ice_endpoints(Ice::EndpointSeq()));
         _name = name;
         _info = info;
         _internalRegistry = internalRegistry;
@@ -290,9 +292,9 @@ ReplicaSessionManager::create(const string& name,
         // an up to date registry proxy, we need to query all the
         // replicas.
         //
-        Ice::EndpointSeq endpoints = comm->getDefaultLocator()->ice_getEndpoints();
+        Ice::EndpointSeq endpoints = prx->ice_getEndpoints();
         id.name = "Query";
-        QueryPrx query = QueryPrx::uncheckedCast(comm->stringToProxy(comm->identityToString(id)));
+        QueryPrx query = QueryPrx::uncheckedCast(prx->ice_identity(id));
         for(Ice::EndpointSeq::const_iterator p = endpoints.begin(); p != endpoints.end(); ++p)
         {
             Ice::EndpointSeq singleEndpoint;
@@ -445,7 +447,7 @@ ReplicaSessionManager::createSession(InternalRegistryPrx& registry, IceUtil::Tim
             {
                 session = createSessionImpl(registry, timeout);
             }
-            catch(const Ice::LocalException& ex)
+            catch(const Ice::Exception& ex)
             {
                 exception.reset(ex.ice_clone());
                 used.insert(registry);
@@ -481,7 +483,7 @@ ReplicaSessionManager::createSession(InternalRegistryPrx& registry, IceUtil::Tim
                         break;
                     }
                 }
-                catch(const Ice::LocalException& ex)
+                catch(const Ice::Exception& ex)
                 {
                     exception.reset(ex.ice_clone());
                     if(newRegistry)
@@ -554,9 +556,10 @@ ReplicaSessionManager::createSession(InternalRegistryPrx& registry, IceUtil::Tim
 ReplicaSessionPrx
 ReplicaSessionManager::createSessionImpl(const InternalRegistryPrx& registry, IceUtil::Time& timeout)
 {           
+    ReplicaSessionPrx session;
     try
     {
-        ReplicaSessionPrx session = registry->registerReplica(_info, _internalRegistry);
+        session = registry->registerReplica(_info, _internalRegistry);
         int t = session->getTimeout();
         if(t > 0)
         {
@@ -573,19 +576,9 @@ ReplicaSessionManager::createSessionImpl(const InternalRegistryPrx& registry, Ic
         session->setDatabaseObserver(_observer);
         return session;
     }
-    catch(const Ice::LocalException&)
+    catch(const Ice::Exception&)
     {
-        if(_observer)
-        {
-            try
-            {
-                _database->getInternalAdapter()->remove(_observer->ice_getIdentity());
-            }
-            catch(const Ice::LocalException&)
-            {
-            }
-            _observer = 0;
-        }
+        destroySession(session);
         throw;
     }
 }
@@ -593,22 +586,25 @@ ReplicaSessionManager::createSessionImpl(const InternalRegistryPrx& registry, Ic
 void
 ReplicaSessionManager::destroySession(const ReplicaSessionPrx& session)
 {
-    try
+    if(session)
     {
-        session->destroy();
-
-        if(_traceLevels && _traceLevels->replica > 0)
+        try
         {
-            Ice::Trace out(_traceLevels->logger, _traceLevels->replicaCat);
-            out << "destroyed master replica session";
+            session->destroy();
+            
+            if(_traceLevels && _traceLevels->replica > 0)
+            {
+                Ice::Trace out(_traceLevels->logger, _traceLevels->replicaCat);
+                out << "destroyed master replica session";
+            }
         }
-    }
-    catch(const Ice::LocalException& ex)
-    {
-        if(_traceLevels && _traceLevels->replica > 1)
+        catch(const Ice::LocalException& ex)
         {
-            Ice::Trace out(_traceLevels->logger, _traceLevels->replicaCat);
-            out << "couldn't destroy master replica session:\n" << ex;
+            if(_traceLevels && _traceLevels->replica > 1)
+            {
+                Ice::Trace out(_traceLevels->logger, _traceLevels->replicaCat);
+                out << "couldn't destroy master replica session:\n" << ex;
+            }
         }
     }
 
diff --git a/cpp/src/IceGrid/Topics.cpp b/cpp/src/IceGrid/Topics.cpp
index 2c641273ccf..389ea08d92c 100644
--- a/cpp/src/IceGrid/Topics.cpp
+++ b/cpp/src/IceGrid/Topics.cpp
@@ -601,21 +601,26 @@ ApplicationObserverTopic::applicationUpdated(int serial, const ApplicationUpdate
     }
     catch(const DeploymentException& ex)
     {
-        cerr << ex.reason << endl;
+        Ice::Error out(_logger);
+        out << "unexpected exception while instantiating application `" << info.descriptor.name << "':\n" << ex.reason;
         assert(false);
     }
     catch(const std::string& msg)
     {
-        cerr << msg << endl;
+        Ice::Error out(_logger);
+        out << "unexpected exception while instantiating application `" << info.descriptor.name << "':\n" << msg;
         assert(false);
     }
     catch(const char* msg)
     {
-        cerr << msg << endl;
+        Ice::Error out(_logger);
+        out << "unexpected exception while instantiating application `" << info.descriptor.name << "':\n" << msg;
         assert(false);
     }
     catch(...)
     {
+        Ice::Error out(_logger);
+        out << "unexpected exception while instantiating application `" << info.descriptor.name << "'";
         assert(false);
     }
     try