Fixed ICE-5786: fixed Ice communicator destroy to be thread safe

12 files changed, 480 insertions, 432 deletions

diff --git a/cpp/src/Ice/Instance.cpp b/cpp/src/Ice/Instance.cpp
index 261cecf46d3..b4bd3d1ae14 100644
--- a/cpp/src/Ice/Instance.cpp
+++ b/cpp/src/Ice/Instance.cpp
@@ -288,7 +288,7 @@ IceInternal::ObserverUpdaterI::updateThreadObservers()
 bool
 IceInternal::Instance::destroyed() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
     return _state == StateDestroyed;
 }
 
@@ -311,7 +311,7 @@ IceInternal::Instance::defaultsAndOverrides() const
 RouterManagerPtr
 IceInternal::Instance::routerManager() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -325,7 +325,7 @@ IceInternal::Instance::routerManager() const
 LocatorManagerPtr
 IceInternal::Instance::locatorManager() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -339,7 +339,7 @@ IceInternal::Instance::locatorManager() const
 ReferenceFactoryPtr
 IceInternal::Instance::referenceFactory() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -353,7 +353,7 @@ IceInternal::Instance::referenceFactory() const
 RequestHandlerFactoryPtr
 IceInternal::Instance::requestHandlerFactory() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -367,7 +367,7 @@ IceInternal::Instance::requestHandlerFactory() const
 ProxyFactoryPtr
 IceInternal::Instance::proxyFactory() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -381,7 +381,7 @@ IceInternal::Instance::proxyFactory() const
 OutgoingConnectionFactoryPtr
 IceInternal::Instance::outgoingConnectionFactory() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -395,7 +395,7 @@ IceInternal::Instance::outgoingConnectionFactory() const
 ObjectFactoryManagerPtr
 IceInternal::Instance::servantFactoryManager() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -409,7 +409,7 @@ IceInternal::Instance::servantFactoryManager() const
 ObjectAdapterFactoryPtr
 IceInternal::Instance::objectAdapterFactory() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -441,7 +441,7 @@ IceInternal::Instance::networkProxy() const
 ThreadPoolPtr
 IceInternal::Instance::clientThreadPool()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -453,17 +453,21 @@ IceInternal::Instance::clientThreadPool()
 }
 
 ThreadPoolPtr
-IceInternal::Instance::serverThreadPool(bool create)
+IceInternal::Instance::serverThreadPool()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
         throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
 
-    if(!_serverThreadPool && create) // Lazy initialization.
+    if(!_serverThreadPool) // Lazy initialization.
     {
+        if(_state == StateDestroyInProgress)
+        {
+            throw CommunicatorDestroyedException(__FILE__, __LINE__);
+        }
         int timeout = _initData.properties->getPropertyAsInt("Ice.ServerIdleTime");
         _serverThreadPool = new ThreadPool(this, "Ice.ThreadPool.Server", timeout);
     }
