Better fix for #291

5 files changed, 139 insertions, 69 deletions

diff --git a/cpp/src/Ice/ConnectionFactory.cpp b/cpp/src/Ice/ConnectionFactory.cpp
index f6f61e6640a..41c7dd448c6 100644
--- a/cpp/src/Ice/ConnectionFactory.cpp
+++ b/cpp/src/Ice/ConnectionFactory.cpp
@@ -152,6 +152,27 @@ private:
     InstancePtr _instance;
 };
 
+#if TARGET_OS_IPHONE != 0
+class FinishCall : public DispatchWorkItem
+{
+public:
+
+    FinishCall(const IncomingConnectionFactoryPtr& factory) : _factory(factory)
+    {
+    }
+
+    virtual void
+    run()
+    {
+        _factory->finish();
+    }
+
+private:
+
+    const IncomingConnectionFactoryPtr _factory;
+};
+#endif
+
 }
 
 bool
@@ -1431,9 +1452,13 @@ IceInternal::IncomingConnectionFactory::finishAsync(SocketOperation)
 
         Error out(_instance->initializationData().logger);
         out << "couldn't accept connection:\n" << ex << '\n' << _acceptor->toString();
-        if(_adapter->getThreadPool()->finish(ICE_SHARED_FROM_THIS, true))
+        if(_acceptorStarted)
         {
-            closeAcceptor();
+            _acceptorStarted = false;
+            if(_adapter->getThreadPool()->finish(ICE_SHARED_FROM_THIS, true))
+            {
+                closeAcceptor();
+            }
         }
     }
     return _state < StateClosed;
@@ -1502,6 +1527,8 @@ IceInternal::IncomingConnectionFactory::message(ThreadPoolCurrent& current)
                 Error out(_instance->initializationData().logger);
                 out << "can't accept more connections:\n" << ex << '\n' << _acceptor->toString();
 
+                assert(_acceptorStarted);
+                _acceptorStarted = false;
                 if(_adapter->getThreadPool()->finish(ICE_SHARED_FROM_THIS, true))
                 {
                     closeAcceptor();
@@ -1562,7 +1589,7 @@ IceInternal::IncomingConnectionFactory::finished(ThreadPoolCurrent&, bool close)
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(*this);
     if(_state < StateClosed)
     {
-        if(_acceptorStarted && close)
+        if(close)
         {
             closeAcceptor();
         }
@@ -1578,22 +1605,29 @@ IceInternal::IncomingConnectionFactory::finished(ThreadPoolCurrent&, bool close)
         }
         return;
     }
-    else if(_state == StateClosed)
-    {
-        setState(StateFinished);
 
-        if(_acceptorStarted && close)
-        {
-            closeAcceptor();
-        }
+    assert(_state == StateClosed);
+    setState(StateFinished);
+
+    if(close)
+    {
+        closeAcceptor();
+    }
 
 #if TARGET_OS_IPHONE != 0
-        sync.release();
-        unregisterForBackgroundNotification(ICE_SHARED_FROM_THIS);
+    sync.release();
+    finish();
 #endif
-    }
 }
 
+#if TARGET_OS_IPHONE != 0
+void
+IceInternal::IncomingConnectionFactory::finish()
+{
+    unregisterForBackgroundNotification(ICE_SHARED_FROM_THIS);
+}
+#endif
+
 string
 IceInternal::IncomingConnectionFactory::toString() const
 {
@@ -1703,7 +1737,7 @@ IceInternal::IncomingConnectionFactory::stopAcceptor()
     }
 
     _acceptorStopped = true;
-
+    _acceptorStarted = false;
     if(_adapter->getThreadPool()->finish(ICE_SHARED_FROM_THIS, true))
     {
         closeAcceptor();
@@ -1842,20 +1876,28 @@ IceInternal::IncomingConnectionFactory::setState(State state)
 
         case StateClosed:
         {
-            //
-            // If possible, close the acceptor now to prevent new connections from
-            // being accepted while we are deactivating. This is especially useful
-            // if there are no more threads in the thread pool available to dispatch
-            // the finish() call. Not all selector implementations do support this
-            // however.
-            //
-            if(_adapter->getThreadPool()->finish(ICE_SHARED_FROM_THIS, true))
+            if(_acceptorStarted)
             {
-                if(_acceptorStarted)
+                //
+                // If possible, close the acceptor now to prevent new connections from
+                // being accepted while we are deactivating. This is especially useful
+                // if there are no more threads in the thread pool available to dispatch
+                // the finish() call. Not all selector implementations do support this
+                // however.
+                //
+                _acceptorStarted = false;
+                if(_adapter->getThreadPool()->finish(ICE_SHARED_FROM_THIS, true))
                 {
                     closeAcceptor();
                 }
             }
+            else
+            {
+#if TARGET_OS_IPHONE != 0
+                _adapter->getThreadPool()->dispatch(new FinishCall(ICE_SHARED_FROM_THIS));
+#endif
+                state = StateFinished;
+            }
 
 #ifdef ICE_CPP11_COMPILER
             for(const auto& conn : _connections)
@@ -1930,6 +1972,6 @@ IceInternal::IncomingConnectionFactory::closeAcceptor()
         out << "stopping to accept " << _endpoint->protocol() << " connections at " << _acceptor->toString();
     }
 
-    _acceptorStarted = false;
+    assert(!_acceptorStarted);
     _acceptor->close();
 }
