http://bugzilla.zeroc.com/bugzilla/show_bug.cgi?id=448

5 files changed, 102 insertions, 324 deletions

diff --git a/cppe/include/IceE/Communicator.h b/cppe/include/IceE/Communicator.h
index 850d5ed8a9b..5299b8d6399 100755
--- a/cppe/include/IceE/Communicator.h
+++ b/cppe/include/IceE/Communicator.h
@@ -21,7 +21,7 @@
 namespace Ice

 {

 

-class ICE_API Communicator : public ::IceUtil::RecMutex, public ::IceUtil::Shared

+class ICE_API Communicator : public ::IceUtil::Shared

 {

 public:

     

@@ -72,9 +72,8 @@ private:
     friend ICE_API CommunicatorPtr initializeWithProperties(int&, char*[], const PropertiesPtr&, Int);

     friend ICE_API ::IceInternal::InstancePtr IceInternal::getInstance(const ::Ice::CommunicatorPtr&);

 

-    bool _destroyed;

-    ::IceInternal::InstancePtr _instance;

-    ::Ice::Context _dfltContext;

+    const ::IceInternal::InstancePtr _instance;

+    const ::Ice::Context _dfltContext;

 };

 

 }

diff --git a/cppe/src/IceE/Communicator.cpp b/cppe/src/IceE/Communicator.cpp
index 9e57ecbdb45..bc60b3d6d7f 100755
--- a/cppe/src/IceE/Communicator.cpp
+++ b/cppe/src/IceE/Communicator.cpp
@@ -38,23 +38,7 @@ IceInternal::decRef(::Ice::Communicator* p)
 void
 Ice::Communicator::destroy()
 {
-    InstancePtr instance;
-
-    {
-	RecMutex::Lock sync(*this);
-	
-	if(_destroyed) // Don't destroy twice.
-	{
-	    return;
-	}
-	_destroyed = true;
-	instance = _instance;
-    }
-
-    if(instance)
-    {
-	instance->destroy();
-    }
+    _instance->destroy();
 }
 
 #ifndef ICEE_PURE_CLIENT
@@ -62,67 +46,25 @@ Ice::Communicator::destroy()
 void
 Ice::Communicator::shutdown()
 { 
-    ObjectAdapterFactoryPtr objectAdapterFactory;
-
-    {
-	RecMutex::Lock sync(*this);
-	if(_destroyed)
-	{
-	    throw CommunicatorDestroyedException(__FILE__, __LINE__);
-	}
-	objectAdapterFactory = _instance->objectAdapterFactory();
-    }
-
-    //
-    // We must call shutdown on the object adapter factory outside the
-    // synchronization, otherwise the communicator is blocked during
-    // shutdown.
-    //
-    objectAdapterFactory->shutdown();
+    _instance->objectAdapterFactory()->shutdown();
 }
 
 void
 Ice::Communicator::waitForShutdown()
 {
-    ObjectAdapterFactoryPtr objectAdapterFactory;
-
-    {
-	RecMutex::Lock sync(*this);
-	if(_destroyed)
-	{
-	    throw CommunicatorDestroyedException(__FILE__, __LINE__);
-	}
-	objectAdapterFactory = _instance->objectAdapterFactory();
-    }
-
-    //
-    // We must call waitForShutdown on the object adapter factory 
-    // outside the synchronization, otherwise the communicator is
-    // blocked during shutdown.
-    //
-    objectAdapterFactory->waitForShutdown();
+    _instance->objectAdapterFactory()->waitForShutdown();
 }
 #endif
 
 ObjectPrx
 Ice::Communicator::stringToProxy(const string& s) const
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     return _instance->proxyFactory()->stringToProxy(s);
 }
 
 string
 Ice::Communicator::proxyToString(const ObjectPrx& proxy) const
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     return _instance->proxyFactory()->proxyToString(proxy);
 }
 
@@ -131,12 +73,6 @@ Ice::Communicator::proxyToString(const ObjectPrx& proxy) const
 ObjectAdapterPtr
 Ice::Communicator::createObjectAdapter(const string& name)
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
-    
     return _instance->objectAdapterFactory()->createObjectAdapter(name);
 }
 
