bug 4718 - Glacier2 helpers need to trap CommunicatorDestroyedException

4 files changed, 129 insertions, 66 deletions

diff --git a/java/src/Glacier2/Application.java b/java/src/Glacier2/Application.java
index 463d35aa9c8..cddb824a93d 100644
--- a/java/src/Glacier2/Application.java
+++ b/java/src/Glacier2/Application.java
@@ -75,7 +75,6 @@ public abstract class Application extends Ice.Application
         super(signalPolicy);
     }
 
-
     /**
      * Called once the communicator has been initialized and the Glacier2 session
      * has been established. A derived class must implement <code>runWithSession</code>,
@@ -169,7 +168,8 @@ public abstract class Application extends Ice.Application
      * @throws SessionNotExistException No session exists.
      **/
     public String
-    categoryForClient() throws SessionNotExistException
+    categoryForClient()
+        throws SessionNotExistException
     {
         if(_router == null)
         {
@@ -185,7 +185,8 @@ public abstract class Application extends Ice.Application
      * @throws SessionNotExistException No session exists.
      **/
     public Ice.Identity
-    createCallbackIdentity(String name) throws SessionNotExistException
+    createCallbackIdentity(String name)
+        throws SessionNotExistException
     {
         return new Ice.Identity(name, categoryForClient());
     }
@@ -197,7 +198,8 @@ public abstract class Application extends Ice.Application
      * @throws SessionNotExistException No session exists.
      **/
     public Ice.ObjectPrx
-    addWithUUID(Ice.Object servant) throws SessionNotExistException 
+    addWithUUID(Ice.Object servant)
+        throws SessionNotExistException
     {
         return objectAdapter().add(servant, createCallbackIdentity(java.util.UUID.randomUUID().toString()));
     }
@@ -208,7 +210,8 @@ public abstract class Application extends Ice.Application
      * @throws SessionNotExistException No session exists.
      */
     public synchronized Ice.ObjectAdapter
-    objectAdapter() throws SessionNotExistException
+    objectAdapter()
+        throws SessionNotExistException
     {
         if(_adapter == null)
         {
@@ -236,35 +239,45 @@ public abstract class Application extends Ice.Application
         {
             while(true)
             {
-                _router.refreshSession_async(new Glacier2.AMI_Router_refreshSession()
-                    {
-                        public void
-                        ice_response()
-                        {
-                        }
-
-                        public void
-                        ice_exception(Ice.LocalException ex)
-                        {
-                            // Here the session has gone. The thread
-                            // terminates, and we notify the
-                            // application that the session has been
-                            // destroyed.
-                            done();
-                            sessionDestroyed();
-                        }
-
-                        public void
-                        ice_exception(Ice.UserException ex)
+                try
+                {
+                    _router.refreshSession_async(new Glacier2.AMI_Router_refreshSession()
                         {
-                            // Here the session has gone. The thread
-                            // terminates, and we notify the
-                            // application that the session has been
-                            // destroyed.
-                            done();
-                            sessionDestroyed();
-                        }
-                    });
+                            public void
+                            ice_response()
+                            {
+                            }
+
+                            public void
+                            ice_exception(Ice.LocalException ex)
+                            {
+                                //
+                                // Here the session has gone. The thread terminates, and we notify the
+                                // application that the session has been destroyed.
+                                //
+                                done();
+                                sessionDestroyed();
+                            }
+
+                            public void
+                            ice_exception(Ice.UserException ex)
+                            {
+                                //
+                                // Here the session has gone. The thread terminates, and we notify the
+                                // application that the session has been destroyed.
+                                //
+                                done();
+                                sessionDestroyed();
+                            }
+                        });
+                }
+                catch(Ice.CommunicatorDestroyedException ex)
+                {
+                    //
+                    // AMI requests can raise CommunicatorDestroyedException directly.
+                    //
+                    break;
+                }
 
                 if(!_done)
                 {
@@ -302,7 +315,9 @@ public abstract class Application extends Ice.Application
     protected int
     doMain(Ice.StringSeqHolder argHolder, Ice.InitializationData initData)
     {
+        //
         // Set the default properties for all Glacier2 applications.
+        //
         initData.properties.setProperty("Ice.ACM.Client", "0");
         initData.properties.setProperty("Ice.RetryIntervals", "-1");
 
@@ -310,9 +325,11 @@ public abstract class Application extends Ice.Application
         Ice.IntHolder ret = new Ice.IntHolder();
         do
         {
-            // A copy of the initialization data and the string seq
+            //
+            // A copy of the initialization data and the string array
             // needs to be passed to doMainInternal, as these can be
             // changed by the application.
+            //
             Ice.InitializationData id = (Ice.InitializationData)initData.clone();
             id.properties = id.properties._clone();
             Ice.StringSeqHolder h = new Ice.StringSeqHolder();
@@ -327,8 +344,10 @@ public abstract class Application extends Ice.Application
     private boolean
     doMain(Ice.StringSeqHolder argHolder, Ice.InitializationData initData, Ice.IntHolder status)
     {
+        //
         // Reset internal state variables from Ice.Application. The
         // remainder are reset at the end of this method.
+        //
         _callbackInProgress = false;
         _destroyed = false;
         _interrupted = false;
@@ -357,7 +376,9 @@ public abstract class Application extends Ice.Application
                     destroyOnInterrupt();
                 }
 
+                //
                 // If createSession throws, we're done.
+                //
                 try
                 {
                     _session = createSession();
@@ -377,10 +398,12 @@ public abstract class Application extends Ice.Application
                 }
             }
         }
-        // We want to restart on those exceptions which indicate a
+        //
+        // We want to restart on those exceptions that indicate a
         // break down in communications, but not those exceptions that
-        // indicate a programming logic error (ie: marshal, protocol
+        // indicate a programming logic error (i.e., marshal, protocol
         // failure, etc).
+        //
         catch(RestartSessionException ex)
         {
             restart = true;
@@ -429,7 +452,9 @@ public abstract class Application extends Ice.Application
             status.value = 1;
         }
 
+        //
         // This clears any set interrupt.
+        //
         if(_signalPolicy == Ice.SignalPolicy.HandleSignals)
         {
             defaultInterrupt();
@@ -458,7 +483,7 @@ public abstract class Application extends Ice.Application
                 //
                 // And _communicator != null, meaning will be
                 // destroyed next, _destroyed = true also ensures that
-                // any remaining callback won't do anything
+                // any remaining callback won't do anything.
                 //
             }
         }
@@ -488,15 +513,21 @@ public abstract class Application extends Ice.Application
             }
             catch(Ice.ConnectionLostException ex)
             {
+                //
                 // Expected if another thread invoked on an object from the session concurrently.
+                //
             }
             catch(Glacier2.SessionNotExistException ex)
             {
+                //
                 // This can also occur.
+                //
             }
             catch(Throwable ex)
             {
+                //
                 // Not expected.
+                //
                 Ice.Util.getProcessLogger().error("unexpected exception when destroying the session:\n" + 
                                                   IceInternal.Ex.toString(ex));
             }
@@ -530,9 +561,11 @@ public abstract class Application extends Ice.Application
             }
         }
 
+        //
         // Reset internal state. We cannot reset the Application state
         // here, since _destroyed must remain true until we re-run
         // this method.
+        //
         _adapter = null;
         _router = null;
         _session = null;
diff --git a/java/src/Glacier2/SessionCallback.java b/java/src/Glacier2/SessionCallback.java
index ac096010d9a..2af9d214108 100644
--- a/java/src/Glacier2/SessionCallback.java
+++ b/java/src/Glacier2/SessionCallback.java
@@ -20,16 +20,14 @@ public interface SessionCallback
      *
      * @param session The Glacier2 session.
      */
-    void
-    createdCommunicator(SessionHelper session);
+    void createdCommunicator(SessionHelper session);
 
     /**
      * Notifies the application that the Glacier2 session has been established.
      *  
      * @param session The established session.
      */
-    void
-    connected(SessionHelper session)
+    void connected(SessionHelper session)
         throws SessionNotExistException;
 
     /**
@@ -37,8 +35,7 @@ public interface SessionCallback
      *
      * @param session The disconnected session.
      */
-    void
-    disconnected(SessionHelper session);
+    void disconnected(SessionHelper session);
 
     /**
      * Notifies the application that the Glacier2 session establishment failed. 
@@ -47,6 +44,5 @@ public interface SessionCallback
      * failure.
      * @param ex The exception.
      */
-    void
-    connectFailed(SessionHelper session, Throwable ex);
+    void connectFailed(SessionHelper session, Throwable ex);
 }
diff --git a/java/src/Glacier2/SessionFactoryHelper.java b/java/src/Glacier2/SessionFactoryHelper.java
index 324e93e4046..37914ac5d00 100644
--- a/java/src/Glacier2/SessionFactoryHelper.java
+++ b/java/src/Glacier2/SessionFactoryHelper.java
@@ -26,7 +26,8 @@ public class SessionFactoryHelper
      * @throws {@link Ice.InitializationException}
      */
     public
-    SessionFactoryHelper(SessionCallback callback) throws Ice.InitializationException
+    SessionFactoryHelper(SessionCallback callback)
+        throws Ice.InitializationException
     {
         initialize(callback, new Ice.InitializationData(), Ice.Util.createProperties());
     }
@@ -39,7 +40,8 @@ public class SessionFactoryHelper
      * @throws {@link Ice.InitializationException}
      */
     public
-    SessionFactoryHelper(Ice.InitializationData initData, SessionCallback callback) throws Ice.InitializationException
+    SessionFactoryHelper(Ice.InitializationData initData, SessionCallback callback)
+        throws Ice.InitializationException
     {
         initialize(callback, initData, initData.properties);
     }
@@ -52,7 +54,8 @@ public class SessionFactoryHelper
      * @throws {@link Ice.InitializationException}
      */
     public
-    SessionFactoryHelper(Ice.Properties properties, SessionCallback callback) throws Ice.InitializationException
+    SessionFactoryHelper(Ice.Properties properties, SessionCallback callback)
+        throws Ice.InitializationException
     {
         initialize(callback, new Ice.InitializationData(), properties);
     }
@@ -63,8 +66,8 @@ public class SessionFactoryHelper
     {
         if(callback == null)
         {
-            throw new Ice.InitializationException("Attempt to create a SessionFactoryHelper with a null SessionCallback" +
-                                                  "argument");
+            throw new Ice.InitializationException("Attempt to create a SessionFactoryHelper with a null " +
+                                                  "SessionCallback argument");
         }
 
         if(initData == null)
@@ -250,7 +253,9 @@ public class SessionFactoryHelper
     private Ice.InitializationData
     createInitData()
     {
+        //
         // Clone the initialization data and properties.
+        //
         Ice.InitializationData initData = (Ice.InitializationData)_initData.clone();
         initData.properties = initData.properties._clone();
 
diff --git a/java/src/Glacier2/SessionHelper.java b/java/src/Glacier2/SessionHelper.java
index e0ca6dd2c9d..66b15a4ebd8 100644
--- a/java/src/Glacier2/SessionHelper.java
+++ b/java/src/Glacier2/SessionHelper.java
@@ -28,24 +28,35 @@ public class SessionHelper
         {
             while(true)
             {
-                _router.refreshSession_async(new Glacier2.AMI_Router_refreshSession()
+                try
                 {
-                    public void ice_response()
+                    _router.refreshSession_async(new Glacier2.AMI_Router_refreshSession()
                     {
-                    }
+                        public void ice_response()
+                        {
+                        }
 
-                    public void ice_exception(Ice.LocalException ex)
-                    {
-                        done();
-                        SessionHelper.this.destroy();
-                    }
+                        public void ice_exception(Ice.LocalException ex)
+                        {
+                            done();
+                            SessionHelper.this.destroy();
+                        }
+
+                        public void ice_exception(Ice.UserException ex)
+                        {
+                            done();
+                            SessionHelper.this.destroy();
+                        }
+                    });
+                }
+                catch(Ice.CommunicatorDestroyedException ex)
+                {
+                    //
+                    // AMI requests can raise CommunicatorDestroyedException directly.
+                    //
+                    break;
+                }
 
-                    public void ice_exception(Ice.UserException ex)
-                    {
-                        done();
-                        SessionHelper.this.destroy();
-                    }
-                });
                 if(!_done)
                 {
                     try
@@ -56,6 +67,7 @@ public class SessionHelper
                     {
                     }
                 }
+
                 if(_done)
                 {
                     break;
@@ -106,10 +118,12 @@ public class SessionHelper
         _destroy = true;
         if(_refreshThread == null)
         {
+            //
             // In this case a connecting session is being
             // destroyed. The communicator and session will be
             // destroyed when the connection establishment has
             // completed.
+            //
             return;
         }
         _session = null;
@@ -127,8 +141,10 @@ public class SessionHelper
             // Ignore
         }
 
+        //
         // Run the destroyInternal in a thread. This is because it
         // destroyInternal makes remote invocations.
+        //
         new Thread(new Runnable()
         {
             public void run()
@@ -225,7 +241,9 @@ public class SessionHelper
         return internalObjectAdapter();
     }
 
+    //
     // Only call this method when the calling thread owns the lock
+    //
     private Ice.ObjectAdapter
     internalObjectAdapter()
         throws SessionNotExistException
@@ -301,7 +319,9 @@ public class SessionHelper
             return;
         }
 
+        //
         // Assign the session after _destroy is checked.
+        //
         _session = session;
         _connected = true;
 
@@ -329,8 +349,9 @@ public class SessionHelper
         }
         catch(SecurityException ex)
         {
-            // Ignore. Unsigned applets cannot registered shutdown
-            // hooks.
+            //
+            // Ignore. Unsigned applets cannot registered shutdown hooks.
+            //
         }
 
         dispatchCallback(new Runnable()
@@ -360,15 +381,21 @@ public class SessionHelper
         }
         catch(Ice.ConnectionLostException e)
         {
+            //
             // Expected if another thread invoked on an object from the session concurrently.
+            //
         }
         catch(SessionNotExistException e)
         {
+            //
             // This can also occur.
+            //
         }
         catch(Throwable e)
         {
+            //
             // Not expected.
+            //
             _communicator.getLogger().warning("SessionHelper: unexpected exception when destroying the session:\n" + e);
         }
         _router = null;
@@ -400,7 +427,9 @@ public class SessionHelper
         }
         _communicator = null;
 
+        //
         // Notify the callback that the session is gone.
+        //
         dispatchCallback(new Runnable()
         {
             public void run()