@@ -474,7 +478,7 @@ IceInternal::Instance::serverThreadPool(bool create)
 EndpointHostResolverPtr
 IceInternal::Instance::endpointHostResolver()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -488,7 +492,7 @@ IceInternal::Instance::endpointHostResolver()
 RetryQueuePtr
 IceInternal::Instance::retryQueue()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -502,7 +506,7 @@ IceInternal::Instance::retryQueue()
 IceUtil::TimerPtr
 IceInternal::Instance::timer()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -515,7 +519,7 @@ IceInternal::Instance::timer()
 EndpointFactoryManagerPtr
 IceInternal::Instance::endpointFactoryManager() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -529,7 +533,7 @@ IceInternal::Instance::endpointFactoryManager() const
 DynamicLibraryListPtr
 IceInternal::Instance::dynamicLibraryList() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -543,7 +547,7 @@ IceInternal::Instance::dynamicLibraryList() const
 PluginManagerPtr
 IceInternal::Instance::pluginManager() const
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -689,7 +693,7 @@ IceInternal::Instance::createAdmin(const ObjectAdapterPtr& adminAdapter, const I
     ObjectAdapterPtr adapter = adminAdapter;
     bool createAdapter = !adminAdapter;
 
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -754,7 +758,7 @@ IceInternal::Instance::createAdmin(const ObjectAdapterPtr& adminAdapter, const I
 Ice::ObjectPrx
 IceInternal::Instance::getAdmin()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -889,7 +893,7 @@ IceInternal::Instance::setServerProcessProxy(const ObjectAdapterPtr& adminAdapte
 void
 IceInternal::Instance::addAdminFacet(const Ice::ObjectPtr& servant, const string& facet)
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -912,7 +916,7 @@ IceInternal::Instance::addAdminFacet(const Ice::ObjectPtr& servant, const string
 Ice::ObjectPtr
 IceInternal::Instance::removeAdminFacet(const string& facet)
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -945,7 +949,7 @@ IceInternal::Instance::removeAdminFacet(const string& facet)
 Ice::ObjectPtr
 IceInternal::Instance::findAdminFacet(const string& facet)
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -977,7 +981,7 @@ IceInternal::Instance::findAdminFacet(const string& facet)
 FacetMap
 IceInternal::Instance::findAllAdminFacets()
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -1003,7 +1007,7 @@ IceInternal::Instance::findAllAdminFacets()
 void
 IceInternal::Instance::setDefaultLocator(const Ice::LocatorPrx& defaultLocator)
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -1016,7 +1020,7 @@ IceInternal::Instance::setDefaultLocator(const Ice::LocatorPrx& defaultLocator)
 void
 IceInternal::Instance::setDefaultRouter(const Ice::RouterPrx& defaultRouter)
 {
-    IceUtil::RecMutex::Lock sync(*this);
+    Lock sync(*this);
 
     if(_state == StateDestroyed)
     {
@@ -1643,6 +1647,30 @@ IceInternal::Instance::finishSetup(int& argc, char* argv[], const Ice::Communica
 void
 IceInternal::Instance::destroy()
 {
+    {
+        Lock sync(*this);
+
+        //
+        // If destroy is in progress, wait for it to be done. This is
+        // necessary in case destroy() is called concurrently by
+        // multiple threads.
+        //
+        while(_state == StateDestroyInProgress)
+        {
+            wait();
+        }
+
+        if(_state == StateDestroyed)
+        {
+            return;
+        }
+        _state = StateDestroyInProgress;
+    }
+
+    //
+    // Shutdown and destroy all the incoming and outgoing Ice
+    // connections and wait for the connections to be finished.
+    //
     if(_objectAdapterFactory)
     {
         _objectAdapterFactory->shutdown();
@@ -1665,7 +1693,7 @@ IceInternal::Instance::destroy()
 
     if(_retryQueue)
     {
-        _retryQueue->destroy();
+        _retryQueue->destroy(); // Must be called before destroying thread pools.
     }
 
     if(_initData.observer)
@@ -1687,113 +1715,67 @@ IceInternal::Instance::destroy()
         logger->destroy();
     }
 
-    ThreadPoolPtr serverThreadPool;
-    ThreadPoolPtr clientThreadPool;
-    EndpointHostResolverPtr endpointHostResolver;
-    TimerPtr timer;
-    PluginManagerPtr pluginManager;
-    bool checkUnused = false;
+    //
+    // Now, destroy the thread pools. This must be done *only* after
+    // all the connections are finished (the connections destruction
+    // can require invoking callbacks with the thread pools).
+    // 
+    if(_serverThreadPool)
     {
-        IceUtil::RecMutex::Lock sync(*this);
-
-        _objectAdapterFactory = 0;
-        _outgoingConnectionFactory = 0;
-        _retryQueue = 0;
-
-        if(_serverThreadPool)
-        {
-            _serverThreadPool->destroy();
-            std::swap(_serverThreadPool, serverThreadPool);
-        }
-
-        if(_clientThreadPool)
-        {
-            _clientThreadPool->destroy();
-            std::swap(_clientThreadPool, clientThreadPool);
-        }
-
-        if(_endpointHostResolver)
-        {
-            _endpointHostResolver->destroy();
-            std::swap(endpointHostResolver, _endpointHostResolver);
-        }
-
-        if(_timer)
-        {
-            std::swap(_timer, timer);
-        }
-
-        if(_servantFactoryManager)
-        {
-            _servantFactoryManager->destroy();
-            _servantFactoryManager = 0;
-        }
-
-        //_referenceFactory->destroy(); // No destroy function defined.
-        _referenceFactory = 0;
-        
-        _requestHandlerFactory = 0;
-
-        // _proxyFactory->destroy(); // No destroy function defined.
-        _proxyFactory = 0;
-
-        if(_routerManager)
-        {
-            _routerManager->destroy();
-            _routerManager = 0;
-        }
-
-        if(_locatorManager)
-        {
-            _locatorManager->destroy();
-            _locatorManager = 0;
-        }
-
-        if(_endpointFactoryManager)
-        {
-            _endpointFactoryManager->destroy();
-            _endpointFactoryManager = 0;
-        }
-
-        std::swap(_pluginManager, pluginManager);
-
-        // No destroy function defined.
-        // _dynamicLibraryList->destroy();
-        _dynamicLibraryList = 0;
-
-        _adminAdapter = 0;
-        _adminFacets.clear();
-
-        if(_state != StateDestroyed)
-        {
-            checkUnused = true;
-        }
-        _state = StateDestroyed;
+        _serverThreadPool->destroy();
+    }
+    if(_clientThreadPool)
+    {
+        _clientThreadPool->destroy();
+    }
+    if(_endpointHostResolver)
+    {
+        _endpointHostResolver->destroy();
+    }
+    if(_timer)
+    {
+        _timer->destroy();
     }
 
     //
-    // Join with the thread pool threads outside the synchronization.
+    // Wait for all the threads to be finished.
     //
-    if(timer)
+    if(_clientThreadPool)
     {
-        timer->destroy();
+        _clientThreadPool->joinWithAllThreads();
     }
-    if(clientThreadPool)
+    if(_serverThreadPool)
     {
-        clientThreadPool->joinWithAllThreads();
-    }
-    if(serverThreadPool)
-    {
-        serverThreadPool->joinWithAllThreads();
+        _serverThreadPool->joinWithAllThreads();
     }
 #ifndef ICE_OS_WINRT
-    if(endpointHostResolver)
+    if(_endpointHostResolver)
     {
-        endpointHostResolver->getThreadControl().join();
+        _endpointHostResolver->getThreadControl().join();
     }
 #endif
+    
+    if(_servantFactoryManager)
+    {
+        _servantFactoryManager->destroy();
+    }
 
-    if(checkUnused && _initData.properties->getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
+    if(_routerManager)
+    {
+        _routerManager->destroy();
+    }
+
+    if(_locatorManager)
+    {
+        _locatorManager->destroy();
+    }
+
+    if(_endpointFactoryManager)
+    {
+        _endpointFactoryManager->destroy();
+    }
+
+    if(_initData.properties->getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
     {
         set<string> unusedProperties = static_cast<PropertiesI*>(_initData.properties.get())->getUnusedProperties();
         if(unusedProperties.size() != 0)
@@ -1810,9 +1792,38 @@ IceInternal::Instance::destroy()
     //
     // Destroy last so that a Logger plugin can receive all log/traces before its destruction.
     //
-    if(pluginManager)
+    if(_pluginManager)
     {
-        pluginManager->destroy();
+        _pluginManager->destroy();
+    }
+
+    {
+        Lock sync(*this);
+
+        _objectAdapterFactory = 0;
+        _outgoingConnectionFactory = 0;
+        _retryQueue = 0;
+
+        _serverThreadPool = 0;
+        _clientThreadPool = 0;
+        _endpointHostResolver = 0;
+        _timer = 0;
+
+        _servantFactoryManager = 0;
+        _referenceFactory = 0;
+        _requestHandlerFactory = 0;
+        _proxyFactory = 0;
+        _routerManager = 0;
+        _locatorManager = 0;
+        _endpointFactoryManager = 0;
+        _pluginManager = 0;
+        _dynamicLibraryList = 0;
+
+        _adminAdapter = 0;
+        _adminFacets.clear();
+
+        _state = StateDestroyed;
+        notifyAll();
     }
 }
 
diff --git a/cpp/src/Ice/Instance.h b/cpp/src/Ice/Instance.h
index 4a41a6b8773..c008d278f2e 100644
--- a/cpp/src/Ice/Instance.h
+++ b/cpp/src/Ice/Instance.h
@@ -63,7 +63,7 @@ typedef IceUtil::Handle<MetricsAdminI> MetricsAdminIPtr;
 class RequestHandlerFactory;
 typedef IceUtil::Handle<RequestHandlerFactory> RequestHandlerFactoryPtr;
 
-class Instance : public IceUtil::Shared, public IceUtil::RecMutex
+class Instance : public IceUtil::Shared, public IceUtil::Monitor<IceUtil::RecMutex>
 {
 public:
 
@@ -83,7 +83,7 @@ public:
     bool preferIPv6() const;
     NetworkProxyPtr networkProxy() const;
     ThreadPoolPtr clientThreadPool();
-    ThreadPoolPtr serverThreadPool(bool create = true);
+    ThreadPoolPtr serverThreadPool();
     EndpointHostResolverPtr endpointHostResolver();
     RetryQueuePtr retryQueue();
     IceUtil::TimerPtr timer();
@@ -136,6 +136,7 @@ private:
     enum State
     {
         StateActive,
+        StateDestroyInProgress,
         StateDestroyed
     };
     State _state;
diff --git a/cs/src/Ice/Instance.cs b/cs/src/Ice/Instance.cs
index 3ec55288b22..495ca77f09d 100644
--- a/cs/src/Ice/Instance.cs
+++ b/cs/src/Ice/Instance.cs
@@ -216,7 +216,7 @@ namespace IceInternal
             }
         }
 
-        public ThreadPool serverThreadPool(bool create)
+        public ThreadPool serverThreadPool()
         {
             lock(this)
             {
@@ -225,8 +225,12 @@ namespace IceInternal
                     throw new Ice.CommunicatorDestroyedException();
                 }
 
-                if(_serverThreadPool == null && create) // Lazy initialization.
+                if(_serverThreadPool == null) // Lazy initialization.
                 {
+                    if(_state == StateDestroyInProgress)
+                    {
+                        throw new Ice.CommunicatorDestroyedException();
+                    }
                     int timeout = _initData.properties.getPropertyAsInt("Ice.ServerIdleTime");
                     _serverThreadPool = new ThreadPool(this, "Ice.ThreadPool.Server", timeout);
                 }
@@ -1095,6 +1099,29 @@ namespace IceInternal
         //
         public void destroy()
         {
+            lock(this)
+            {
+                //
+                // If destroy is in progress, wait for it to be done. This
+                // is necessary in case destroy() is called concurrently
+                // by multiple threads.
+                //
+                while(_state == StateDestroyInProgress)
+                {
+                    Monitor.Wait(this);
+                }
+                
+                if(_state == StateDestroyed)
+                {
+                    return;
+                }
+                _state = StateDestroyInProgress;
+            }
+
+            //
+            // Shutdown and destroy all the incoming and outgoing Ice
+            // connections and wait for the connections to be finished.
+            //
             if(_objectAdapterFactory != null)
             {
                 _objectAdapterFactory.shutdown();
@@ -1117,7 +1144,7 @@ namespace IceInternal
 
             if(_retryQueue != null)
             {
-                _retryQueue.destroy();
+                _retryQueue.destroy(); // Must be called before destroying thread pools.
             }
 
             if(_initData.observer != null)
@@ -1131,133 +1158,77 @@ namespace IceInternal
                 logger.destroy();
             }
 
-            ThreadPool serverThreadPool = null;
-            ThreadPool clientThreadPool = null;
-            AsyncIOThread asyncIOThread = null;
-            IceInternal.Timer timer = null;
-            Ice.PluginManager pluginManager = null;
-            bool checkUnused = false;
-
-#if !SILVERLIGHT
-            EndpointHostResolver endpointHostResolver = null;
-#endif
-            lock(this)
+            //
+            // Now, destroy the thread pools. This must be done *only* after
+            // all the connections are finished (the connections destruction
+            // can require invoking callbacks with the thread pools).
+            // 
+            if(_serverThreadPool != null)
             {
-                _objectAdapterFactory = null;
-                _outgoingConnectionFactory = null;
-                _retryQueue = null;
-
-                if(_serverThreadPool != null)
-                {
-                    _serverThreadPool.destroy();
-                    serverThreadPool = _serverThreadPool;
-                    _serverThreadPool = null;
-                }
-
-                if(_clientThreadPool != null)
-                {
-                    _clientThreadPool.destroy();
-                    clientThreadPool = _clientThreadPool;
-                    _clientThreadPool = null;
-                }
-
-                if(_asyncIOThread != null)
-                {
-                    _asyncIOThread.destroy();
-                    asyncIOThread = _asyncIOThread;
-                    _asyncIOThread = null;
-                }
-
+                _serverThreadPool.destroy();
+            }
+            if(_clientThreadPool != null)
+            {
+                _clientThreadPool.destroy();
+            }
+            if(_asyncIOThread != null)
+            {
+                _asyncIOThread.destroy();
+            }
 #if !SILVERLIGHT
-                if(_endpointHostResolver != null)
-                {
-                    _endpointHostResolver.destroy();
-                    endpointHostResolver = _endpointHostResolver;
-                    _endpointHostResolver = null;
-                }
-#endif
-
-                if(_timer != null)
-                {
-                    timer = _timer;
-                    _timer = null;
-                }
-
-                if(_servantFactoryManager != null)
-                {
-                    _servantFactoryManager.destroy();
-                    _servantFactoryManager = null;
-                }
-
-                // No destroy function defined.
-                //_referenceFactory.destroy();
-                _referenceFactory = null;
-
-                // No destroy function defined.
-                _requestHandlerFactory = null;
-
-                // No destroy function defined.
-                // _proxyFactory.destroy();
-                _proxyFactory = null;
-
-                if(_routerManager != null)
-                {
-                    _routerManager.destroy();
-                    _routerManager = null;
-                }
-
-                if(_locatorManager != null)
-                {
-                    _locatorManager.destroy();
-                    _locatorManager = null;
-                }
-
-                if(_endpointFactoryManager != null)
-                {
-                    _endpointFactoryManager.destroy();
-                    _endpointFactoryManager = null;
-                }
-
-                pluginManager = _pluginManager;
-                _pluginManager = null;
-
-                _adminAdapter = null;
-                _adminFacets.Clear();
-
-                if(_state != StateDestroyed)
-                {
-                    checkUnused = true;
-                }
-
-                _state = StateDestroyed;
+            if(_endpointHostResolver != null)
+            {
+                _endpointHostResolver.destroy();
             }
+#endif
 
             //
-            // Join with threads outside the synchronization.
+            // Wait for all the threads to be finished.
             //
-            if(timer != null)
+            if(_timer != null)
             {
-                timer.destroy();
+                _timer.destroy();
             }
-            if(clientThreadPool != null)
+            if(_clientThreadPool != null)
             {
-                clientThreadPool.joinWithAllThreads();
+                _clientThreadPool.joinWithAllThreads();
             }
-            if(serverThreadPool != null)
+            if(_serverThreadPool != null)
             {
-                serverThreadPool.joinWithAllThreads();
+                _serverThreadPool.joinWithAllThreads();
             }
-            if(asyncIOThread != null)
+            if(_asyncIOThread != null)
             {
-                asyncIOThread.joinWithThread();
+                _asyncIOThread.joinWithThread();
             }
 #if !SILVERLIGHT
-            if(endpointHostResolver != null)
+            if(_endpointHostResolver != null)
             {
-                endpointHostResolver.joinWithThread();
+                _endpointHostResolver.joinWithThread();
             }
 #endif
-            if(checkUnused && _initData.properties.getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
+
+            if(_servantFactoryManager != null)
+            {
+                _servantFactoryManager.destroy();
+            }
+
+            if(_routerManager != null)
+            {
+                _routerManager.destroy();
+            }
+
+            if(_locatorManager != null)
+            {
+                _locatorManager.destroy();
+            }
+
+            if(_endpointFactoryManager != null)
+            {
+                _endpointFactoryManager.destroy();
+            }
+
+            if(_initData.properties.getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
             {
                 List<string> unusedProperties = ((Ice.PropertiesI)_initData.properties).getUnusedProperties();
                 if (unusedProperties.Count != 0)
@@ -1275,9 +1246,37 @@ namespace IceInternal
             //
             // Destroy last so that a Logger plugin can receive all log/traces before its destruction.
             //
-            if(pluginManager != null)
+            if(_pluginManager != null)
             {
-                pluginManager.destroy();
+                _pluginManager.destroy();
+            }
+
+            lock(this)
+            {
+                _objectAdapterFactory = null;
+                _outgoingConnectionFactory = null;
+                _retryQueue = null;
+
+                _serverThreadPool = null;
+                _clientThreadPool = null;
+                _asyncIOThread = null;
+                _endpointHostResolver = null;
+                _timer = null;
+
+                _servantFactoryManager = null;
+                _referenceFactory = null;
+                _requestHandlerFactory = null;
+                _proxyFactory = null;
+                _routerManager = null;
+                _locatorManager = null;
+                _endpointFactoryManager = null;
+                _pluginManager = null;
+
+                _adminAdapter = null;
+                _adminFacets.Clear();
+
+                _state = StateDestroyed;
+                Monitor.PulseAll(this);
             }
         }
 
@@ -1424,7 +1423,8 @@ namespace IceInternal
         }
 
         private const int StateActive = 0;
-        private const int StateDestroyed = 1;
+        private const int StateDestroyInProgress = 1;
+        private const int StateDestroyed = 2;
         private int _state;
         private Ice.InitializationData _initData; // Immutable, not reset by destroy().
         private TraceLevels _traceLevels; // Immutable, not reset by destroy().
diff --git a/cs/src/Ice/ObjectAdapterI.cs b/cs/src/Ice/ObjectAdapterI.cs
index 7756016527d..12c393f3a50 100644
--- a/cs/src/Ice/ObjectAdapterI.cs
+++ b/cs/src/Ice/ObjectAdapterI.cs
@@ -790,7 +790,7 @@ namespace Ice
             }
             else
             {
-                return instance_.serverThreadPool(true);
+                return instance_.serverThreadPool();
             }
             
         }
diff --git a/java/src/Ice/src/main/java/Ice/ObjectAdapterI.java b/java/src/Ice/src/main/java/Ice/ObjectAdapterI.java
index 48ad2739082..8a30f970865 100644
--- a/java/src/Ice/src/main/java/Ice/ObjectAdapterI.java
+++ b/java/src/Ice/src/main/java/Ice/ObjectAdapterI.java
@@ -830,7 +830,7 @@ public final class ObjectAdapterI implements ObjectAdapter
         }
         else
         {
-            return _instance.serverThreadPool(true);
+            return _instance.serverThreadPool();
         }
     }
 
diff --git a/java/src/Ice/src/main/java/IceInternal/Instance.java b/java/src/Ice/src/main/java/IceInternal/Instance.java
index 11057dcb5ef..0160d6564d4 100644
--- a/java/src/Ice/src/main/java/IceInternal/Instance.java
+++ b/java/src/Ice/src/main/java/IceInternal/Instance.java
@@ -125,7 +125,6 @@ public final class Instance
             _observerHelper.afterExecute();
         }
 
-
         public void destroy()
             throws InterruptedException
         {
@@ -308,15 +307,20 @@ public final class Instance
     }
 
     public synchronized ThreadPool
-    serverThreadPool(boolean create)
+    serverThreadPool()
     {
         if(_state == StateDestroyed)
         {
             throw new Ice.CommunicatorDestroyedException();
         }
 
-        if(_serverThreadPool == null && create) // Lazy initialization.
+        if(_serverThreadPool == null) // Lazy initialization.
         {
+            if(_state == StateDestroyInProgress)
+            {
+                throw new Ice.CommunicatorDestroyedException();
+            }
+
             int timeout = _initData.properties.getPropertyAsInt("Ice.ServerIdleTime");
             _serverThreadPool = new ThreadPool(this, "Ice.ThreadPool.Server", timeout);
         }
@@ -1192,198 +1196,222 @@ public final class Instance
             throw new Ice.OperationInterruptedException();
         }
 
-        if(_objectAdapterFactory != null)
+        synchronized(this)
         {
-            _objectAdapterFactory.shutdown();
-        }
+            //
+            // If destroy is in progress, wait for it to be done. This
+            // is necessary in case destroy() is called concurrently
+            // by multiple threads.
+            //
+            while(_state == StateDestroyInProgress)
+            {
+                try
+                {
+                    wait();
+                }
+                catch(InterruptedException ex)
+                {
+                    throw new Ice.OperationInterruptedException();
+                }
+            }
 
-        if(_outgoingConnectionFactory != null)
-        {
-            _outgoingConnectionFactory.destroy();
+            if(_state == StateDestroyed)
+            {
+                return;
+            }
+            _state = StateDestroyInProgress;
         }
 
-        if(_objectAdapterFactory != null)
+        try
         {
-            _objectAdapterFactory.destroy();
-        }
+            //
+            // Shutdown and destroy all the incoming and outgoing Ice
+            // connections and wait for the connections to be finished.
+            //
+            if(_objectAdapterFactory != null)
+            {
+                _objectAdapterFactory.shutdown();
+            }
 
-        if(_outgoingConnectionFactory != null)
-        {
-            try
+            if(_outgoingConnectionFactory != null)
             {
-                _outgoingConnectionFactory.waitUntilFinished();
+                _outgoingConnectionFactory.destroy();
             }
-            catch (InterruptedException e)
+
+            if(_objectAdapterFactory != null)
             {
-                throw new Ice.OperationInterruptedException();
+                _objectAdapterFactory.destroy();
             }
-        }
 
-        if(_retryQueue != null)
-        {
-            _retryQueue.destroy();
-        }
+            if(_outgoingConnectionFactory != null)
+            {
+                _outgoingConnectionFactory.waitUntilFinished();
+            }
 
-        if(_initData.observer != null)
-        {
-            _initData.observer.setObserverUpdater(null);
-        }
+            if(_retryQueue != null)
+            {
+                _retryQueue.destroy(); // Must be called before destroying thread pools.
+            }
 
-        if(_initData.logger instanceof LoggerAdminLogger)
-        {
-            //
-            // This only disables the remote logging; we don't set or reset _initData.logger
-            //
-            ((LoggerAdminLogger)_initData.logger).destroy();
-        }
+            if(_initData.observer != null)
+            {
+                _initData.observer.setObserverUpdater(null);
+            }
 
-        ThreadPool serverThreadPool = null;
-        ThreadPool clientThreadPool = null;
-        EndpointHostResolver endpointHostResolver = null;
-        QueueExecutor queueExecutor = null;
-        Ice.PluginManager pluginManager = null;
-        boolean checkUnused = false;
-        synchronized(this)
-        {
-            _objectAdapterFactory = null;
-            _outgoingConnectionFactory = null;
-            _retryQueue = null;
+            if(_initData.logger instanceof LoggerAdminLogger)
+            {
+                //
+                // This only disables the remote logging; we don't set or reset _initData.logger
+                //
+                ((LoggerAdminLogger)_initData.logger).destroy();
+            }
 
+            //
+            // Now, destroy the thread pools. This must be done *only* after
+            // all the connections are finished (the connections destruction
+            // can require invoking callbacks with the thread pools).
+            // 
             if(_serverThreadPool != null)
             {
                 _serverThreadPool.destroy();
-                serverThreadPool = _serverThreadPool;
-                _serverThreadPool = null;
             }
-
             if(_clientThreadPool != null)
             {
                 _clientThreadPool.destroy();
-                clientThreadPool = _clientThreadPool;
-                _clientThreadPool = null;
             }
-
             if(_endpointHostResolver != null)
             {
                 _endpointHostResolver.destroy();
-                endpointHostResolver = _endpointHostResolver;
-                _endpointHostResolver = null;
             }
-
             if(_timer != null)
             {
-                // Shutdown the executor. It isn't necessary to call
-                // awaitTermination since the threads are not daemon and
-                // therefore the VM will block until all threads have
-                // terminated.
-                _timer.shutdown();
-                // Once we support interrupt we can use shutdownNow.
-                //_timer.shutdownNow();
+                _timer.shutdown(); // Don't use shutdownNow(), timers don't support interrupts
+            }
 
-                _timer = null;
+            //
+            // Wait for all the threads to be finished.
+            //
+            try
+            {
+                if(_clientThreadPool != null)
+                {
+                    _clientThreadPool.joinWithAllThreads();
+                }
+                if(_serverThreadPool != null)
+                {
+                    _serverThreadPool.joinWithAllThreads();
+                }
+                if(_endpointHostResolver != null)
+                {
+                    _endpointHostResolver.joinWithThread();
+                }
+                if(_queueExecutor != null)
+                {
+                    _queueExecutor.destroy();
+                }
+                if(_timer != null)
+                {
+                    _timer.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+                }
+            }
+            catch(InterruptedException ex)
+            {
+                throw new Ice.OperationInterruptedException();
             }
 
+            //
+            // NOTE: at this point destroy() can't be interrupted
+            // anymore. The calls bellow are therefore garanteed to be
+            // called once.
+            //
+
             if(_servantFactoryManager != null)
             {
                 _servantFactoryManager.destroy();
-                _servantFactoryManager = null;
             }
 
-            //_referenceFactory.destroy(); // No destroy function defined.
-            _referenceFactory = null;
-            
-            _requestHandlerFactory = null;
-            
-            // _proxyFactory.destroy(); // No destroy function defined.
-            _proxyFactory = null;
-
             if(_routerManager != null)
             {
                 _routerManager.destroy();
-                _routerManager = null;
             }
 
             if(_locatorManager != null)
             {
                 _locatorManager.destroy();
-                _locatorManager = null;
             }
 
             if(_endpointFactoryManager != null)
             {
                 _endpointFactoryManager.destroy();
-                _endpointFactoryManager = null;
             }
 
-            pluginManager = _pluginManager;
-            _pluginManager = null;
-
-            _adminAdapter = null;
-            _adminFacets.clear();
-
-            queueExecutor = _queueExecutor;
-            _queueExecutor = null;
-
-            _queueExecutor = null;
-
-            _typeToClassMap.clear();
-
-            if(_state != StateDestroyed)
+            if(_initData.properties.getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
             {
-                checkUnused = true;
+                java.util.List<String> unusedProperties = ((Ice.PropertiesI)_initData.properties).getUnusedProperties();
+                if(unusedProperties.size() != 0)
+                {
+                    StringBuffer message = new StringBuffer("The following properties were set but never read:");
+                    for(String p : unusedProperties)
+                    {
+                        message.append("\n    ");
+                        message.append(p);
+                    }
+                    _initData.logger.warning(message.toString());
+                }
             }
-            _state = StateDestroyed;
-        }
 
-        try
-        {
             //
-            // Join with threads outside the synchronization.
+            // Destroy last so that a Logger plugin can receive all log/traces before its destruction.
             //
-            if(clientThreadPool != null)
+            if(_pluginManager != null)
             {
-                clientThreadPool.joinWithAllThreads();
+                _pluginManager.destroy();
             }
-            if(serverThreadPool != null)
-            {
-                serverThreadPool.joinWithAllThreads();
-            }
-            if(endpointHostResolver != null)
-            {
-                endpointHostResolver.joinWithThread();
-            }
-            if(queueExecutor != null)
+
+            synchronized(this)
             {
-                queueExecutor.destroy();
+                _objectAdapterFactory = null;
+                _outgoingConnectionFactory = null;
+                _retryQueue = null;
+
+                _serverThreadPool = null;
+                _clientThreadPool = null;
+                _endpointHostResolver = null;
+                _timer = null;
+
+                _servantFactoryManager = null;
+                _referenceFactory = null;
+                _requestHandlerFactory = null;
+                _proxyFactory = null;
+                _routerManager = null;
+                _locatorManager = null;
+                _endpointFactoryManager = null;
+
+                _pluginManager = null;
+
+                _adminAdapter = null;
+                _adminFacets.clear();
+
+                _queueExecutor = null;
+                _queueExecutorService = null;
+
+                _typeToClassMap.clear();
+
+                _state = StateDestroyed;
+                notifyAll();
             }
         }
-        catch(InterruptedException ex)
-        {
-            throw new Ice.OperationInterruptedException();
-        }
-
-        if(checkUnused && _initData.properties.getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
+        finally
         {
-            java.util.List<String> unusedProperties = ((Ice.PropertiesI)_initData.properties).getUnusedProperties();
-            if(unusedProperties.size() != 0)
+            synchronized(this)
             {
-                StringBuffer message = new StringBuffer("The following properties were set but never read:");
-                for(String p : unusedProperties)
+                if(_state == StateDestroyInProgress)
                 {
-                    message.append("\n    ");
-                    message.append(p);
+                    _state = StateActive;
+                    notifyAll();
                 }
-                _initData.logger.warning(message.toString());
             }
         }
-
-        //
-        // Destroy last so that a Logger plugin can receive all log/traces before its destruction.
-        //
-        if(pluginManager != null)
-        {
-            pluginManager.destroy();
-        }
     }
 
     private void
@@ -1576,7 +1604,8 @@ public final class Instance
     }
 
     private static final int StateActive = 0;
-    private static final int StateDestroyed = 1;
+    private static final int StateDestroyInProgress = 1;
+    private static final int StateDestroyed = 2;
     private int _state;
 
     private final Ice.InitializationData _initData; // Immutable, not reset by destroy().
diff --git a/java/src/Ice/src/main/java/IceInternal/OutgoingConnectionFactory.java b/java/src/Ice/src/main/java/IceInternal/OutgoingConnectionFactory.java
index 116038ad537..223820f69dc 100644
--- a/java/src/Ice/src/main/java/IceInternal/OutgoingConnectionFactory.java
+++ b/java/src/Ice/src/main/java/IceInternal/OutgoingConnectionFactory.java
@@ -84,7 +84,6 @@ public final class OutgoingConnectionFactory
     // Called from Instance.destroy().
     public void
     waitUntilFinished()
-        throws InterruptedException
     {
         java.util.Map<Connector, java.util.List<Ice.ConnectionI> > connections = null;
         synchronized(this)
@@ -97,7 +96,14 @@ public final class OutgoingConnectionFactory
             //
             while(!_destroyed || !_pending.isEmpty() || _pendingConnectCount > 0)
             {
-                wait();
+                try
+                {
+                    wait();
+                }
+                catch(InterruptedException ex)
+                {
+                    throw new Ice.OperationInterruptedException();
+                }
             }
 
             //
@@ -130,7 +136,7 @@ public final class OutgoingConnectionFactory
                             c.close(true);
                         }
                     }
-                    throw e;
+                    throw new Ice.OperationInterruptedException();
                 }
             }
         }
diff --git a/java/src/Ice/src/main/java/IceInternal/RetryQueue.java b/java/src/Ice/src/main/java/IceInternal/RetryQueue.java
index 28c10b7bd48..b913c16b8bb 100644
--- a/java/src/Ice/src/main/java/IceInternal/RetryQueue.java
+++ b/java/src/Ice/src/main/java/IceInternal/RetryQueue.java
@@ -30,7 +30,12 @@ public class RetryQueue
 
     synchronized public void destroy()
     {
-         java.util.HashSet<RetryTask> keep = new java.util.HashSet<RetryTask>();
+        if(_instance == null)
+        {
+            return; // Already destroyed.
+        }
+
+        java.util.HashSet<RetryTask> keep = new java.util.HashSet<RetryTask>();
         for(RetryTask task : _requests)
         {
             if(!task.destroy())
diff --git a/js/src/Ice/Instance.js b/js/src/Ice/Instance.js
index ab9fdbfb710..78c148e2729 100644
--- a/js/src/Ice/Instance.js
+++ b/js/src/Ice/Instance.js
@@ -426,23 +426,25 @@ var Instance = Ice.Class({
         var promise = new AsyncResultBase(null, "destroy", null, this, null);
 
         //
-        // If the _state is not StateActive then the instance is
-        // either being destroyed, or has already been destroyed.
+        // If destroy is in progress, wait for it to be done. This is
+        // necessary in case destroy() is called concurrently by
+        // multiple threads.
         //
-        if(this._state != StateActive)
+        if(this._state == StateDestroyInProgress)
         {
-            promise.succeed(promise);
+            if(!this._destroyPromises)
+            {
+                this._destroyPromises = [];
+            }
+            this._destroyPromises.push(promise);
             return promise;
         }
+        this._state = StateDestroyInProgress;
 
         //
-        // We cannot set state to StateDestroyed otherwise instance
-        // methods called during the destroy process (such as
-        // outgoingConnectionFactory() from
-        // ObjectAdapterI::deactivate() will cause an exception.
+        // Shutdown and destroy all the incoming and outgoing Ice
+        // connections and wait for the connections to be finished.
         //
-        this._state = StateDestroyInProgress;
-
         var self = this;
         Ice.Promise.try(
             function()
@@ -480,52 +482,28 @@ var Instance = Ice.Class({
                 {
                     self._retryQueue.destroy();
                 }
-
-                self._objectAdapterFactory = null;
-                self._outgoingConnectionFactory = null;
-                self._retryQueue = null;
-
                 if(self._timer)
                 {
                     self._timer.destroy();
-                    self._timer = null;
                 }
 
                 if(self._servantFactoryManager)
                 {
                     self._servantFactoryManager.destroy();
-                    self._servantFactoryManager = null;
                 }
-
-                //self._referenceFactory.destroy(); // No destroy function defined.
-                self._referenceFactory = null;
-
-                //self._requestHandlerFactory.destroy(); // No destroy function defined.
-                self._requestHandlerFactory = null;
-
-                // self._proxyFactory.destroy(); // No destroy function defined.
-                self._proxyFactory = null;
-
                 if(self._routerManager)
                 {
                     self._routerManager.destroy();
-                    self._routerManager = null;
                 }
-
                 if(self._locatorManager)
                 {
                     self._locatorManager.destroy();
-                    self._locatorManager = null;
                 }
-
                 if(self._endpointFactoryManager)
                 {
                     self._endpointFactoryManager.destroy();
-                    self._endpointFactoryManager = null;
                 }
 
-                self._state = StateDestroyed;
-
                 if(self._initData.properties.getPropertyAsInt("Ice.Warn.UnusedProperties") > 0)
                 {
                     var unusedProperties = self._initData.properties.getUnusedProperties();
@@ -542,12 +520,41 @@ var Instance = Ice.Class({
                     }
                 }
 
+                self._objectAdapterFactory = null;
+                self._outgoingConnectionFactory = null;
+                self._retryQueue = null;
+                self._timer = null;
+
+                self._servantFactoryManager = null;
+                self._referenceFactory = null;
+                self._requestHandlerFactory = null;
+                self._proxyFactory = null;
+                self._routerManager = null;
+                self._locatorManager = null;
+                self._endpointFactoryManager = null;
+
+                self._state = StateDestroyed;
+
+                if(this._destroyPromises)
+                {
+                    for(var i = 0; i < this._destroyPromises.length; ++i)
+                    {
+                        this._destroyPromises[i].succeed(this._destroyPromises[i]);
+                    }
+                }
                 promise.succeed(promise);
             }
         ).exception(
             function(ex)
             {
-                promise.fail(ex);
+                if(this._destroyPromises)
+                {
+                    for(var i = 0; i < this._destroyPromises.length; ++i)
+                    {
+                        this._destroyPromises[i].fail(ex, this._destroyPromises[i]);
+                    }
+                }
+                promise.fail(ex, promise);
             }
         );
         return promise;
diff --git a/js/src/Ice/ObjectAdapterFactory.js b/js/src/Ice/ObjectAdapterFactory.js
index 46e8761b3ce..aede99ab193 100644
--- a/js/src/Ice/ObjectAdapterFactory.js
+++ b/js/src/Ice/ObjectAdapterFactory.js
@@ -47,22 +47,11 @@ var ObjectAdapterFactory = Ice.Class({
 
         this._instance = null;
         this._communicator = null;
-        var self = this;
-        Promise.all(
+        this._shutdownPromise = Promise.all(
             this._adapters.map(function(adapter)
-                                {
-                                    return adapter.deactivate();
-                                })
-        ).then(
-            function()
-            {
-                self._shutdownPromise.succeed();
-            },
-            function(ex)
-            {
-                self._shutdownPromise.fail(ex);
-            }
-        );
+                               {
+                                   return adapter.deactivate();
+                               }));
         return this._shutdownPromise;
     },
     waitForShutdown: function()
@@ -72,9 +61,9 @@ var ObjectAdapterFactory = Ice.Class({
             function()
             {
                 return Promise.all(self._adapters.map(function(adapter)
-                                                        {
-                                                            return adapter.waitForDeactivate();
-                                                        }));
+                                                      {
+                                                          return adapter.waitForDeactivate();
+                                                      }));
             });
     },
     isShutdown: function()
@@ -88,9 +77,9 @@ var ObjectAdapterFactory = Ice.Class({
             function()
             {
                 return Promise.all(self._adapters.map(function(adapter)
-                                                        {
-                                                            return adapter.destroy();
-                                                        }));
+                                                      {
+                                                          return adapter.destroy();
+                                                      }));
             });
     },
     createObjectAdapter: function(name, router, promise)
diff --git a/js/src/Ice/ObjectAdapterI.js b/js/src/Ice/ObjectAdapterI.js
index 9c4119b73ea..ec069e75fa8 100644
--- a/js/src/Ice/ObjectAdapterI.js
+++ b/js/src/Ice/ObjectAdapterI.js
@@ -313,8 +313,7 @@ var ObjectAdapterI = Ice.Class({
             return promise.succeed(promise);
         };
 
-        return this._state < StateDeactivated ?
-            this.deactivate().then(destroyInternal) : destroyInternal();        
+        return this._state < StateDeactivated ? this.deactivate().then(destroyInternal) : destroyInternal();
     },
     add: function(object, ident)
     {
diff --git a/js/test/Ice/operations/Client.js b/js/test/Ice/operations/Client.js
index 8cdf7298955..66a8d8b5273 100644
--- a/js/test/Ice/operations/Client.js
+++ b/js/test/Ice/operations/Client.js
@@ -91,6 +91,7 @@
         ).finally(
             function()
             {
+                c.destroy(); // Test concurrent destroy() calls
                 return c.destroy();
             }
         );