@@ -165,53 +101,30 @@ Ice::Communicator::createObjectAdapterWithEndpoints(const string& name, const st
 void
 Ice::Communicator::setDefaultContext(const Context& ctx)
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     _instance->setDefaultContext(ctx);
 }
 
 Ice::Context
 Ice::Communicator::getDefaultContext() const
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     return _instance->getDefaultContext();
 }
 
 PropertiesPtr
 Ice::Communicator::getProperties() const
 {
-    //
-    // No check for destruction. It must be possible to access the
-    // properties after destruction.
-    //
     return _instance->properties();
 }
 
 LoggerPtr
 Ice::Communicator::getLogger() const
 {
-    //
-    // No check for destruction. It must be possible to access the
-    // logger after destruction.
-    //
     return _instance->logger();
 }
 
 void
 Ice::Communicator::setLogger(const LoggerPtr& logger)
 {
-    //
-    // No check for destruction. It must be possible to set the logger
-    // after destruction (needed by logger plugins for example to
-    // unset the logger).
-    //
     _instance->logger(logger);
 }
 
@@ -220,22 +133,12 @@ Ice::Communicator::setLogger(const LoggerPtr& logger)
 RouterPrx
 Ice::Communicator::getDefaultRouter() const
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     return _instance->referenceFactory()->getDefaultRouter();
 }
 
 void
 Ice::Communicator::setDefaultRouter(const RouterPrx& router)
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     _instance->referenceFactory()->setDefaultRouter(router);
 }
 
@@ -246,22 +149,12 @@ Ice::Communicator::setDefaultRouter(const RouterPrx& router)
 LocatorPrx
 Ice::Communicator::getDefaultLocator() const
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     return _instance->referenceFactory()->getDefaultLocator();
 }
 
 void
 Ice::Communicator::setDefaultLocator(const LocatorPrx& locator)
 {
-    RecMutex::Lock sync(*this);
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
     _instance->referenceFactory()->setDefaultLocator(locator);
 }
 
@@ -277,13 +170,12 @@ Ice::Communicator::flushBatchRequests()
 
 #endif
 
