Fixed bug 3776, 3755, 3778 - getAdmin bugs and replicaGroup test warnings

10 files changed, 188 insertions, 89 deletions

diff --git a/cpp/src/Ice/Instance.cpp b/cpp/src/Ice/Instance.cpp
index 8c1f054805a..16cc8b849aa 100644
--- a/cpp/src/Ice/Instance.cpp
+++ b/cpp/src/Ice/Instance.cpp
@@ -620,18 +620,22 @@ IceInternal::Instance::getAdmin()
                 // (can't call again getAdmin() after fixing the problem)
                 // since all the facets (servants) in the adapter are lost
                 //
+                adapter->destroy();
                 sync.acquire();
                 _adminAdapter = 0;
-                adapter->destroy();
                 throw;
             }
             
+            Ice::ObjectPrx admin = adapter->createProxy(_adminIdentity);
             if(defaultLocator != 0 && serverId != "")
             {    
-                ProcessPrx process = ProcessPrx::uncheckedCast(
-                    adapter->createProxy(_adminIdentity)->ice_facet("Process"));
+                ProcessPrx process = ProcessPrx::uncheckedCast(admin->ice_facet("Process"));
                 try
                 {
+                    //
+                    // Note that as soon as the process proxy is registered, the communicator might be 
+                    // shutdown by a remote client and admin facets might start receiving calls.
+                    //
                     defaultLocator->getRegistry()->setServerProcessProxy(serverId, process);
                 }
                 catch(const ServerNotFoundException&)
@@ -663,7 +667,7 @@ IceInternal::Instance::getAdmin()
                 out << "registered server `" + serverId + "' with the locator registry";
             }
             
-            return adapter->createProxy(_adminIdentity);
+            return admin;
         }
         else
         {
@@ -1140,11 +1144,6 @@ IceInternal::Instance::finishSetup(int& argc, char* argv[])
 #endif
     }
     
-    if(_initData.properties->getPropertyAsIntWithDefault("Ice.Admin.DelayCreation", 0) <= 0)
-    {
-        getAdmin();
-    }
-
     //
     // Start connection monitor if necessary. Set the check interval to
     // 1/10 of the ACM timeout with a minmal value of 1 second and a 
@@ -1177,6 +1176,16 @@ IceInternal::Instance::finishSetup(int& argc, char* argv[])
     // Thread pool initialization is now lazy initialization in
     // clientThreadPool() and serverThreadPool().
     //
+
+    //
+    // This must be done last as this call creates the Ice.Admin object adapter
+    // and eventually register a process proxy with the Ice locator (allowing 
+    // remote clients to invoke on Ice.Admin facets as soon as it's registered).
+    //
+    if(_initData.properties->getPropertyAsIntWithDefault("Ice.Admin.DelayCreation", 0) <= 0)
+    {
+        getAdmin();
+    }
 }
 
 bool
diff --git a/cpp/src/IceGrid/AdapterCache.h b/cpp/src/IceGrid/AdapterCache.h
index 8ee42bc1531..cb3216405b7 100644
--- a/cpp/src/IceGrid/AdapterCache.h
+++ b/cpp/src/IceGrid/AdapterCache.h
@@ -44,12 +44,9 @@ public:
     AdapterEntry(AdapterCache&, const std::string&, const std::string&);
 
     virtual void getLocatorAdapterInfo(LocatorAdapterInfoSeq&, int&, bool&, bool&, const std::set<std::string>&) = 0;
-    void getLocatorAdapterInfo(LocatorAdapterInfoSeq& adapters, int& count, bool& replicaGroup, bool& roundRobin)
-    {
-        getLocatorAdapterInfo(adapters, count, replicaGroup, roundRobin, std::set<std::string>());
-    }
     virtual float getLeastLoadedNodeLoad(LoadSample) const = 0;
     virtual AdapterInfoSeq getAdapterInfo() const = 0;
+    virtual AdapterPrx getProxy(const std::string&, bool) const = 0;
 
     virtual bool canRemove();
 
@@ -74,9 +71,9 @@ public:
     virtual void getLocatorAdapterInfo(LocatorAdapterInfoSeq&, int&, bool&, bool&, const std::set<std::string>&);
     virtual float getLeastLoadedNodeLoad(LoadSample) const;
     virtual AdapterInfoSeq getAdapterInfo() const;
-    virtual const std::string& getReplicaGroupId() const { return _replicaGroupId; }
+    virtual AdapterPrx getProxy(const std::string&, bool) const;
 
-    AdapterPrx getProxy(const std::string&, bool) const;
+    const std::string& getReplicaGroupId() const { return _replicaGroupId; }
     int getPriority() const;
     
 private:
