Repository: ably/ably-flutter
Issue: Android: `java.lang.ArrayIndexOutOfBoundsException` thrown by `AblyInstanceStore`'s `setPaginatedResult` method

We've had a customer report that a small subset of their app users have experienced this crash.

Information provided via Firebase Crashlytics...

{code}
Fatal Exception: java.lang.ArrayIndexOutOfBoundsException
src.length=11 srcPos=0 dst.length=25 dstPos=0 length=12
java.lang.System.arraycopy (System.java)
com.android.internal.util.GrowingArrayUtils.insert (GrowingArrayUtils.java:141)
android.util.LongSparseArray.put (LongSparseArray.java:218)
io.ably.flutter.plugin.AblyInstanceStore.setPaginatedResult (AblyInstanceStore.java:105)
io.ably.flutter.plugin.AblyMethodCallHandler$3.onSuccess (AblyMethodCallHandler.java:280)
io.ably.flutter.plugin.AblyMethodCallHandler$3.onSuccess (AblyMethodCallHandler.java:277)
io.ably.lib.http.BasePaginatedQuery$ResultRequest$1.onSuccess (BasePaginatedQuery.java:215)
io.ably.lib.http.BasePaginatedQuery$ResultRequest$1.onSuccess (BasePaginatedQuery.java:212)
io.ably.lib.http.HttpScheduler$AsyncRequest.setResult (HttpScheduler.java:321)
io.ably.lib.http.HttpScheduler$AblyRequestWithFallback.run (HttpScheduler.java:206)
java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1167)
java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:641)
java.lang.Thread.run (Thread.java:919)
{code}
From the crash log for a Nokia 5.4 device running Android 10.

Other potentially salient details:

* 6 instances of this crash seen across 14,600 installations of the app
* *OS*: 50% running Android 9
* *Devices*: 33% Samsung, 33% Xiaomi, 17% Vivo, 17% HMD Global
* *Device states*: 50% background

