ICE-6302 Fixed SessionHelper to terminate connection attempt when destroy is called

4 files changed, 137 insertions, 16 deletions

diff --git a/CHANGELOG-3.6.md b/CHANGELOG-3.6.md
index 2a4f56fb3e2..6a1cddf1d4c 100644
--- a/CHANGELOG-3.6.md
+++ b/CHANGELOG-3.6.md
@@ -2,8 +2,10 @@ The entries below contain brief descriptions of the changes in a release, in no
 
 We recommend that you use the release notes as a guide for migrating your applications to this release, and the manual for complete details on a particular aspect of Ice.
 
-- [Changes in Ice 3.6.0](#changes-in-ice-360)
+- [Changes in Ice 3.6.1](#changes-in-ice-361)
   - [General Changes](#general-changes)
+- [Changes in Ice 3.6.0](#changes-in-ice-360)
+  - [General Changes](#general-changes-1)
   - [C++ Changes](#c-changes)
   - [Java Changes](#java-changes)
   - [C# Changes](#c#-changes)
@@ -13,13 +15,39 @@ We recommend that you use the release notes as a guide for migrating your applic
   - [PHP Changes](#php-changes)
   - [Objective-C Changes](#objective-c-changes)
 
+# Changes in Ice 3.6.1
+
+These are the changes since Ice 3.6.0.
+
+## General Changes
+
+- Calling `destroy()` on the `Glacier2::SessionHelper` during connection establishment will now immediately cause the termination of the connection attempt and `SessionCallback::connectFailed()` will be called with a `CommunicatorDestroyedException`. Previously, the helper would wait until the connection attempt either succeeded or failed before calling `SessionCallback::disconnected()` or `SessionCallback::connectFailed`.
+
+- Fixed a bug in Slice compilers which would incorrectly reject valid Slice where a parameter with the same name as a type would prevent further use of that type in the same scope. For example:
+  ```
+  module Test
+  {
+  sequence<int> Seq;
+  struct S1
+  {
+    Seq seq;
+    Seq seq2; // This would fail with 'Seq is not a type'
+  };
+
+  interface Intf
+  {
+    void method(Seq seq, Seq seq2); // This would fail with 'Seq is not a type' for second parameter
+  };
+  }
+  ```
+
 # Changes in Ice 3.6.0
 
 These are the changes since Ice 3.5.1.
 
 ## General Changes
 
-- Exceptions raised by the Ice::Plugin initialize method are now caught by the communicator, which raises Ice::PluginInitializationException instead.
+- Exceptions raised by the `Ice::Plugin` initialize method are now caught by the communicator, which raises `Ice::PluginInitializationException` instead.
 
 - Fixed IceGrid bug where an application update could fail and leave the registry in an invalid state if some allocation requests were pending while the application was updated.
 
diff --git a/cpp/src/Glacier2Lib/SessionHelper.cpp b/cpp/src/Glacier2Lib/SessionHelper.cpp
index 0ebfeaa98aa..fde5247a843 100644
--- a/cpp/src/Glacier2Lib/SessionHelper.cpp
+++ b/cpp/src/Glacier2Lib/SessionHelper.cpp
@@ -94,6 +94,7 @@ public:
     Ice::ObjectAdapterPtr objectAdapter();
 
     friend class DestroyInternal;
+    friend class DestroyCommunicator;
     friend class ConnectThread;
     friend class DispatchCallThread;
     friend class Glacier2::SessionFactoryHelper;
@@ -103,6 +104,7 @@ private:
     Ice::ObjectAdapterPtr internalObjectAdapter();
     void connected(const Glacier2::RouterPrx&, const Glacier2::SessionPrx&);
     void destroyInternal(const Ice::DispatcherCallPtr&);
+    void destroyCommunicator();
     void connectFailed();
 
     void connect(const std::map<std::string, std::string>&);
@@ -163,6 +165,27 @@ private:
     const Ice::DispatcherCallPtr _disconnected;
 };
 
+class DestroyCommunicator : public IceUtil::Thread
+{
+
+public:
+
+    DestroyCommunicator(const SessionHelperIPtr& session) :
+        _session(session)
+    {
+    }
+
+    virtual void run()
+    {
+        _session->destroyCommunicator();
+        _session = 0;
+    }
+
+private:
+
+    SessionHelperIPtr _session;
+};
+
 }
 
 SessionHelperI::SessionHelperI(const Glacier2::SessionThreadCallbackPtr& threadCB,
@@ -193,12 +216,13 @@ SessionHelperI::destroy()
     if(!_connected)
     {
         //
-        // In this case a connecting session is being
-        // destroyed. The communicator and session will be
-        // destroyed when the connection establishment has
-        // completed.
+        // In this case a connecting session is being destroyed.
+        // We destroy the communicator to trigger the immediate
+        // failure of the connection establishment.
         //
+        IceUtil::ThreadPtr destroyCommunicator = new DestroyCommunicator(this);
         _threadCB = 0;
+        destroyCommunicator->start();
         return;
     }
 
@@ -424,6 +448,28 @@ SessionHelperI::destroyInternal(const Ice::DispatcherCallPtr& disconnected)
 }
 
 void
+SessionHelperI::destroyCommunicator()
+{
+    Ice::CommunicatorPtr communicator;
+    {
+        IceUtil::Mutex::Lock sync(_mutex);
+        communicator = _communicator;
+    }
+
+    if(communicator)
+    {
+        try
+        {
+            communicator->destroy();
+        }
+        catch(...)
+        {
+        }
+        communicator = 0;
+    }
+}
+
+void
 SessionHelperI::connectFailed()
 {
     Ice::CommunicatorPtr communicator;
diff --git a/csharp/src/Glacier2/SessionHelper.cs b/csharp/src/Glacier2/SessionHelper.cs
index f9432d6003f..d1f60742c74 100644
--- a/csharp/src/Glacier2/SessionHelper.cs
+++ b/csharp/src/Glacier2/SessionHelper.cs
@@ -76,20 +76,22 @@ public class SessionHelper
             if(!_connected)
             {
                 //
-                // In this case a connecting session is being
-                // destroyed. The communicator and session will be
-                // destroyed when the connection establishment has
-                // completed.
+                // In this case a connecting session is being destroyed.
+                // We destroy the communicator to trigger the immediate
+                // failure of the connection establishment.
                 //
+                Thread t1 = new Thread(new ThreadStart(destroyCommunicator));
+                t1.Start();
                 return;
             }
             _session = null;
             _connected = false;
+
             //
             // Run destroyInternal in a thread because it makes remote invocations.
             //
-            Thread t = new Thread(new ThreadStart(destroyInternal));
-            t.Start();
+            Thread t2 = new Thread(new ThreadStart(destroyInternal));
+            t2.Start();
         }
     }
 
@@ -394,6 +396,23 @@ public class SessionHelper
             }, null);
     }
 
+    private void
+    destroyCommunicator()
+    {
+        Ice.Communicator communicator;
+        lock(this)
+        {
+            communicator = _communicator;
+        }
+        try
+        {
+            communicator.destroy();
+        }
+        catch(Exception)
+        {
+        }
+    }
+
     delegate Glacier2.SessionPrx ConnectStrategy(Glacier2.RouterPrx router);
 
     private void
diff --git a/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java b/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java
index 904fa652423..b5bc909c113 100644
--- a/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java
+++ b/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java
@@ -50,11 +50,18 @@ public class SessionHelper
             if(!_connected)
             {
                 //
-                // In this case a connecting session is being
-                // destroyed. The communicator and session will be
-                // destroyed when the connection establishment has
-                // completed.
+                // In this case a connecting session is being destroyed.
+                // We destroy the communicator to trigger the immediate
+                // failure of the connection establishment.
                 //
+                new Thread(new Runnable()
+                {
+                    @Override
+                    public void run()
+                    {
+                        destroyCommunicator();
+                    }
+                }).start();
                 return;
             }
             _session = null;
@@ -446,6 +453,27 @@ public class SessionHelper
     }
 
     private void
+    destroyCommunicator()
+    {
+        Ice.Communicator communicator = null;
+        synchronized(this)
+        {
+            communicator = _communicator;
+        }
+
+        if(communicator != null)
+        {
+            try
+            {
+                _communicator.destroy();
+            }
+            catch(Throwable ex)
+            {
+            }
+        }
+    }
+
+    private void
     connectImpl(final ConnectStrategy factory)
     {
         assert !_destroy;