ICE-6285 - Glacier2 session helper bug

ICE-6167 - ACM improvements for Glacier2 helpers

3 files changed, 65 insertions, 107 deletions

diff --git a/java/src/Glacier2/src/main/java/Glacier2/Application.java b/java/src/Glacier2/src/main/java/Glacier2/Application.java
index 8a1e2b62d9f..90a4ff21c30 100644
--- a/java/src/Glacier2/src/main/java/Glacier2/Application.java
+++ b/java/src/Glacier2/src/main/java/Glacier2/Application.java
@@ -291,7 +291,6 @@ public abstract class Application extends Ice.Application
         boolean restart = false;
         status.value = 0;
 
-
         try
         {
             _communicator = Ice.Util.initialize(argHolder, initData);
@@ -336,6 +335,10 @@ public abstract class Application extends Ice.Application
                     catch(Ice.OperationNotExistException ex)
                     {
                     }
+                    if(acmTimeout <= 0)
+                    {
+                        acmTimeout = (int)_router.getSessionTimeout();
+                    }
                     if(acmTimeout > 0)
                     {
                         Ice.Connection connection = _router.ice_getCachedConnection();
@@ -345,61 +348,6 @@ public abstract class Application extends Ice.Application
                                           new Ice.Optional<Ice.ACMHeartbeat>(Ice.ACMHeartbeat.HeartbeatAlways));
                         connection.setCallback(new ConnectionCallbackI());
                     }
-                    else
-                    {
-                        long timeout = _router.getSessionTimeout();
-                        if(timeout > 0)
-                        {
-                            java.util.concurrent.ScheduledExecutorService timer =
-                                IceInternal.Util.getInstance(_communicator).timer();
-                            //
-                            // We don't need to cancel the task as the communicator is destroyed at the end and
-                            // ContinueExistingPeriodicTasksAfterShutdownPolicy is false.
-                            //
-                            timer.scheduleAtFixedRate(new Runnable()
-                                {
-                                    @Override
-                                    public void run()
-                                    {
-                                        _router.begin_refreshSession(new Glacier2.Callback_Router_refreshSession()
-                                            {
-                                                @Override
-                                                public void
-                                                response()
-                                                {
-                                                }
-
-                                                @Override
-                                                public void
-                                                exception(Ice.LocalException ex)
-                                                {
-                                                    //
-                                                    // Here the session has gone and we notify the application that
-                                                    // the session has been destroyed.
-                                                    //
-                                                    sessionDestroyed();
-                                                }
-
-                                                @Override
-                                                public void
-                                                exception(Ice.UserException ex)
-                                                {
-                                                    //
-                                                    // Here the session has gone and we notify the application that
-                                                    // the session has been destroyed.
-                                                    //
-                                                    sessionDestroyed();
-                                                }
-                                            });
-                                        //
-                                        // AMI requests can raise CommunicatorDestroyedException directly. We let this
-                                        // out of the task and terminate the timer.
-                                        //
-
-                                    }
-                                }, timeout / 2, timeout / 2, java.util.concurrent.TimeUnit.SECONDS);
-                        }
-                    }
                     _category = _router.getCategoryForClient();
                     status.value = runWithSession(argHolder.value);
                 }
diff --git a/java/src/Glacier2/src/main/java/Glacier2/SessionFactoryHelper.java b/java/src/Glacier2/src/main/java/Glacier2/SessionFactoryHelper.java
index eeb90412060..0899bb786cb 100644
--- a/java/src/Glacier2/src/main/java/Glacier2/SessionFactoryHelper.java
+++ b/java/src/Glacier2/src/main/java/Glacier2/SessionFactoryHelper.java
@@ -272,6 +272,30 @@ public class SessionFactoryHelper
     }
 
     /**
+     * Determines whether the session should create an object adapter that the client
+     * can use for receiving callbacks.
+     *
+     * @param useCallbacks True if the session should create an object adapter.
+     */
+    synchronized public void
+    setUseCallbacks(boolean useCallbacks)
+    {
+        _useCallbacks = useCallbacks;
+    }
+
+    /**
+     * Indicates whether a newly-created session will also create an object adapter that
+     * the client can use for receiving callbacks.
+     *
+     * @return True if the session will create an object adapter.
+     */
+    synchronized public boolean
+    getUseCallbacks()
+    {
+        return _useCallbacks;
+    }
+
+    /**
      * Connects to the Glacier2 router using the associated SSL credentials.
      *
      * Once the connection is established, {@link SessionCallback#connected} is called on the callback object;
@@ -282,7 +306,7 @@ public class SessionFactoryHelper
     synchronized public SessionHelper
     connect()
     {
-        SessionHelper session = new SessionHelper(_callback, createInitData(), getRouterFinderStr());
+        SessionHelper session = new SessionHelper(_callback, createInitData(), getRouterFinderStr(), _useCallbacks);
         session.connect(_context);
         return session;
     }
@@ -300,7 +324,7 @@ public class SessionFactoryHelper
     synchronized public SessionHelper
     connect(final String username, final String password)
     {
-        SessionHelper session = new SessionHelper(_callback, createInitData(), getRouterFinderStr());
+        SessionHelper session = new SessionHelper(_callback, createInitData(), getRouterFinderStr(), _useCallbacks);
         session.connect(username, password, _context);
         return session;
     }
@@ -368,6 +392,7 @@ public class SessionFactoryHelper
     private int _port = 0;
     private int _timeout = 10000;
     private java.util.Map<String, String> _context;
+    private boolean _useCallbacks = true;
     private static final int GLACIER2_SSL_PORT = 4064;
     private static final int GLACIER2_TCP_PORT = 4063;
 }
diff --git a/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java b/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java
index f39223c3e10..3e3311bacb7 100644
--- a/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java
+++ b/java/src/Glacier2/src/main/java/Glacier2/SessionHelper.java
@@ -19,12 +19,16 @@ public class SessionHelper
      *
      * @param callback The callback for notifications about session establishment.
      * @param initData The {@link Ice.InitializationData} for initializing the communicator.
+     * @param finderStr The stringified Ice.RouterFinder proxy.
+     * @param useCallbacks True if the session should create an object adapter for receiving callbacks.
      */
