Fixed bug 3722 & 3723 - IceGrid leaks upon start failure

6 files changed, 112 insertions, 37 deletions

diff --git a/cpp/src/IceGrid/Activator.cpp b/cpp/src/IceGrid/Activator.cpp
index 0b99b68a417..aad9b15b64f 100644
--- a/cpp/src/IceGrid/Activator.cpp
+++ b/cpp/src/IceGrid/Activator.cpp
@@ -1054,8 +1054,11 @@ Activator::destroy()
     // when there's no more processes and when _deactivating is set to
     // true.
     //
-    _thread->getThreadControl().join();
-    _thread = 0;
+    if(_thread)
+    {
+        _thread->getThreadControl().join();
+        _thread = 0;
+    }
     assert(_processes.empty());
 }
 
diff --git a/cpp/src/IceGrid/IceGridNode.cpp b/cpp/src/IceGrid/IceGridNode.cpp
index 4dc9a62e978..a78d45531a7 100644
--- a/cpp/src/IceGrid/IceGridNode.cpp
+++ b/cpp/src/IceGrid/IceGridNode.cpp
@@ -77,6 +77,7 @@ public:
 protected:
 
     virtual bool start(int, char*[]);
+    bool startImpl(int, char*[]);
     virtual void waitForShutdown();
     virtual bool stop();
     virtual CommunicatorPtr initializeCommunicator(int&, char*[], const InitializationData&);