-Ice::Communicator::Communicator(const PropertiesPtr& properties) :
-    _destroyed(false)
+Ice::Communicator::Communicator(const PropertiesPtr& properties) 
 {
     __setNoDelete(true);
     try
     {
-	_instance = new Instance(this, properties);
+	const_cast<InstancePtr&>(_instance) = new Instance(this, properties);
     }
     catch(...)
     {
@@ -295,8 +187,7 @@ Ice::Communicator::Communicator(const PropertiesPtr& properties) :
 
 Ice::Communicator::~Communicator()
 {
-    RecMutex::Lock sync(*this);
-    if(!_destroyed)
+    if(!_instance->destroyed())
     {
 	Warning out(_instance->logger());
 	out << "Ice::Communicator::destroy() has not been called";
@@ -325,7 +216,6 @@ Ice::Communicator::finishSetup(int& argc, char* argv[])
     }
     catch(...)
     {
-	_destroyed = true;
 	_instance->destroy();
 	throw;
     }
diff --git a/cppe/src/IceE/Instance.cpp b/cppe/src/IceE/Instance.cpp
index e9497bff3f7..5009ed783af 100644
--- a/cppe/src/IceE/Instance.cpp
+++ b/cppe/src/IceE/Instance.cpp
@@ -58,6 +58,13 @@ extern bool ICE_API nullHandleAbort;
 void IceInternal::incRef(Instance* p) { p->__incRef(); }
 void IceInternal::decRef(Instance* p) { p->__decRef(); }
 
+bool
+IceInternal::Instance::destroyed() const
+{
+    IceUtil::RecMutex::Lock sync(*this);
+    return _state == StateDestroyed;
+}
+
 CommunicatorPtr
 IceInternal::Instance::communicator() const
 {
@@ -67,32 +74,35 @@ IceInternal::Instance::communicator() const
 PropertiesPtr
 IceInternal::Instance::properties() const
 {
+    //
+    // No check for destruction. It must be possible to access the
+    // properties after destruction.
+    //
     // No mutex lock, immutable.
+    //
     return _properties;
 }
 
 LoggerPtr
 IceInternal::Instance::logger() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
-
     //
-    // Don't throw CommunicatorDestroyedException if destroyed. We
-    // need the logger also after destructions.
+    // No check for destruction. It must be possible to access the
+    // logger after destruction.
     //
+    IceUtil::RecMutex::Lock sync(*this);
     return _logger;
 }
 
 void
 IceInternal::Instance::logger(const LoggerPtr& logger)
 {
+    //
+    // No check for destruction. It must be possible to set the logger
+    // after destruction (needed by logger plugins for example to
+    // unset the logger).
+    //
     IceUtil::RecMutex::Lock sync(*this);
-
-    if(_destroyed)
-    {
-	throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
-
     _logger = logger;
 }
 
@@ -117,7 +127,7 @@ IceInternal::Instance::routerManager() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -134,7 +144,7 @@ IceInternal::Instance::locatorManager() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -149,7 +159,7 @@ IceInternal::Instance::referenceFactory() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -162,7 +172,7 @@ IceInternal::Instance::proxyFactory() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -175,7 +185,7 @@ IceInternal::Instance::outgoingConnectionFactory() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -189,7 +199,7 @@ IceInternal::Instance::objectAdapterFactory() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -210,7 +220,7 @@ IceInternal::Instance::endpointFactory() const
 {
     IceUtil::RecMutex::Lock sync(*this);
 
-    if(_destroyed)
+    if(_state == StateDestroyed)
     {
 	throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
@@ -238,7 +248,7 @@ IceInternal::Instance::flushBatchRequests()
     {
 	IceUtil::RecMutex::Lock sync(*this);
 
-	if(_destroyed)
+	if(_state == StateDestroyed)
 	{
 	    throw CommunicatorDestroyedException(__FILE__, __LINE__);
 	}
@@ -260,18 +270,32 @@ IceInternal::Instance::flushBatchRequests()
 void
 IceInternal::Instance::setDefaultContext(const Context& ctx)
 {
+    IceUtil::RecMutex::Lock sync(*this);
+    
+    if(_state == StateDestroyed)
+    {
+	throw CommunicatorDestroyedException(__FILE__, __LINE__);
+    }
+
     _defaultContext = ctx;
 }
 
 const Context&
 IceInternal::Instance::getDefaultContext() const
 {
+    IceUtil::RecMutex::Lock sync(*this);
+    
+    if(_state == StateDestroyed)
+    {
+	throw CommunicatorDestroyedException(__FILE__, __LINE__);
+    }
+
     return _defaultContext;
 }
 
 IceInternal::Instance::Instance(const CommunicatorPtr& communicator, const PropertiesPtr& properties) :
     _communicator(communicator.get()),
-    _destroyed(false),
+    _state(StateActive),
     _properties(properties),
     _messageSizeMax(0),
     _threadPerConnectionStackSize(0)
@@ -466,7 +490,7 @@ IceInternal::Instance::Instance(const CommunicatorPtr& communicator, const Prope
 
 IceInternal::Instance::~Instance()
 {
-    assert(_destroyed);
+    assert(_state == StateDestroyed);
     assert(!_referenceFactory);
     assert(!_proxyFactory);
     assert(!_outgoingConnectionFactory);
@@ -553,10 +577,29 @@ IceInternal::Instance::finishSetup(int& argc, char* argv[])
 #endif
 }
 
-void
+void

 IceInternal::Instance::destroy()
 {
-    assert(!_destroyed);
+    {
+	IceUtil::RecMutex::Lock sync(*this);
+	
+	//
+	// If the _state is not StateActive then the instance is
+	// either being destroyed, or has already been destroyed.
+	//
+	if(_state != StateActive)
+	{
+	    return;
+	}
+
+	//
+	// We cannot set state to StateDestroyed otherwise instance
+	// methods called during the destroy process (such as
+	// outgoingConnectionFactory() from
+	// ObjectAdapterI::deactivate() will cause an exception.
+	//
+	_state = StateDestroyInProgress;
+    }
 
 #ifndef ICEE_PURE_CLIENT
     if(_objectAdapterFactory)
@@ -626,6 +669,6 @@ IceInternal::Instance::destroy()
 	    _endpointFactory = 0;
 	}
 
-	_destroyed = true;
-    }
+	_state = StateDestroyed;
+    }

 }
diff --git a/cppe/src/IceE/Instance.h b/cppe/src/IceE/Instance.h
index 7826e8d6819..e7a68f230c4 100644
--- a/cppe/src/IceE/Instance.h
+++ b/cppe/src/IceE/Instance.h
@@ -35,6 +35,7 @@ class Instance : public IceUtil::Shared, public IceUtil::RecMutex
 {
 public:
 
+    bool destroyed() const;
     Ice::CommunicatorPtr communicator() const;
     Ice::PropertiesPtr properties() const;
     Ice::LoggerPtr logger() const;
@@ -74,7 +75,13 @@ private:
     friend class Ice::Communicator;
 
     Ice::Communicator* _communicator; // Not a Ptr, to avoid having Instance and Communicator point at each other.
-    bool _destroyed;
+    enum State
+    {
+	StateActive,
+	StateDestroyInProgress,
+	StateDestroyed
+    };
+    State _state;
     const Ice::PropertiesPtr _properties; // Immutable, not reset by destroy().
     Ice::LoggerPtr _logger; // Not reset by destroy().
     const TraceLevelsPtr _traceLevels; // Immutable, not reset by destroy().
diff --git a/cs/src/Ice/CommunicatorI.cs b/cs/src/Ice/CommunicatorI.cs
index cca4907aa44..69145b5c796 100755
--- a/cs/src/Ice/CommunicatorI.cs
+++ b/cs/src/Ice/CommunicatorI.cs
@@ -14,100 +14,32 @@ namespace Ice
     {
 	public void destroy()
 	{
-	    IceInternal.Instance instance = null;
-	    
-	    lock(this)
-	    {
-		if(!_destroyed) // Don't destroy twice.
-		{
-		    _destroyed = true;
-		    instance = _instance;
-		}
-	    }
-	    
-	    if(instance != null)
-	    {
-		instance.destroy();
-	    }
+	    _instance.destroy();
 	}
 
 	public void shutdown()
 	{
-	    IceInternal.ObjectAdapterFactory objectAdapterFactory;
-	    
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		objectAdapterFactory = _instance.objectAdapterFactory();
-	    }
-	    
-	    //
-	    // We must call shutdown on the object adapter factory
-	    // outside the synchronization, otherwise the communicator is
-	    // blocked while we wait for shutdown.
-	    //
-	    objectAdapterFactory.shutdown();
+	    _instance.objectAdapterFactory().shutdown();
 	}
 	
 	public void waitForShutdown()
 	{
-	    IceInternal.ObjectAdapterFactory objectAdapterFactory;
-	    
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		objectAdapterFactory = _instance.objectAdapterFactory();
-	    }
-	    
-	    //
-	    // We must call waitForShutdown on the object adapter factory
-	    // outside the synchronization, otherwise the communicator is
-	    // blocked while we wait for shutdown.
-	    //
-	    objectAdapterFactory.waitForShutdown();
+	    _instance.objectAdapterFactory().waitForShutdown();
 	}
 	
 	public Ice.ObjectPrx stringToProxy(string s)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		return _instance.proxyFactory().stringToProxy(s);
-	    }
+	    return _instance.proxyFactory().stringToProxy(s);
 	}
 	
 	public string proxyToString(Ice.ObjectPrx proxy)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		return _instance.proxyFactory().proxyToString(proxy);
-	    }
+	    return _instance.proxyFactory().proxyToString(proxy);
 	}
 
 	public ObjectAdapter createObjectAdapter(string name)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		
-		return _instance.objectAdapterFactory().createObjectAdapter(name);
-	    }
+	    return _instance.objectAdapterFactory().createObjectAdapter(name);
 	}
 	
 	public ObjectAdapter createObjectAdapterWithEndpoints(string name, string endpoints)