diff --git a/cpp/src/Ice/ConnectionFactory.h b/cpp/src/Ice/ConnectionFactory.h
index f5f95ce9d4d..d44874d0b23 100644
--- a/cpp/src/Ice/ConnectionFactory.h
+++ b/cpp/src/Ice/ConnectionFactory.h
@@ -205,6 +205,9 @@ public:
 
     virtual void message(ThreadPoolCurrent&);
     virtual void finished(ThreadPoolCurrent&, bool);
+#if TARGET_OS_IPHONE != 0
+    void finish();
+#endif
     virtual std::string toString() const;
     virtual NativeInfoPtr getNativeInfo();
 
diff --git a/csharp/src/Ice/ConnectionFactory.cs b/csharp/src/Ice/ConnectionFactory.cs
index ce7941142d5..786d031e24d 100644
--- a/csharp/src/Ice/ConnectionFactory.cs
+++ b/csharp/src/Ice/ConnectionFactory.cs
@@ -1377,8 +1377,12 @@ namespace IceInternal
 
                 string s = "couldn't accept connection:\n" + ex + '\n' + _acceptor.ToString();
                 _instance.initializationData().logger.error(s);
-                _adapter.getThreadPool().finish(this);
-                closeAcceptor();
+                if(_acceptorStarted)
+                {
+                    _acceptorStarted = false;
+                    _adapter.getThreadPool().finish(this);
+                    closeAcceptor();
+                }
             }
             return _state < StateClosed;
         }
@@ -1447,6 +1451,8 @@ namespace IceInternal
                         {
                             string s = "can't accept more connections:\n" + ex + '\n' + _acceptor.ToString();
                             _instance.initializationData().logger.error(s);
+                            Debug.Assert(_acceptorStarted);
+                            _acceptorStarted = false;
                             _adapter.getThreadPool().finish(this);
                             closeAcceptor();
                         }
@@ -1508,15 +1514,15 @@ namespace IceInternal
                 if(_state < StateClosed)
                 {
                     //
-                    // If the acceptor got closed because of an un-expected error, try to restart it in 1 second.
+                    // If the acceptor hasn't been explicitly stopped (which is the case if the acceptor got closed
+                    // because of an unexpected error), try to restart the acceptor in 1 second.
                     //
                     _instance.timer().schedule(new StartAcceptor(this), 1000);
                     return;
                 }
-                else if(_state == StateClosed)
-                {
-                    setState(StateFinished);
-                }
+
+                Debug.Assert(_state == StateClosed);
+                setState(StateFinished);
             }
         }
 
@@ -1713,11 +1719,16 @@ namespace IceInternal
 
                 case StateClosed:
                 {
-                    _adapter.getThreadPool().finish(this);
                     if(_acceptorStarted)
                     {
+                        _acceptorStarted = false;
+                        _adapter.getThreadPool().finish(this);
                         closeAcceptor();
                     }
+                    else
+                    {
+                        state = StateFinished;
+                    }
 
                     foreach(Ice.ConnectionI connection in _connections)
                     {
@@ -1796,7 +1807,7 @@ namespace IceInternal
                 _instance.initializationData().logger.trace(_instance.traceLevels().networkCat, s.ToString());
             }
 
-            _acceptorStarted = false;
+            Debug.Assert(!_acceptorStarted);
             _acceptor.close();
         }
 
diff --git a/java-compat/src/Ice/src/main/java/IceInternal/IncomingConnectionFactory.java b/java-compat/src/Ice/src/main/java/IceInternal/IncomingConnectionFactory.java
index b73eaa3c251..aa3ed035f46 100644
--- a/java-compat/src/Ice/src/main/java/IceInternal/IncomingConnectionFactory.java
+++ b/java-compat/src/Ice/src/main/java/IceInternal/IncomingConnectionFactory.java
@@ -312,6 +312,8 @@ public final class IncomingConnectionFactory extends EventHandler implements Ice
                         // Ignore, could be a class loading error.
                     }
 