@@ -96,6 +93,7 @@ public:
     virtual void getLocatorAdapterInfo(LocatorAdapterInfoSeq&, int&, bool&, bool&, const std::set<std::string>&);
     virtual float getLeastLoadedNodeLoad(LoadSample) const;
     virtual AdapterInfoSeq getAdapterInfo() const;
+    virtual AdapterPrx getProxy(const std::string&, bool) const { return 0; }
 
     void addReplica(const std::string&, const ServerAdapterEntryPtr&);
     void removeReplica(const std::string&);
diff --git a/cpp/src/IceGrid/Database.cpp b/cpp/src/IceGrid/Database.cpp
index 52f79c8377b..bd3674ea4c8 100644
--- a/cpp/src/IceGrid/Database.cpp
+++ b/cpp/src/IceGrid/Database.cpp
@@ -810,10 +810,23 @@ Database::removeAdapter(const string& adapterId)
     _adapterObserverTopic->waitForSyncedSubscribers(serial);
 }
 
-AdapterEntryPtr
-Database::getAdapter(const string& id) const
+AdapterPrx
+Database::getAdapterProxy(const string& adapterId, const string& replicaGroupId, bool upToDate)
 {
-    return _adapterCache.get(id);
+    Lock sync(*this); // make sure this isn't call during an update.
+    return _adapterCache.get(adapterId)->getProxy(replicaGroupId, upToDate);
+}
+
+void 
+Database::getLocatorAdapterInfo(const string& id, 
+                                LocatorAdapterInfoSeq& adpts, 
+                                int& count, 
+                                bool& replicaGroup, 
+                                bool& roundRobin,
+                                const set<string>& excludes)
+{
+    Lock sync(*this); // Make sure this isn't call during an update.
+    _adapterCache.get(id)->getLocatorAdapterInfo(adpts, count, replicaGroup, roundRobin, excludes);
 }
 
 AdapterInfoSeq
@@ -826,9 +839,7 @@ Database::getAdapterInfo(const string& id)
     //
     try
     {
-#if defined(__BCPLUSPLUS__) && (__BCPLUSPLUS__ >= 0x0600)
-        IceUtil::DummyBCC dummy;
-#endif
+        Lock sync(*this); // Make sure this isn't call during an update.
         return _adapterCache.get(id)->getAdapterInfo();
     }
     catch(AdapterNotExistException&)
diff --git a/cpp/src/IceGrid/Database.h b/cpp/src/IceGrid/Database.h
index 16077223312..b09dd264972 100644
--- a/cpp/src/IceGrid/Database.h
+++ b/cpp/src/IceGrid/Database.h
@@ -101,7 +101,9 @@ public:
     Ice::ObjectPrx getAdapterDirectProxy(const std::string&);
 
     void removeAdapter(const std::string&);
-    AdapterEntryPtr getAdapter(const std::string&) const;
+    AdapterPrx getAdapterProxy(const std::string&, const std::string&, bool);
+    void getLocatorAdapterInfo(const std::string&, LocatorAdapterInfoSeq&, int&, bool&, bool&,
+                               const std::set<std::string>& = std::set<std::string>());
 
     std::vector<std::pair<std::string, AdapterPrx> > getAdapters(const std::string&, int&, bool&);
     AdapterInfoSeq getAdapterInfo(const std::string&);