@@ -131,162 +63,72 @@ namespace Ice
 	
 	public void addObjectFactory(ObjectFactory factory, string id)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		_instance.servantFactoryManager().add(factory, id);
-	    }
+	    _instance.servantFactoryManager().add(factory, id);
 	}
 	
 	public void removeObjectFactory(string id)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		_instance.servantFactoryManager().remove(id);
-	    }
+	    _instance.servantFactoryManager().remove(id);
 	}
 	
 	public ObjectFactory findObjectFactory(string id)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		return _instance.servantFactoryManager().find(id);
-	    }
+	    return _instance.servantFactoryManager().find(id);
 	}
 	
 	public Properties getProperties()
 	{
-	    //
-	    // No check for destruction. It must be possible to access the
-	    // properties after destruction.
-	    //
 	    return _instance.properties();
 	}
 	
 	public Logger getLogger()
 	{
-	    //
-	    // No check for destruction. It must be possible to access the
-	    // logger after destruction.
-	    //
 	    return _instance.logger();
 	}
 
 	public void setLogger(Logger logger)
 	{
-	    //
-	    // No check for destruction. It must be possible to set the
-	    // logger after destruction (needed by logger plugins for
-	    // example to unset the logger).
-	    //
 	    _instance.logger(logger);
 	}
 	    
 	public Stats getStats()
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-			throw new CommunicatorDestroyedException();
-		}
-		return _instance.stats();
-	    }
+	    return _instance.stats();
 	}
 
 	public void setStats(Stats stats)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		_instance.stats(stats);
