fix: keep AndroidKeyStore access off the main thread during initialize

gleap/src/main/java/io/gleap/GleapPreferencesHelper.java

@@ -93,18 +93,29 @@ public byte[] decode(String text) {
     };
 
     private final SharedPreferences prefs;
-    // null: no usable key, values are stored as plain text.
-    private final ValueCipher cipher;
+    // Loaded on first use: the key lookup is a keystore call, which must not run when the SDK
+    // is initialized on the main thread. Null once resolved: no usable key, values are plain text.
+    private ValueCipher cipher;
+    private boolean cipherResolved;
 
     private static GleapPreferencesHelper instance;
 
     private GleapPreferencesHelper(Context context) {
-        this(context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE), keystoreCipher());
+        this.prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE);
     }
 
     GleapPreferencesHelper(SharedPreferences prefs, ValueCipher cipher) {
         this.prefs = prefs;
         this.cipher = cipher;
+        this.cipherResolved = true;
+    }
+
+    private synchronized ValueCipher cipher() {
+        if (!cipherResolved) {
+            cipher = keystoreCipher();
+            cipherResolved = true;
+        }
+        return cipher;
     }
 
     static synchronized GleapPreferencesHelper getInstance(Context context) {
@@ -120,6 +131,7 @@ public void putString(String key, String value) {
             prefs.edit().remove(key).apply();
             return;
         }
+        ValueCipher cipher = cipher();
         if (cipher != null) {
             try {
                 prefs.edit().putString(key, cipher.encrypt(value)).apply();
@@ -137,6 +149,7 @@ public String getString(String key, String defaultValue) {
         if (stored == null) {
             return defaultValue;
         }
+        ValueCipher cipher = cipher();
         if (cipher != null) {
             try {
                 return cipher.decrypt(stored);

gleap/src/main/java/io/gleap/GleapSessionController.java

@@ -11,6 +11,11 @@ public class GleapSessionController {
     private static volatile GleapSessionController instance;
     private GleapSessionProperties gleapSessionProperties;
     private volatile GleapSession gleapSession;
+    // The stored session and user are read on first use (see ensureStateLoaded).
+    private volatile boolean stateLoaded = false;
+    // Only guards the load. Not `this`: the load is slow, and the many methods that lock the
+    // controller (e.g. setPendingIdentificationAction) must not wait for it.
+    private final Object stateLoadLock = new Object();
     private GleapSessionProperties pendingIdentificationAction;
     private GleapSessionProperties pendingUpdateAction;
     private volatile boolean isSessionLoaded = false;
@@ -32,22 +37,45 @@ private GleapSessionController(Application application) {
 
     GleapSessionController(KeyValueStore store) {
         this.store = store;
+    }
 
-        // Load existing session.
-        String id = store.getString("session_id", "");
-        String hash = store.getString("session_hash", "");
-        if (!id.equals("") && !hash.equals("")) {
-            gleapSession = new GleapSession(id, hash);
+    /**
+     * Loads the stored session and user. Reading them decrypts every value with the keystore key
+     * (a binder call each), so the constructor, which runs on the app's main thread, leaves it
+     * to the first caller and {@link #initialize} warms it up on the SDK's own thread. Every method
+     * that writes the session or the user calls this first, so a load never overwrites a write.
+     * The load takes only {@code stateLoadLock}, never the controller's monitor, so it can be
+     * awaited while holding it.
+     */
+    private void ensureStateLoaded() {
+        if (stateLoaded) {
+            return;
+        }
+        synchronized (stateLoadLock) {
+            if (stateLoaded) {
+                return;
+            }
+            String id = store.getString("session_id", "");
+            String hash = store.getString("session_hash", "");
+            if (!id.equals("") && !hash.equals("")) {
+                gleapSession = new GleapSession(id, hash);
+            }
+            this.gleapSessionProperties = getStoredGleapUser();
+            stateLoaded = true;
         }
-
-        // Load existing session properties.
-        this.gleapSessionProperties = getStoredGleapUser();
     }
 
     public static GleapSessionController initialize(Application application) {
         synchronized (GleapSessionController.class) {
             if (instance == null) {
                 instance = new GleapSessionController(application);
+                final GleapSessionController controller = instance;
+                GleapExecutor.SERIAL.execute(new Runnable() {
+                    @Override
+                    public void run() {
+                        controller.ensureStateLoaded();
+                    }
+                });
             }
             return instance;
         }
@@ -136,6 +164,7 @@ boolean hasIdentifyHash() {
      * project requires a file session and the current session has no valid one.
      */
     boolean needsFileAccessIdentify(GleapSessionProperties identify) {
+        ensureStateLoaded();
         GleapSession session = gleapSession;
         return identify != null && identify.getHash() != null && !identify.getHash().isEmpty()
                 && session != null && session.isAuthenticatedFilesRequired()
@@ -189,6 +218,7 @@ boolean isCurrentGeneration(int generation) {
     }
 
     public void clearUserSession() {
+        ensureStateLoaded();
         synchronized (this) {
             identityGeneration.incrementAndGet();
             // Revoke the file session before the session that holds it is dropped.
@@ -244,6 +274,7 @@ void keepIdentifyPending(GleapSessionProperties identify, int generation, boolea
     }
 
     public void mergeUserSession(String id, String hash) {
+        ensureStateLoaded();
         if (gleapSession == null) {
             gleapSession = new GleapSession(id, hash);
         } else {
@@ -255,10 +286,12 @@ public void mergeUserSession(String id, String hash) {
     }
 
     public GleapSession getUserSession() {
+        ensureStateLoaded();
         return gleapSession;
     }
 
     public void setGleapUserSession(GleapSessionProperties gleapUser) {
+        ensureStateLoaded();
         this.gleapSessionProperties = gleapUser;
 
         if (gleapUser == null) {
@@ -395,6 +428,7 @@ void processSessionActionResult(JSONObject result, boolean restartEventServices,
                 // registerPushMessageGroup(hash) below would no-op when the previous
                 // value happens to be cached here.
                 GleapSession session;
+                ensureStateLoaded();
                 synchronized (this) {
                     if (!isCurrentGeneration(generation)) {
                         GleapLog.i("Dropped a session answer from before the logout");
@@ -477,6 +511,7 @@ private static boolean sameValue(String a, String b) {
     }
 
     public GleapSessionProperties getGleapUserSession() {
+        ensureStateLoaded();
         return gleapSessionProperties;
     }
 

gleap/src/test/java/io/gleap/GleapSessionControllerTest.java

@@ -15,6 +15,8 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import io.gleap.callbacks.GetActivityCallback;
 import io.gleap.callbacks.InitializationDoneCallback;
@@ -190,4 +192,76 @@ public void theIdentifiedUserSurvivesARestart() throws Exception {
         assertEquals(3, restored.getSla(), 0);
         assertEquals("gold", restored.getCustomData().getString("tier"));
     }
+
+    @Test
+    public void storedSessionIsNotReadWhenTheControllerIsCreated() {
+        final int[] reads = {0};
+        InMemoryKeyValueStore counting = new InMemoryKeyValueStore() {
+            @Override
+            public String getString(String key, String defaultValue) {
+                reads[0]++;
+                return super.getString(key, defaultValue);
+            }
+        };
+        counting.values.put("session_id", "stored-id");
+        counting.values.put("session_hash", "stored-hash");
+
+        GleapSessionController created = new GleapSessionController(counting);
+
+        assertEquals(0, reads[0]);
+        assertEquals("stored-id", created.getUserSession().getId());
+        assertEquals("stored-hash", created.getUserSession().getHash());
+    }
+
+    @Test
+    public void queueingAnIdentifyDoesNotWaitForTheStoredStateToLoad() throws Exception {
+        final CountDownLatch loadStarted = new CountDownLatch(1);
+        final CountDownLatch releaseLoad = new CountDownLatch(1);
+        InMemoryKeyValueStore slow = new InMemoryKeyValueStore() {
+            @Override
+            public String getString(String key, String defaultValue) {
+                loadStarted.countDown();
+                try {
+                    releaseLoad.await(10, TimeUnit.SECONDS);
+                } catch (InterruptedException ignored) {
+                }
+                return super.getString(key, defaultValue);
+            }
+        };
+        slow.values.put("session_id", "stored-id");
+        slow.values.put("session_hash", "stored-hash");
+        final GleapSessionController slowController = new GleapSessionController(slow);
+
+        Thread loader = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                slowController.getUserSession();
+            }
+        });
+        loader.start();
+        try {
+            assertTrue(loadStarted.await(5, TimeUnit.SECONDS));
+
+            final CountDownLatch queued = new CountDownLatch(1);
+            final GleapSessionProperties identify = new GleapSessionProperties();
+            identify.setUserId("user-1");
+            Thread caller = new Thread(new Runnable() {
+                @Override
+                public void run() {
+                    slowController.setPendingIdentificationAction(identify);
+                    slowController.getPendingIdentificationAction();
+                    slowController.isIdentifyInFlight();
+                    queued.countDown();
+                }
+            });
+            caller.start();
+
+            assertTrue("queueing an identify waited for the load", queued.await(2, TimeUnit.SECONDS));
+            assertEquals(identify, slowController.getPendingIdentificationAction());
+        } finally {
+            releaseLoad.countDown();
+            loader.join(5000);
+        }
+        assertEquals("stored-id", slowController.getUserSession().getId());
+    }
 }