From 883c159960f645d51e7fd7a8afc1480f072b1c4d Mon Sep 17 00:00:00 2001 From: Jacint Varga Date: Tue, 13 Oct 2026 14:53:23 +0200 Subject: [PATCH 1/2] fix: keep AndroidKeyStore access off the main thread during initialize --- .../java/io/gleap/GleapPreferencesHelper.java | 19 ++++++-- .../java/io/gleap/GleapSessionController.java | 45 +++++++++++++++---- .../io/gleap/GleapSessionControllerTest.java | 20 +++++++++ 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/gleap/src/main/java/io/gleap/GleapPreferencesHelper.java b/gleap/src/main/java/io/gleap/GleapPreferencesHelper.java index 5f795d88..249d6610 100644 --- a/gleap/src/main/java/io/gleap/GleapPreferencesHelper.java +++ b/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); diff --git a/gleap/src/main/java/io/gleap/GleapSessionController.java b/gleap/src/main/java/io/gleap/GleapSessionController.java index 9ac88220..4198987c 100644 --- a/gleap/src/main/java/io/gleap/GleapSessionController.java +++ b/gleap/src/main/java/io/gleap/GleapSessionController.java @@ -11,6 +11,8 @@ 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; private GleapSessionProperties pendingIdentificationAction; private GleapSessionProperties pendingUpdateAction; private volatile boolean isSessionLoaded = false; @@ -32,22 +34,42 @@ 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. + */ + private void ensureStateLoaded() { + if (stateLoaded) { + return; + } + synchronized (this) { + 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 +158,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 +212,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 +268,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 +280,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 +422,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 +505,7 @@ private static boolean sameValue(String a, String b) { } public GleapSessionProperties getGleapUserSession() { + ensureStateLoaded(); return gleapSessionProperties; } diff --git a/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java b/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java index 6a446d2f..3c1f7d8e 100644 --- a/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java +++ b/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java @@ -190,4 +190,24 @@ 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()); + } } From c20874f96f0d5a0c76b62418fa9b329d8a0db4be Mon Sep 17 00:00:00 2001 From: Jacint Varga Date: Tue, 13 Oct 2026 14:59:11 +0200 Subject: [PATCH 2/2] fix: do not hold the controller lock while loading stored state The load decrypts about 14 stored values with the keystore key. It held the controller's monitor, so unrelated calls that lock the controller, such as queueing an identify, waited for the whole load. The load now has its own lock. --- .../java/io/gleap/GleapSessionController.java | 10 +++- .../io/gleap/GleapSessionControllerTest.java | 54 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/gleap/src/main/java/io/gleap/GleapSessionController.java b/gleap/src/main/java/io/gleap/GleapSessionController.java index 4198987c..2e207e27 100644 --- a/gleap/src/main/java/io/gleap/GleapSessionController.java +++ b/gleap/src/main/java/io/gleap/GleapSessionController.java @@ -13,6 +13,9 @@ public class GleapSessionController { 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; @@ -39,13 +42,16 @@ private GleapSessionController(Application application) { /** * 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. + * 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 (this) { + synchronized (stateLoadLock) { if (stateLoaded) { return; } diff --git a/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java b/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java index 3c1f7d8e..628a2093 100644 --- a/gleap/src/test/java/io/gleap/GleapSessionControllerTest.java +++ b/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; @@ -210,4 +212,56 @@ public String getString(String key, String defaultValue) { 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()); + } }