-	    }
+	    _instance.stats(stats);
 	}
 
 	public RouterPrx getDefaultRouter()
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		return _instance.referenceFactory().getDefaultRouter();
-	    }
+	    return _instance.referenceFactory().getDefaultRouter();
 	}
 
 	public void setDefaultRouter(RouterPrx router)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		_instance.referenceFactory().setDefaultRouter(router);
-	    }
+	    _instance.referenceFactory().setDefaultRouter(router);
 	}
 
 	public LocatorPrx getDefaultLocator()
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		return _instance.referenceFactory().getDefaultLocator();
-	    }
+	    return _instance.referenceFactory().getDefaultLocator();
 	}
 
 	public void setDefaultLocator(LocatorPrx locator)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-		_instance.referenceFactory().setDefaultLocator(locator);
-	    }
+	    _instance.referenceFactory().setDefaultLocator(locator);
 	}
 	
 	public Ice.Context getDefaultContext()
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-	        return _instance.getDefaultContext();
-	    }
+	    return _instance.getDefaultContext();
 	}
 	
 	public void setDefaultContext(Ice.Context ctx)
 	{
-	    lock(this)
-	    {
-		if(_destroyed)
-		{
-		    throw new CommunicatorDestroyedException();
-		}
-	        _instance.setDefaultContext(ctx);
-	    }
+	    _instance.setDefaultContext(ctx);
 	}
 
 	public PluginManager getPluginManager()
@@ -301,7 +143,6 @@ namespace Ice
 	
 	internal CommunicatorI(Properties properties)
 	{
-	    _destroyed = false;
 	    _instance = new IceInternal.Instance(this, properties);
 	}
 	
@@ -310,7 +151,7 @@ namespace Ice
 	{
 	    lock(this)
 	    {
-		if(!_destroyed)
+		if(!_instance.destroyed())
 		{
 		    if(!System.Environment.HasShutdownStarted)
 		    {
@@ -337,7 +178,6 @@ namespace Ice
 	    }
 	    catch(System.Exception)
 	    {
-		_destroyed = true;
 		_instance.destroy();
 		throw;
 	    }
@@ -352,7 +192,6 @@ namespace Ice
 	}
 	
 	
-	private bool _destroyed;
 	private IceInternal.Instance _instance;
     }