From f7b2cf35dad61149a95874a50d25cbb6580b75dc Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Wed, 11 Feb 2009 15:37:23 +0100 Subject: Fixed bug 3722 & 3723 - IceGrid leaks upon start failure --- cpp/src/IceGrid/Activator.cpp | 7 ++- cpp/src/IceGrid/IceGridNode.cpp | 96 +++++++++++++++++++++---------- cpp/src/IceGrid/NodeSessionManager.cpp | 14 ++++- cpp/src/IceGrid/RegistryI.cpp | 24 +++++++- cpp/src/IceGrid/RegistryI.h | 1 + cpp/src/IceGrid/ReplicaSessionManager.cpp | 7 ++- 6 files changed, 112 insertions(+), 37 deletions(-) (limited to 'cpp/src') 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&); @@ -183,6 +184,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; @@ -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 @@ -166,6 +166,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; -- cgit v1.2.3