+                    assert(_acceptorStarted);
+                    _acceptorStarted = false;
                     if(_adapter.getThreadPool().finish(this, true))
                     {
                         closeAcceptor();
@@ -369,13 +371,14 @@ public final class IncomingConnectionFactory extends EventHandler implements Ice
     {
         if(_state < StateClosed)
         {
-            if(_acceptorStarted && close)
+            if(close)
             {
                 closeAcceptor();
             }
 
             //
-            // If the acceptor got closed because of an un-expected error, try to restart it in 1 second.
+            // If the acceptor hasn't been explicitly stopped (which is the case if the acceptor got closed
+            // because of an unexpected error), try to restart the acceptor in 1 second.
             //
             _instance.timer().schedule(new Runnable()
                                        {
@@ -386,14 +389,13 @@ public final class IncomingConnectionFactory extends EventHandler implements Ice
                                        }, 1, java.util.concurrent.TimeUnit.SECONDS);
             return;
         }
-        else if(_state == StateClosed)
-        {
-            setState(StateFinished);
 
-            if(_acceptorStarted && close)
-            {
-                closeAcceptor();
-            }
+        assert(_state == StateClosed);
+        setState(StateFinished);
+
+        if(close)
+        {
+            closeAcceptor();
         }
     }
 
@@ -630,19 +632,24 @@ public final class IncomingConnectionFactory extends EventHandler implements Ice
 
             case StateClosed:
             {
-                //
-                // If possible, close the acceptor now to prevent new connections from
-                // being accepted while we are deactivating. This is especially useful
-                // if there are no more threads in the thread pool available to dispatch
-                // the finish() call.
-                //
-                if(_adapter.getThreadPool().finish(this, true))
+                if(_acceptorStarted)
                 {
-                    if(_acceptorStarted)
+                    //
+                    // If possible, close the acceptor now to prevent new connections from
+                    // being accepted while we are deactivating. This is especially useful
+                    // if there are no more threads in the thread pool available to dispatch
+                    // the finish() call.
+                    //
+                    _acceptorStarted = false;
+                    if(_adapter.getThreadPool().finish(this, true))
                     {
                         closeAcceptor();
                     }
                 }
+                else
+                {
+                    state = StateFinished;
+                }
 
                 for(Ice.ConnectionI connection : _connections)
                 {
@@ -724,7 +731,7 @@ public final class IncomingConnectionFactory extends EventHandler implements Ice
             _instance.initializationData().logger.trace(_instance.traceLevels().networkCat, s.toString());
         }
 
-        _acceptorStarted = false;
+        assert(!_acceptorStarted);
         _acceptor.close();
     }
 
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/IncomingConnectionFactory.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/IncomingConnectionFactory.java
index 4f068a74651..88013d9bd62 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/IncomingConnectionFactory.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/IncomingConnectionFactory.java
@@ -308,6 +308,8 @@ public final class IncomingConnectionFactory extends EventHandler implements Con
                         // Ignore, could be a class loading error.
                     }
 
+                    assert(_acceptorStarted);
+                    _acceptorStarted = false;
                     if(_adapter.getThreadPool().finish(this, true))
                     {
                         closeAcceptor();
@@ -365,25 +367,25 @@ public final class IncomingConnectionFactory extends EventHandler implements Con
     {
         if(_state < StateClosed)
         {
-            if(_acceptorStarted && close)
+            if(close)
             {
                 closeAcceptor();
             }
 
             //
-            // If the acceptor got closed because of an un-expected error, try to restart it in 1 second.
+            // If the acceptor hasn't been explicitly stopped (which is the case if the acceptor got closed
+            // because of an unexpected error), try to restart the acceptor in 1 second.
             //
             _instance.timer().schedule(() -> startAcceptor(), 1, java.util.concurrent.TimeUnit.SECONDS);
             return;
         }
-        else if(_state == StateClosed)
-        {
-            setState(StateFinished);
 
-            if(_acceptorStarted && close)
-            {
-                closeAcceptor();
-            }
+        assert(_state == StateClosed);
+        setState(StateFinished);
+
+        if(close)
+        {
+            closeAcceptor();
         }
     }
 
@@ -621,19 +623,24 @@ public final class IncomingConnectionFactory extends EventHandler implements Con
 
             case StateClosed:
             {
-                //
-                // If possible, close the acceptor now to prevent new connections from
-                // being accepted while we are deactivating. This is especially useful
-                // if there are no more threads in the thread pool available to dispatch
-                // the finish() call.
-                //
-                if(_adapter.getThreadPool().finish(this, true))
+                if(_acceptorStarted)
                 {
-                    if(_acceptorStarted)
+                    //
+                    // If possible, close the acceptor now to prevent new connections from
+                    // being accepted while we are deactivating. This is especially useful
+                    // if there are no more threads in the thread pool available to dispatch
+                    // the finish() call.
+                    //
+                    _acceptorStarted = false;
+                    if(_adapter.getThreadPool().finish(this, true))
                     {
                         closeAcceptor();
                     }
                 }
+                else
+                {
+                    state = StateFinished;
+                }
 
                 for(ConnectionI connection : _connections)
                 {
@@ -715,7 +722,7 @@ public final class IncomingConnectionFactory extends EventHandler implements Con
             _instance.initializationData().logger.trace(_instance.traceLevels().networkCat, s.toString());
         }
 
-        _acceptorStarted = false;
+        assert(!_acceptorStarted);
         _acceptor.close();
     }