@@ -184,6 +185,25 @@ NodeService::shutdown()
 bool
 NodeService::start(int argc, char* argv[])
 {
+    try
+    {
+        if(!startImpl(argc, argv))
+        {
+            stop();
+            return false;
+        }
+    }
+    catch(...)
+    {
+        stop();
+        throw;
+    }
+    return true;
+}
+
+bool
+NodeService::startImpl(int argc, char* argv[])
+{
     bool nowarn = false;
     bool readonly = false;
     string desc;
@@ -668,43 +688,53 @@ NodeService::waitForShutdown()
 bool
 NodeService::stop()
 {
-    try
-    {
-        _activator->destroy();
-    }
-    catch(...)
+    if(_activator)
     {
-        assert(false);
+        try
+        {
+            _activator->shutdown();
+            _activator->destroy();
+        }
+        catch(...)
+        {
+            assert(false);
+        }
+        _activator = 0;
     }
 
-    //
-    // The timer must be destroyed after the activator and before the
-    // communicator is shutdown.
-    //
-    try
-    {
-        _timer->destroy();
-    }
-    catch(...)
+    if(_timer)
     {
-        assert(false);
+        //
+        // The timer must be destroyed after the activator and before the
+        // communicator is shutdown.
+        //
+        try
+        {
+            _timer->destroy();
+        }
+        catch(...)
+        {
+            assert(false);
+        }
+        _timer = 0;
     }
 
-    _activator = 0;
-
     //
     // Deactivate the node object adapter.
     //
-    try
+    if(_adapter)
     {
-        _adapter->deactivate();
-        _adapter = 0;
-    }
-    catch(const Ice::LocalException& ex)
-    {
-        ostringstream ostr;
-        ostr << "unexpected exception while shutting down node:\n" << ex;
-        warning(ostr.str());
+        try
+        {
+            _adapter->deactivate();
+            _adapter = 0;
+        }
+        catch(const Ice::LocalException& ex)
+        {
+            ostringstream ostr;
+            ostr << "unexpected exception while shutting down node:\n" << ex;
+            warning(ostr.str());
+        }
     }
 
     //
@@ -715,7 +745,10 @@ NodeService::stop()
     //
     // Stop the platform info thread.
     //
-    _node->getPlatformInfo().stop();
+    if(_node)
+    {
+        _node->getPlatformInfo().stop();
+    }
 
     //
     // We can now safely shutdown the communicator.
@@ -735,8 +768,11 @@ NodeService::stop()
     //
     // Break cylic reference counts.
     //
-    _node->shutdown();
-    _node = 0;
+    if(_node)
+    {
+        _node->shutdown();
+        _node = 0;
+    }
 
     //
     // And shutdown the collocated registry.
diff --git a/cpp/src/IceGrid/NodeSessionManager.cpp b/cpp/src/IceGrid/NodeSessionManager.cpp
index 718a559f92f..878c0bb03d7 100644
--- a/cpp/src/IceGrid/NodeSessionManager.cpp
+++ b/cpp/src/IceGrid/NodeSessionManager.cpp
@@ -322,19 +322,29 @@ NodeSessionManager::destroy()
     NodeSessionMap sessions;
     {
         Lock sync(*this);
+        if(_destroyed)
+        {
+            return;
+        }
         _destroyed = true;
         _sessions.swap(sessions);
         notifyAll();
     }
 
-    _thread->terminate();
+    if(_thread)
+    {
+        _thread->terminate();
+    }
     NodeSessionMap::const_iterator p;
     for(p = sessions.begin(); p != sessions.end(); ++p)
     {
         p->second->terminate();
     }
 
-    _thread->getThreadControl().join();
+    if(_thread)
+    {
+        _thread->getThreadControl().join();
+    }
     for(p = sessions.begin(); p != sessions.end(); ++p)
     {
         p->second->getThreadControl().join();
diff --git a/cpp/src/IceGrid/RegistryI.cpp b/cpp/src/IceGrid/RegistryI.cpp
index b5c58b51583..088a1c6d8a3 100644
--- a/cpp/src/IceGrid/RegistryI.cpp
+++ b/cpp/src/IceGrid/RegistryI.cpp
@@ -167,6 +167,25 @@ RegistryI::~RegistryI()
 bool
 RegistryI::start()
 {
+    try
+    {
+        if(!startImpl())
+        {
+            stop();
+            return false;
+        }
+    }
+    catch(...)
+    {
+        stop();
+        throw;
+    }
+    return true;
+}
+
+bool
+RegistryI::startImpl()
+{
     assert(_communicator);
     PropertiesPtr properties = _communicator->getProperties();
 
@@ -687,7 +706,10 @@ RegistryI::stop()
     // ensure that there will be no more invocations on IceStorm once
     // it's shutdown.
     //
-    _database->destroyTopics();
+    if(_database)
+    {
+        _database->destroyTopics();
+    }
 
     try
     {
diff --git a/cpp/src/IceGrid/RegistryI.h b/cpp/src/IceGrid/RegistryI.h
index 59102782ee6..430920de7a8 100644
--- a/cpp/src/IceGrid/RegistryI.h
+++ b/cpp/src/IceGrid/RegistryI.h
@@ -52,6 +52,7 @@ public:
     ~RegistryI();
 
     bool start();
+    bool startImpl();
     void stop();
 
     virtual SessionPrx createSession(const std::string&, const std::string&, const Ice::Current&);
diff --git a/cpp/src/IceGrid/ReplicaSessionManager.cpp b/cpp/src/IceGrid/ReplicaSessionManager.cpp
index 05e57c908ef..74ff82acb46 100644
--- a/cpp/src/IceGrid/ReplicaSessionManager.cpp
+++ b/cpp/src/IceGrid/ReplicaSessionManager.cpp
@@ -346,16 +346,19 @@ ReplicaSessionManager::getNodes(const NodePrxSeq& nodes) const
 void
 ReplicaSessionManager::destroy()
 {
+    ThreadPtr thread;
     {
         Lock sync(*this);
         if(!_thread)
         {
             return;
         }
+        thread = _thread;
+        _thread = 0;
     }
 
-    _thread->terminate();
-    _thread->getThreadControl().join();
+    thread->terminate();
+    thread->getThreadControl().join();
 
     _database = 0;
     _wellKnownObjects = 0;