Repository navigation
fix: keep AndroidKeyStore access off the main thread during initialize - #52
Open
vargajacint wants to merge 2 commits into
Open
vargajacint wants to merge 2 commits into
vargajacint wants to merge 2 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Gleap.initializecan stall the main thread on AndroidKeyStore binder calls and cause ANRs shortly after launch. This is most visible when the host callsinitializeon the UI thread (the React Native bridge does, insiderunOnUiThread) on low-end devices (Android 13-15).Root cause
GleapSessionController.initializeconstructs the controller synchronously, and the construction does all the keystore work on the calling thread:GleapSessionController(Application)callsGleapPreferencesHelper.getInstance(GleapSessionController.java:30), whose constructor callskeystoreCipher()->getOrCreateKey()(GleapPreferencesHelper.java:102,:179,:190). On first launch this isKeyGenerator.generateKey(), a keystore2generateKeybinder call into the TEE.GleapSessionController.java:37-44): about 14getStringcalls, each aValueCipher.decrypt(GleapPreferencesHelper.java:66) that does a keystore2createOperationbinder call.Each call is a blocking IPC into the secure hardware, so the total can exceed the ANR threshold on slow devices.
Fix
GleapPreferencesHelper: resolve the keystore cipher lazily on first read or write instead of in the constructor, sogetInstanceno longer touches the keystore.GleapSessionController: do not read the stored session and user in the constructor. They are loaded byensureStateLoaded()on first access (every accessor of the session or the user goes through it), andinitializewarms it up onGleapExecutor.SERIAL, so the keystore work happens on the SDK's own thread. The load is guarded by its own lock (stateLoadLock), not the controller's monitor, so the unrelated methods that lock the controller (for examplesetPendingIdentificationAction, called by everyidentifyContact) never wait for it.No storage format change, no new public API.
Backward compatibility
gleap_prefs_keyalias, Base64(IV + ciphertext)), so everything already on users' devices is read as before, and the plain-text fallback is unchanged.mergeUserSession,setGleapUserSession,clearUserSession) load first so they cannot be overwritten by a late load.getIdentity()andclearIdentity().isUserIdentified()does not wait for the load but still reads the stored user on the calling thread, as before.initialize,identifyContact/identifyUser,updateContactand the widget open calls do not wait on it.initialize,identify,clearIdentity,getIdentityandisUserIdentifiedon the UI thread, which is why the load does not hold the controller lock.Testing
./gradlew :gleap:testDebugUnitTest: 216 tests, 0 failures, including two newGleapSessionControllerTestcases: the constructor does not read the store (the stored session is still returned on first use), and queueing an identify returns promptly while a load is blocked in the store../gradlew :gleap:assembleRelease: builds.ANR stack traces (main thread, SDK 19.2.2)
Key generation on first launch (Android 16, thread state: waiting for a binder transaction):
Key generation (Android 14, thread state: waiting for a binder transaction):
Decrypting stored values during controller construction (Android 13, low-end device, thread state: waiting for a binder transaction):