Also see [this internal Ably Support Team Slack thread|https://ably-real-time.slack.com/archives/C8SPU4589/p1643257377074100].

Changed files:
android/src/main/java/io/ably/flutter/plugin/AblyInstanceStore.java, android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java

diff --git a/android/src/main/java/io/ably/flutter/plugin/AblyInstanceStore.java b/android/src/main/java/io/ably/flutter/plugin/AblyInstanceStore.java
index f0fd689c..d3040f3b 100644
--- a/android/src/main/java/io/ably/flutter/plugin/AblyInstanceStore.java
+++ b/android/src/main/java/io/ably/flutter/plugin/AblyInstanceStore.java
@@ -3,6 +3,8 @@ package io.ably.flutter.plugin;
 import android.content.Context;
 import android.util.LongSparseArray;
 
+import java.util.concurrent.atomic.AtomicLong;
+
 import io.ably.lib.push.Push;
 import io.ably.lib.push.PushChannel;
 import io.ably.lib.realtime.AblyRealtime;
@@ -23,9 +25,7 @@ import io.ably.lib.types.ClientOptions;
  * client by calling @{AblyInstanceStore#getRealtime(final long handle)}.
  */
 class AblyInstanceStore {
-
     private static final AblyInstanceStore instance = new AblyInstanceStore();
-    private long nextHandle = 1;
 
     // Android Studio warns against using HashMap with integer keys, and
     // suggests using LongSparseArray. More information at https://stackoverflow.com/a/31413003
@@ -34,38 +34,100 @@ class AblyInstanceStore {
     private final LongSparseArray<AblyRest> restInstances = new LongSparseArray<>();
     private final LongSparseArray<AblyRealtime> realtimeInstances = new LongSparseArray<>();
     private final LongSparseArray<AsyncPaginatedResult<Object>> paginatedResults = new LongSparseArray<>();
+    private final AtomicLong nextHandle = new AtomicLong(1);
 
     static synchronized AblyInstanceStore getInstance() {
         return instance;
     }
 
     /**
-     * Returns a handle representing the next client that will be created. This handle can be used
-     * to get the client **after** it is instantiated using [createRest] or [createRealtime].
+     * A reserved client handle. Safe to be used from any thread.
+     *
+     * Instances support the creation of a single Rest or Realtime instance, where only one of the
+     * create methods may be called and it may only be called once.
      */
-    long getHandleForNextClient() {
-        return nextHandle;
+    interface ClientHandle {
+        /**
+         * Get the handle that will be used to store this client when it is created, or the handle
+         * that was used to store it when it was created.
+         * This property may be read at any time, from any thread.
+         */
+        long getHandle();
+
+        /**
+         * Create an {@link AblyRest} instance and store it using this handle.
+         * @param clientOptions The Ably client options for the new Rest instance.
+         * @param applicationContext The Android application context to supply to the new Rest
+         * instance using its {@link AblyRest#setAndroidContext(Context)} method.
+         * @return The handle used to store the instance. Same as {@link #getHandle()}.
+         * @throws IllegalStateException If this handle has already been used to create a Rest or
+         * Realtime instance.
+         * @throws AblyException If the {@link AblyRest} instance creation failed.
+         */
+        long createRest(ClientOptions clientOptions, Context applicationContext) throws AblyException;
+
+        /**
+         * Create an {@link AblyRealtime} instance and store it using this handle.
+         * @param clientOptions The Ably client options for the new Realtime instance.
+         * @param applicationContext The Android application context to supply to the new Realtime
+         * instance using its {@link AblyRealtime#setAndroidContext(Context)} method.
+         * @return The handle used to store the instance. Same as {@link #getHandle()}.
+         * @throws IllegalStateException If this handle has already been used to create a Rest or
+         * Realtime instance.
+         * @throws AblyException If the {@link AblyRealtime} instance creation failed.
+         */
+        long createRealtime(ClientOptions clientOptions, Context applicationContext) throws AblyException;
     }
 
-    long createRest(final ClientOptions clientOptions, Context applicationContext) throws AblyException {
-        final AblyRest rest = new AblyRest(clientOptions);
-        rest.setAndroidContext(applicationContext);
-        restInstances.put(nextHandle, rest);
-        return nextHandle++;
+    private class ReservedClientHandle implements ClientHandle {
+        private final long handle;
+        private volatile boolean used = false;
+
+        ReservedClientHandle(final long handle) {
+            this.handle = handle;
+        }
+
+        @Override
+        public long getHandle() {
+            return handle;
+        }
+
+        @Override
+        public synchronized long createRest(final ClientOptions clientOptions, final Context applicationContext) throws AblyException {
+            final long handle = use();
+            final AblyRest rest = new AblyRest(clientOptions);
+            rest.setAndroidContext(applicationContext);
+            restInstances.put(handle, rest);
+            return handle;
+        }
+
+        @Override
+        public synchronized long createRealtime(final ClientOptions clientOptions, final Context applicationContext) throws AblyException {
+            final long handle = use();
+            final AblyRealtime realtime = new AblyRealtime(clientOptions);
+            realtime.setAndroidContext(applicationContext);
+            realtimeInstances.put(handle, realtime);
+            return handle;
+        }
+
+        synchronized long use() {
+            if (used) {
+                throw new IllegalStateException("Reserved handle has already been used to create a client instance (handle=" + handle + ").");
+            }
+            used = true;
+            return handle;
+        }
     }
 
-    AblyRest getRest(final long handle) {
-        return restInstances.get(handle);
+    synchronized ClientHandle reserveClientHandle() {
+        return new ReservedClientHandle(nextHandle.getAndIncrement());
     }
 
-    long createRealtime(final ClientOptions clientOptions, Context applicationContext) throws AblyException {
-        final AblyRealtime realtime = new AblyRealtime(clientOptions);
-        realtime.setAndroidContext(applicationContext);
-        realtimeInstances.put(nextHandle, realtime);
-        return nextHandle++;
+    synchronized AblyRest getRest(final long handle) {
+        return restInstances.get(handle);
     }
 
-    AblyRealtime getRealtime(final long handle) {
+    synchronized AblyRealtime getRealtime(final long handle) {
         return realtimeInstances.get(handle);
     }
 
@@ -79,26 +141,26 @@ class AblyInstanceStore {
      * @param handle integer handle to either AblyRealtime or AblyRest
      * @return AblyBase
      */
-    AblyBase getAblyClient(final long handle) {
+    synchronized AblyBase getAblyClient(final long handle) {
         AblyRealtime realtime = getRealtime(handle);
         return (realtime != null) ? realtime : getRest(handle);
     }
     
-    Push getPush(final long handle) {
+    synchronized Push getPush(final long handle) {
         AblyRealtime realtime = getRealtime(handle);
         return (realtime != null) ? realtime.push : getRest(handle).push;
     }
     
-    PushChannel getPushChannel(final long handle, final String channelName) {
+    synchronized PushChannel getPushChannel(final long handle, final String channelName) {
         return getAblyClient(handle)
                 .channels
                 .get(channelName).push;
     }
 
-    long setPaginatedResult(AsyncPaginatedResult result, Integer handle) {
+    synchronized long setPaginatedResult(AsyncPaginatedResult result, Integer handle) {
         long longHandle;
         if (handle == null) {
-            longHandle = nextHandle++;
+            longHandle = nextHandle.getAndIncrement();
         } else {
             longHandle = handle.longValue();
         }
@@ -106,11 +168,11 @@ class AblyInstanceStore {
         return longHandle;
     }
 
-    AsyncPaginatedResult<Object> getPaginatedResult(long handle) {
+    synchronized AsyncPaginatedResult<Object> getPaginatedResult(long handle) {
         return paginatedResults.get(handle);
     }
 
-    void reset() {
+    synchronized void reset() {
         for (int i = 0; i < realtimeInstances.size(); i++) {
             long key = realtimeInstances.keyAt(i);
             AblyRealtime r = realtimeInstances.get(key);
diff --git a/android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java b/android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java
index 4b3528a6..04bc3676 100644
--- a/android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java
+++ b/android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java
@@ -183,13 +183,13 @@ public class AblyMethodCallHandler implements MethodChannel.MethodCallHandler {
     final AblyFlutterMessage<PlatformClientOptions> message = (AblyFlutterMessage<PlatformClientOptions>) call.arguments;
     this.<PlatformClientOptions>ablyDo(message, (ablyLibrary, clientOptions) -> {
       try {
-        final long handle = ablyLibrary.getHandleForNextClient();
+        final AblyInstanceStore.ClientHandle clientHandle = ablyLibrary.reserveClientHandle();
         if (clientOptions.hasAuthCallback) {
           clientOptions.options.authCallback = (Auth.TokenParams params) -> {
             final Object[] token = {null};
             final CountDownLatch latch = new CountDownLatch(1);
             new Handler(Looper.getMainLooper()).post(() -> {
-              AblyFlutterMessage<Auth.TokenParams> channelMessage = new AblyFlutterMessage<>(params, handle);
+              AblyFlutterMessage<Auth.TokenParams> channelMessage = new AblyFlutterMessage<>(params, clientHandle.getHandle());
               methodChannel.invokeMethod(PlatformConstants.PlatformMethod.authCallback, channelMessage, new MethodChannel.Result() {
                 @Override
                 public void success(@Nullable Object result) {
@@ -220,7 +220,7 @@ public class AblyMethodCallHandler implements MethodChannel.MethodCallHandler {
             return token[0];
           };
         }
-        result.success(ablyLibrary.createRest(clientOptions.options, applicationContext));
+        result.success(clientHandle.createRest(clientOptions.options, applicationContext));
       } catch (final AblyException e) {
         handleAblyException(result, e);
       }
@@ -448,13 +448,13 @@ public class AblyMethodCallHandler implements MethodChannel.MethodCallHandler {
     final AblyFlutterMessage<PlatformClientOptions> message = (AblyFlutterMessage<PlatformClientOptions>) call.arguments;
     this.<PlatformClientOptions>ablyDo(message, (ablyLibrary, clientOptions) -> {
       try {
-        final long handle = ablyLibrary.getHandleForNextClient();
+        final AblyInstanceStore.ClientHandle clientHandle = ablyLibrary.reserveClientHandle();
         if (clientOptions.hasAuthCallback) {
           clientOptions.options.authCallback = (Auth.TokenParams params) -> {
             final Object[] token = {null};
             final CountDownLatch latch = new CountDownLatch(1);
             new Handler(Looper.getMainLooper()).post(() -> {
-              AblyFlutterMessage<Auth.TokenParams> channelMessage = new AblyFlutterMessage<>(params, handle);
+              AblyFlutterMessage<Auth.TokenParams> channelMessage = new AblyFlutterMessage<>(params, clientHandle.getHandle());
               methodChannel.invokeMethod(PlatformConstants.PlatformMethod.realtimeAuthCallback, channelMessage, new MethodChannel.Result() {
                 @Override
                 public void success(@Nullable Object result) {
@@ -487,7 +487,7 @@ public class AblyMethodCallHandler implements MethodChannel.MethodCallHandler {
             return token[0];
           };
         }
-        result.success(ablyLibrary.createRealtime(clientOptions.options, applicationContext));
+        result.success(clientHandle.createRealtime(clientOptions.options, applicationContext));
       } catch (final AblyException e) {
         handleAblyException(result, e);
       }