Repository: ably/ably-java
Issue: RTN23a: Transport not disconnecting after TTL passed

Per RTN23a, a transport should be disconnected after receiving no activity for `maxIdleInterval` + `realtimeRequestTimeout`. This is implemented in `checkActivity()`: https://github.com/ably/ably-java/blob/deec71a40affe522c59160b9278334768b928d90/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java#L326

However, the timer that is set cannot do anything because it calls `checkActivity` again, which checks if a timer is set, which it is, so it does nothing (and starts no future timers).

This means on a network drop event, the library will wait over a minute to begin the process of trying fallback hosts etc.

Changed files:
lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java, lib/src/test/java/io/ably/lib/test/realtime/ConnectionManagerTest.java

diff --git a/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java b/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java
index 85284b17..62a1d4b9 100644
--- a/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java
+++ b/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java
@@ -306,36 +306,34 @@ public class WebSocketTransport implements ITransport {
         }
 
         private synchronized void checkActivity() {
-            long timeout = connectionManager.maxIdleInterval;
+            long timeout = getActivityTimeout();
             if (timeout == 0) {
                 Log.v(TAG, "checkActivity: infinite timeout");
                 return;
             }
-            if(activityTimerTask != null) {
-                /* timer already running */
+
+            // Check if timer already running
+            if (activityTimerTask != null) {
                 return;
             }
-            timeout += connectionManager.ably.options.realtimeRequestTimeout;
-            long now = System.currentTimeMillis();
-            long next = lastActivityTime + timeout;
-            if (now < next) {
-                /* We have not reached maxIdleInterval+realtimeRequestTimeout
-                 * of inactivity.  Schedule a new timer for that long after the
-                 * last activity time. */
-                Log.v(TAG, "checkActivity: ok");
+
+            // Start the activity timer task
+            startActivityTimer(timeout + 100);
+        }
+
+
+        private synchronized void startActivityTimer(long timeout)
+        {
+            if (activityTimerTask == null) {
                 schedule((activityTimerTask = new TimerTask() {
                     public void run() {
                         try {
-                            checkActivity();
+                            onActivityTimerExpiry();
                         } catch(Throwable t) {
                             Log.e(TAG, "Unexpected exception in activity timer handler", t);
                         }
                     }
-                }), next - now);
-            } else {
-                /* Timeout has been reached. Close the connection. */
-                Log.e(TAG, "No activity for " + timeout + "ms, closing connection");
-                closeConnection(CloseFrame.ABNORMAL_CLOSE, "timed out");
+                }), timeout);
             }
         }
 
@@ -349,6 +347,29 @@ public class WebSocketTransport implements ITransport {
             }
         }
 
+        private synchronized void onActivityTimerExpiry()
+        {
+            activityTimerTask = null;
+            long timeSinceLastActivity = System.currentTimeMillis() - lastActivityTime;
+            long timeRemaining = getActivityTimeout() - timeSinceLastActivity;
+
+            // If we have no time remaining, then close the connection
+            if (timeRemaining <= 0) {
+                Log.e(TAG, "No activity for " + getActivityTimeout() + "ms, closing connection");
+                closeConnection(CloseFrame.ABNORMAL_CLOSE, "timed out");
+                return;
+            }
+
+            // Otherwise, we've had some activity, restart the timer for the next timeout
+            Log.v(TAG, "onActivityTimerExpiry: ok");
+            startActivityTimer(timeRemaining + 100);
+        }
+
+        private long getActivityTimeout()
+        {
+            return connectionManager.maxIdleInterval + connectionManager.ably.options.realtimeRequestTimeout;
+        }
+
         /***************************
          * WsClient private members
          ***************************/
diff --git a/lib/src/test/java/io/ably/lib/test/realtime/ConnectionManagerTest.java b/lib/src/test/java/io/ably/lib/test/realtime/ConnectionManagerTest.java
index 2182f537..e7f7c133 100644
--- a/lib/src/test/java/io/ably/lib/test/realtime/ConnectionManagerTest.java
+++ b/lib/src/test/java/io/ably/lib/test/realtime/ConnectionManagerTest.java
@@ -553,6 +553,35 @@ public class ConnectionManagerTest extends ParameterizedTest {
         }
     }
 
+    /**
+     * RTN23
+     */
+    @Test
+    public void connection_is_closed_after_max_idle_interval() throws AblyException {
+        ClientOptions opts = createOptions(testVars.keys[0].keyStr);
+        opts.realtimeRequestTimeout = 2000;
+        try(AblyRealtime ably = new AblyRealtime(opts)) {
+            final long newIdleInterval = 500L;
+
+            // When we connect, we set the max idle interval to be very small
+            ably.connection.on(ConnectionEvent.connected, state -> {
+                try {
+                    Field maxIdleField = ably.connection.connectionManager.getClass().getDeclaredField("maxIdleInterval");
+                    maxIdleField.setAccessible(true);
+                    maxIdleField.setLong(ably.connection.connectionManager, newIdleInterval);
+                } catch (NoSuchFieldException | IllegalAccessException e) {
+                    fail("Unexpected exception in checking connectionStateTtl");
+                }
+            });
+
+            // The original max idle interval we receive from the server is 15s.
+            // We should wait for this, plus a tiny bit extra (as we set the new idle interval to be very low
+            // after connecting) to make sure that the connection is disconnected
+            ConnectionWaiter connectionWaiter = new ConnectionWaiter(ably.connection);
+            assertTrue(connectionWaiter.waitFor(ConnectionState.disconnected, 1, 25000));
+        }
+    }
+
     /**
      * RTN15g1, RTN15g2. Connect, disconnect, reconnect after (ttl + idle interval) period has passed,
      * check that the connection is a new one;
@@ -683,13 +712,13 @@ public class ConnectionManagerTest extends ParameterizedTest {
             });
             final Channel suspendedChannel = ably.channels.get("test-reattach-suspended-after-ttl" + testParams.name);
             suspendedChannel.state = ChannelState.suspended;
-            ChannelWaiter suspendedChannelWaiter = new Helpers.ChannelWaiter(suspendedChannel);
             suspendedChannel.on(new ChannelStateListener() {
                 @Override
                 public void onChannelStateChanged(ChannelStateChange stateChange) {
                     suspendedChannelHistory.add(stateChange.current.name());
                 }
             });
+            ChannelWaiter suspendedChannelWaiter = new Helpers.ChannelWaiter(suspendedChannel);
 
             /* attach first channel and wait for it to be attached */
             attachedChannel.attach();