-    public SessionHelper(SessionCallback callback, Ice.InitializationData initData, String finderStr)
+    public SessionHelper(SessionCallback callback, Ice.InitializationData initData, String finderStr,
+                         boolean useCallbacks)
     {
         _callback = callback;
         _initData = initData;
         _finderStr = finderStr;
+        _useCallbacks = useCallbacks;
     }
 
     /**
@@ -71,8 +75,7 @@ public class SessionHelper
         }
 
         //
-        // Run the destroyInternal in a thread. This is because it
-        // destroyInternal makes remote invocations.
+        // Run destroyInternal in a thread because it makes remote invocations.
         //
         new Thread(new Runnable()
         {
@@ -182,10 +185,10 @@ public class SessionHelper
         {
             throw new SessionNotExistException();
         }
-        if(_adapter == null)
+        if(!_useCallbacks)
         {
-            _adapter = _communicator.createObjectAdapterWithRouter("", _router);
-            _adapter.activate();
+            throw new Ice.InitializationException(
+                "Object adapter not available, call SessionFactoryHelper.setUseCallbacks(true)");
         }
         return _adapter;
     }
@@ -246,8 +249,12 @@ public class SessionHelper
     private void
     connected(RouterPrx router, SessionPrx session)
     {
+        //
+        // Remote invocation should be done without acquiring a mutex lock.
+        //
+        assert(router != null);
+        Ice.Connection conn = router.ice_getCachedConnection();
         String category = router.getCategoryForClient();
-        long sessionTimeout = router.getSessionTimeout();
         int acmTimeout = 0;
         try
         {
@@ -256,7 +263,24 @@ public class SessionHelper
         catch(Ice.OperationNotExistException ex)
         {
         }
-        Ice.Connection conn = router.ice_getCachedConnection();
+
+        if(acmTimeout <= 0)
+        {
+            acmTimeout = (int)router.getSessionTimeout();
+        }
+
+        //
+        // We create the callback object adapter here because createObjectAdapter internally
+        // makes synchronous RPCs to the router. We can't create the OA on-demand when the
+        // client calls objectAdapter() or addWithUUID() because they can be called from the
+        // GUI thread.
+        //
+        if(_useCallbacks)
+        {
+            assert(_adapter == null);
+            _adapter = _communicator.createObjectAdapterWithRouter("", router);
+            _adapter.activate();
+        }
 
         synchronized(this)
         {
@@ -265,8 +289,7 @@ public class SessionHelper
             if(_destroy)
             {
                 //
-                // Run the destroyInternal in a thread. This is because it
-                // destroyInternal makes remote invocations.
+                // Run destroyInternal in a thread because it makes remote invocations.
                 //
                 new Thread(new Runnable()
                 {
@@ -311,45 +334,6 @@ public class SessionHelper
                                            }
                                        });
             }
-            else if(sessionTimeout > 0)
-            {
-                java.util.concurrent.ScheduledExecutorService timer =
-                    IceInternal.Util.getInstance(_communicator).timer();
-                //
-                // We don't need to cancel the task as the communicator is destroyed at the end and
-                // ContinueExistingPeriodicTasksAfterShutdownPolicy is false.
-                //
-                timer.scheduleAtFixedRate(new Runnable()
-                    {
-                        @Override
-                        public void run()
-                        {
-                            _router.begin_refreshSession(new Glacier2.Callback_Router_refreshSession()
-                            {
-                                @Override
-                                public void response()
-                                {
-                                }
-
-                                @Override
-                                public void exception(Ice.LocalException ex)
-                                {
-                                    SessionHelper.this.destroy();
-                                }
-
-                                @Override
-                                public void exception(Ice.UserException ex)
-                                {
-                                    SessionHelper.this.destroy();
-                                }
-                            });
-                            //
-                            // AMI requests can raise CommunicatorDestroyedException directly. We let this
-                            // out of the task and terminate the timer.
-                            //
-                        }
-                    }, sessionTimeout / 2, sessionTimeout / 2, java.util.concurrent.TimeUnit.SECONDS);
-            }
 
             _shutdownHook = new Thread("Shutdown hook")
             {
@@ -601,6 +585,7 @@ public class SessionHelper
     private Glacier2.SessionPrx _session;
     private String _category;
     private String _finderStr;
+    private boolean _useCallbacks;
 
     private final SessionCallback _callback;
     private boolean _destroy = false;