Fixed ICE-8237 - deadlock in ConnectRequestHandler

4 files changed, 47 insertions, 40 deletions

diff --git a/cpp/src/Ice/ConnectRequestHandler.cpp b/cpp/src/Ice/ConnectRequestHandler.cpp
index b201a5c1c30..285402b333a 100644
--- a/cpp/src/Ice/ConnectRequestHandler.cpp
+++ b/cpp/src/Ice/ConnectRequestHandler.cpp
@@ -170,12 +170,15 @@ ConnectRequestHandler::setConnection(const Ice::ConnectionIPtr& connection, bool
 void
 ConnectRequestHandler::setException(const Ice::LocalException& ex)
 {
-    Lock sync(*this);
-    assert(!_initialized && !_exception);
-    ICE_SET_EXCEPTION_FROM_CLONE(_exception, ex.ice_clone());
+    {
+        Lock sync(*this);
+        assert(!_initialized && !_exception);
+        ICE_SET_EXCEPTION_FROM_CLONE(_exception, ex.ice_clone());
 
-    _proxies.clear();
-    _proxy = 0; // Break cyclic reference count.
+        _proxies.clear();
+        _proxy = 0; // Break cyclic reference count.
+        notifyAll();
+    }
 
     //
     // NOTE: remove the request handler *before* notifying the
@@ -199,9 +202,7 @@ ConnectRequestHandler::setException(const Ice::LocalException& ex)
             (*p)->invokeExceptionAsync();
         }
     }
-
     _requests.clear();
-    notifyAll();
 }
 
 void
diff --git a/csharp/src/Ice/ConnectRequestHandler.cs b/csharp/src/Ice/ConnectRequestHandler.cs
index da1cd6e90fd..a03a6adb02c 100644
--- a/csharp/src/Ice/ConnectRequestHandler.cs
+++ b/csharp/src/Ice/ConnectRequestHandler.cs
@@ -138,32 +138,32 @@ namespace IceInternal
                 _exception = ex;
                 _proxies.Clear();
                 _proxy = null; // Break cyclic reference count.
+                Monitor.PulseAll(this);
+            }
 
-                //
-                // NOTE: remove the request handler *before* notifying the
-                // requests that the connection failed. It's important to ensure
-                // that future invocations will obtain a new connect request
-                // handler once invocations are notified.
-                //
-                try
-                {
-                    _reference.getInstance().requestHandlerFactory().removeRequestHandler(_reference, this);
-                }
-                catch(Ice.CommunicatorDestroyedException)
-                {
-                    // Ignore
-                }
+            //
+            // NOTE: remove the request handler *before* notifying the
+            // requests that the connection failed. It's important to ensure
+            // that future invocations will obtain a new connect request
+            // handler once invocations are notified.
+            //
+            try
+            {
+                _reference.getInstance().requestHandlerFactory().removeRequestHandler(_reference, this);
+            }
+            catch(Ice.CommunicatorDestroyedException)
+            {
+                // Ignore
+            }
 
-                foreach(ProxyOutgoingAsyncBase outAsync in _requests)
+            foreach(ProxyOutgoingAsyncBase outAsync in _requests)
+            {
+                if(outAsync.exception(_exception))
                 {
-                    if(outAsync.exception(_exception))
-                    {
-                        outAsync.invokeExceptionAsync();
-                    }
+                    outAsync.invokeExceptionAsync();
                 }
-                _requests.Clear();
-                Monitor.PulseAll(this);
             }
+            _requests.Clear();
         }
 
         //
diff --git a/java-compat/src/Ice/src/main/java/IceInternal/ConnectRequestHandler.java b/java-compat/src/Ice/src/main/java/IceInternal/ConnectRequestHandler.java
index 3ea6ad4741c..b8e9426cc63 100644
--- a/java-compat/src/Ice/src/main/java/IceInternal/ConnectRequestHandler.java
+++ b/java-compat/src/Ice/src/main/java/IceInternal/ConnectRequestHandler.java
@@ -140,13 +140,17 @@ public class ConnectRequestHandler
     }
 
     @Override
-    public synchronized void
+    public void
     setException(final Ice.LocalException ex)
     {
-        assert(!_initialized && _exception == null);
-        _exception = ex;
-        _proxies.clear();
-        _proxy = null; // Break cyclic reference count.
+        synchronized(this)
+        {
+            assert(!_initialized && _exception == null);
+            _exception = ex;
+            _proxies.clear();
+            _proxy = null; // Break cyclic reference count.
+            notifyAll();
+        }
 
         //
         // NOTE: remove the request handler *before* notifying the
@@ -171,7 +175,6 @@ public class ConnectRequestHandler
             }
         }
         _requests.clear();
-        notifyAll();
     }
 
     //
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectRequestHandler.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectRequestHandler.java
index 47b2d4ce287..bf98fe557e2 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectRequestHandler.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectRequestHandler.java
@@ -138,13 +138,17 @@ public class ConnectRequestHandler
     }
 
     @Override
-    public synchronized void
+    public void
     setException(final com.zeroc.Ice.LocalException ex)
     {
-        assert(!_initialized && _exception == null);
-        _exception = ex;
-        _proxies.clear();
-        _proxy = null; // Break cyclic reference count.
+        synchronized(this)
+        {
+            assert(!_initialized && _exception == null);
+            _exception = ex;
+            _proxies.clear();
+            _proxy = null; // Break cyclic reference count.
+            notifyAll();
+        }
 
         //
         // NOTE: remove the request handler *before* notifying the
@@ -169,7 +173,6 @@ public class ConnectRequestHandler
             }
         }
         _requests.clear();
-        notifyAll();
     }
 
     //