diff --git a/cpp/src/IceGrid/LocatorI.cpp b/cpp/src/IceGrid/LocatorI.cpp
index 1f2291fe0e9..2db95739e9a 100644
--- a/cpp/src/IceGrid/LocatorI.cpp
+++ b/cpp/src/IceGrid/LocatorI.cpp
@@ -704,7 +704,7 @@ LocatorI::findAdapterById_async(const Ice::AMD_Locator_findAdapterByIdPtr& cb,
         int count;
         LocatorAdapterInfoSeq adapters;
         bool roundRobin;
-        _database->getAdapter(id)->getLocatorAdapterInfo(adapters, count, replicaGroup, roundRobin);
+        _database->getLocatorAdapterInfo(id, adapters, count, replicaGroup, roundRobin);
         RequestPtr request;
         if(roundRobin)
         {
@@ -899,5 +899,5 @@ LocatorI::getAdapterInfo(const string& id,
                          bool& roundRobin, 
                          const set<string>& excludes)
 {
-    _database->getAdapter(id)->getLocatorAdapterInfo(adapters, count, replicaGroup, roundRobin, excludes);
+    _database->getLocatorAdapterInfo(id, adapters, count, replicaGroup, roundRobin, excludes);
 }
diff --git a/cpp/src/IceGrid/LocatorRegistryI.cpp b/cpp/src/IceGrid/LocatorRegistryI.cpp
index 8d24ca1dce1..f84d40ec9b6 100644
--- a/cpp/src/IceGrid/LocatorRegistryI.cpp
+++ b/cpp/src/IceGrid/LocatorRegistryI.cpp
@@ -243,12 +243,12 @@ LocatorRegistryI::setAdapterDirectProxy(const AMI_Adapter_setDirectProxyPtr& ami
             //
             // Get the adapter from the registry and set its direct proxy.
             //
-            ServerAdapterEntryPtr adapter = ServerAdapterEntryPtr::dynamicCast(_database->getAdapter(adapterId));
+            AdapterPrx adapter = _database->getAdapterProxy(adapterId, replicaGroupId, false);
             if(!adapter)
             {
                 throw Ice::AdapterNotFoundException();
             }
-            adapter->getProxy(replicaGroupId, false)->setDirectProxy_async(amiCB, proxy);
+            adapter->setDirectProxy_async(amiCB, proxy);
             return;
         }
         catch(const AdapterNotExistException&)
diff --git a/cpp/src/IceGrid/ServerAdapterI.cpp b/cpp/src/IceGrid/ServerAdapterI.cpp
index 1f9e2fef819..c3114989e5c 100644
--- a/cpp/src/IceGrid/ServerAdapterI.cpp
+++ b/cpp/src/IceGrid/ServerAdapterI.cpp
@@ -31,6 +31,7 @@ ServerAdapterI::ServerAdapterI(const NodeIPtr& node,
 
 ServerAdapterI::~ServerAdapterI()
 {
+    assert(_activateCB.empty());
 }
 
 void
@@ -70,7 +71,7 @@ ServerAdapterI::activate_async(const AMD_Adapter_activatePtr& cb, const Ice::Cur
         {
             return;
         }
-        _activateAfterDeactivating = _server->getState() >= Deactivating;
+        _activateAfterDeactivating = _server->getState() >= Deactivating && _server->getState() < Destroying;
     }
 
     //
@@ -94,7 +95,6 @@ ServerAdapterI::activate_async(const AMD_Adapter_activatePtr& cb, const Ice::Cur
         // inconsistent if this happens. The best thing to do is to destroy the adapter.
         //
         destroy();
-        activationFailed("server destroyed");
     }
     catch(const Ice::Exception& ex)
     {
@@ -154,7 +154,7 @@ ServerAdapterI::setDirectProxy(const Ice::ObjectPrx& prx, const Ice::Current&)
     // now. The server is going to be activated again and the adapter
     // activated.
     //
-    if(_server->getState() < Deactivating || !_activateAfterDeactivating)
+    if(_server->getState() < Deactivating || _server->getState() >= Destroying || !_activateAfterDeactivating)
     {
         for(vector<AMD_Adapter_activatePtr>::const_iterator p = _activateCB.begin(); p != _activateCB.end(); ++p)
         {
@@ -194,6 +194,7 @@ ServerAdapterI::setDirectProxy(const Ice::ObjectPrx& prx, const Ice::Current&)
 void
 ServerAdapterI::destroy()
 {
+    activationFailed("adapter destroyed");
     try
     {
         _node->getAdapter()->remove(_this->ice_getIdentity());
@@ -233,6 +234,29 @@ ServerAdapterI::activationFailed(const std::string& reason)
     _activateCB.clear();
 }
 
+void 
+ServerAdapterI::activationCompleted()
+{
+    Lock sync(*this);
+    if(!_proxy)
+    {
+        //
+        // The server activation completed, but the adapter hasn't been activated.
+        //
+        if(_node->getTraceLevels()->adapter > 1)
+        {
+            Ice::Trace out(_node->getTraceLevels()->logger, _node->getTraceLevels()->adapterCat);
+            out << "server `" + _serverId + "' adapter `" << _id << "' activation failed: server activation completed";
+        }
+    }
+    
+    for(vector<AMD_Adapter_activatePtr>::const_iterator p = _activateCB.begin(); p != _activateCB.end(); ++p)
+    {
+        (*p)->ice_response(_proxy);
+    }
+    _activateCB.clear();
+}
+
 AdapterPrx
 ServerAdapterI::getProxy() const
 {
diff --git a/cpp/src/IceGrid/ServerAdapterI.h b/cpp/src/IceGrid/ServerAdapterI.h
index f2db60ae1fc..0239e1ec710 100644
--- a/cpp/src/IceGrid/ServerAdapterI.h
+++ b/cpp/src/IceGrid/ServerAdapterI.h
@@ -34,6 +34,7 @@ public:
     void destroy();
     void clear();
     void activationFailed(const std::string&);
+    void activationCompleted();
 
     AdapterPrx getProxy() const;
 
diff --git a/cpp/src/IceGrid/ServerI.cpp b/cpp/src/IceGrid/ServerI.cpp
index 72493162cb1..6c1941f3ad5 100644
--- a/cpp/src/IceGrid/ServerI.cpp
+++ b/cpp/src/IceGrid/ServerI.cpp
@@ -845,6 +845,7 @@ void
 ServerI::setProcess_async(const AMD_Server_setProcessPtr& amdCB, const Ice::ProcessPrx& process, const Ice::Current&)
 {
     bool deact = false;
+    ServerAdapterDict adpts;
     ServerCommandPtr command;
     {
         Lock sync(*this);
@@ -856,17 +857,32 @@ ServerI::setProcess_async(const AMD_Server_setProcessPtr& amdCB, const Ice::Proc
         }
         else
         {
-            checkActivation();
+            if(checkActivation())
+            {
+                adpts = _adapters;
+            }
             command = nextCommand();
         }
     }
     amdCB->ice_response();
 
+    for(ServerAdapterDict::iterator r = adpts.begin(); r != adpts.end(); ++r)
+    {
+        try
+        {
+            r->second->activationCompleted();
+        }
+        catch(const Ice::ObjectNotExistException&)
+        {
+        }
+    }
+
     if(deact)
     {
         deactivate();
     }
-    else if(command)
+
+    if(command)
     {
         command->execute();
     }
@@ -1185,13 +1201,14 @@ ServerI::finishPatch()
         }
     }
 #endif
-    setState(Inactive);
+    setState(ServerI::Inactive);
 }
 
 void
 ServerI::adapterActivated(const string& id)
 {
     ServerCommandPtr command;
+    ServerAdapterDict adpts;
     {
         Lock sync(*this);
         if(_state != ServerI::Activating && 
@@ -1201,9 +1218,25 @@ ServerI::adapterActivated(const string& id)
             return;
         }
         _activatedAdapters.insert(id);
-        checkActivation();
+        if(checkActivation())
+        {
+            adpts = _adapters;
+        }
         command = nextCommand();
     }
+    for(ServerAdapterDict::iterator r = adpts.begin(); r != adpts.end(); ++r)
+    {
+        if(r->first != id)
+        {
+            try
+            {
+                r->second->activationCompleted();
+            }
+            catch(const Ice::ObjectNotExistException&)
+            {
+            }
+        }
+    }
     if(command)
     {
         command->execute();
@@ -1343,88 +1376,107 @@ ServerI::activate()
     uid_t uid;
     gid_t gid;
 #endif
+    string failure;
+    try
     {
-        Lock sync(*this);
-        assert(_state == Activating && _desc);
-        desc = _desc;
-        adpts = _adapters;
+        {
+            Lock sync(*this);
+            assert(_state == Activating && _desc);
+            desc = _desc;
+            adpts = _adapters;
 
-        //
-        // The first time the server is started, we ensure that the
-        // replication of its descriptor is completed. This is to make
-        // sure all the replicas are up to date when the server
-        // starts for the first time with a given descriptor.
-        //
-        waitForReplication = _waitForReplication;
-        _waitForReplication = false;
+            if(_activation == Disabled)
+            {
+                throw string("The server is disabled.");
+            }
 
-        _process = 0;
+            //
+            // The first time the server is started, we ensure that the
+            // replication of its descriptor is completed. This is to make
+            // sure all the replicas are up to date when the server
+            // starts for the first time with a given descriptor.
+            //
+            waitForReplication = _waitForReplication;
+            _waitForReplication = false;
+
+            _process = 0;
         
 #ifndef _WIN32
-        uid = _uid;
-        gid = _gid;
+            uid = _uid;
+            gid = _gid;
 #endif
-    }
+        }
 
-    //
-    // We first ensure that the application is replicated on all the
-    // registries before to start the server. We only do this each
-    // time the server is updated or the initialy loaded on the node.
-    //
-    if(waitForReplication)
-    {
-        NodeSessionPrx session = _node->getMasterNodeSession();
-        if(session)
+        //
+        // We first ensure that the application is replicated on all the
+        // registries before to start the server. We only do this each
+        // time the server is updated or the initialy loaded on the node.
+        //
+        if(waitForReplication)
         {
-            AMI_NodeSession_waitForApplicationUpdatePtr cb = new WaitForApplicationUpdateCB(this);
-            _node->getMasterNodeSession()->waitForApplicationUpdate_async(cb, desc->uuid, desc->revision);
-            return;
+            NodeSessionPrx session = _node->getMasterNodeSession();
+            if(session)
+            {
+                AMI_NodeSession_waitForApplicationUpdatePtr cb = new WaitForApplicationUpdateCB(this);
+                _node->getMasterNodeSession()->waitForApplicationUpdate_async(cb, desc->uuid, desc->revision);
+                return;
+            }
         }
-    }
 
-    //
-    // Compute the server command line options.
-    //
-    Ice::StringSeq options;
-    copy(desc->options.begin(), desc->options.end(), back_inserter(options));
-    options.push_back("--Ice.Config=" + escapeProperty(_serverDir + "/config/config"));
+        //
+        // Compute the server command line options.
+        //
+        Ice::StringSeq options;
+        copy(desc->options.begin(), desc->options.end(), back_inserter(options));
+        options.push_back("--Ice.Config=" + escapeProperty(_serverDir + "/config/config"));
 
-    Ice::StringSeq envs;
-    transform(desc->envs.begin(), desc->envs.end(), back_inserter(envs), EnvironmentEval());
+        Ice::StringSeq envs;
+        transform(desc->envs.begin(), desc->envs.end(), back_inserter(envs), EnvironmentEval());
 
-    //
-    // Clear the adapters direct proxy (this is usefull if the server
-    // was manually activated).
-    //
-    for(ServerAdapterDict::iterator p = adpts.begin(); p != adpts.end(); ++p)
-    {
-        try
-        {
-            p->second->clear();
-        }
-        catch(const Ice::ObjectNotExistException&)
+        //
+        // Clear the adapters direct proxy (this is usefull if the server
+        // was manually activated).
+        //
+        for(ServerAdapterDict::iterator p = adpts.begin(); p != adpts.end(); ++p)
         {
+            try
+            {
+                p->second->clear();
+            }
+            catch(const Ice::ObjectNotExistException&)
+            {
+            }
         }
-    }
 
-    string failure;
-    try
-    {
 #ifndef _WIN32
         int pid = _node->getActivator()->activate(desc->id, desc->exe, desc->pwd, uid, gid, options, envs, this);
 #else
         int pid = _node->getActivator()->activate(desc->id, desc->exe, desc->pwd, options, envs, this);
 #endif
         ServerCommandPtr command;
+        bool active = false;
         {
             Lock sync(*this);
             assert(_state == Activating);
             _pid = pid;
             setStateNoSync(ServerI::WaitForActivation);
-            checkActivation();
+            active = checkActivation();
             command = nextCommand();
             notifyAll(); // Terminated might be waiting for the state change.
         }
+        if(active)
+        {
+            for(ServerAdapterDict::iterator r = adpts.begin(); r != adpts.end(); ++r)
+            {
+                try
+                {
+                    r->second->activationCompleted();
+                }
+                catch(const Ice::ObjectNotExistException&)
+                {
+                }
+            }
+        }
         if(command)
         {
             command->execute();
@@ -1435,7 +1487,7 @@ ServerI::activate()
     {
         failure = ex;
     }
-    catch(const Ice::SyscallException& ex)
+    catch(const Ice::Exception& ex)
     {
         Ice::Warning out(_node->getTraceLevels()->logger);
         out << "activation failed for server `" << _id << "':\n";
@@ -2321,7 +2373,7 @@ ServerI::updateRevision(const string& uuid, int revision)
     }
 }
 
-void
+bool
 ServerI::checkActivation()
 {
     //assert(locked());
@@ -2336,8 +2388,10 @@ ServerI::checkActivation()
                     _serverLifetimeAdapters.begin(), _serverLifetimeAdapters.end()))
         {
             setStateNoSync(ServerI::Active);
+            return true;
         }
     }
+    return false;
 }
 
 void
diff --git a/cpp/src/IceGrid/ServerI.h b/cpp/src/IceGrid/ServerI.h
index 9d04b017b7f..64aa208e7cb 100644
--- a/cpp/src/IceGrid/ServerI.h
+++ b/cpp/src/IceGrid/ServerI.h
@@ -121,7 +121,7 @@ private:
     void updateImpl(const InternalServerDescriptorPtr&);
     void checkRevision(const std::string&, const std::string&, int) const;
     void updateRevision(const std::string&, int);
-    void checkActivation();
+    bool checkActivation();
     void checkDestroyed() const;
     void disableOnFailure();
     void enableAfterFailure(bool);