launchdarkly / android-client-sdk

LaunchDarkly Client-side SDK for Android
Other
45 stars 23 forks source link

Fatal Exception: java.lang.SecurityException on Samsung devices #83

Closed paulfthomas closed 4 years ago

paulfthomas commented 5 years ago

Describe the bug Crash only impacting Samsung devices, probably related to the way the PendingIntent is retrieved in the PollingUpdater. See https://github.com/launchdarkly/android-client-sdk/blob/master/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/android/PollingUpdater.java#L56

Logs

Fatal Exception: java.lang.SecurityException: !@Too many alarms (500) registered from uid 10085
       at android.os.Parcel.createException + 1966(Parcel.java:1966)
       at android.os.Parcel.readException + 1934(Parcel.java:1934)
       at android.os.Parcel.readException + 1884(Parcel.java:1884)
       at android.app.IAlarmManager$Stub$Proxy.set + 240(IAlarmManager.java:240)
       at android.app.AlarmManager.setImpl + 722(AlarmManager.java:722)
       at android.app.AlarmManager.setInexactRepeating + 823(AlarmManager.java:823)
       at com.launchdarkly.android.PollingUpdater.startPolling + 36(PollingUpdater.java:36)
       at com.launchdarkly.android.PollingUpdater.startBackgroundPolling + 27(PollingUpdater.java:27)
       at com.launchdarkly.android.ConnectivityManager.startBackgroundPolling + 182(ConnectivityManager.java:182)
       at com.launchdarkly.android.ConnectivityManager.attemptTransition + 258(ConnectivityManager.java:258)
       at com.launchdarkly.android.ConnectivityManager.access$300 + 18(ConnectivityManager.java:18)
       at com.launchdarkly.android.ConnectivityManager$1.run + 64(ConnectivityManager.java:64)
       at com.launchdarkly.android.Throttler$1.run + 49(Throttler.java:49)
       at android.os.Handler.handleCallback + 873(Handler.java:873)
       at android.os.Handler.dispatchMessage + 99(Handler.java:99)
       at android.os.Looper.loop + 214(Looper.java:214)
       at android.os.HandlerThread.run + 65(HandlerThread.java:65)

Caused by android.os.RemoteException: Remote stack trace:
    at com.android.server.SamsungAlarmManagerService.checkMaliciousAppLocked(SamsungAlarmManagerService.java:306)
    at com.android.server.AlarmManagerService.setImpl(AlarmManagerService.java:1748)
    at com.android.server.AlarmManagerService$2.set(AlarmManagerService.java:2090)
    at android.app.IAlarmManager$Stub.onTransact(IAlarmManager.java:92)
    at android.os.Binder.execTransact(Binder.java:739)

SDK version 2.8.4

Additional context https://stackoverflow.com/questions/29344971/java-lang-securityexception-too-many-alarms-500-registered-from-pid-10790-u

gwhelanLD commented 5 years ago

Hi @paulfthomas,

Thanks for the issue report, we appreciate the logs and additional context information. We'll investigate the issue and I'll update when we know more.

For context, is 2.8.4 the first version of the SDK integrated into your application, or is this a new issue after an upgrade?

Thanks, @gwhelanLD

paulfthomas commented 5 years ago

Hi @gwhelanLD,

This issue is not new and was also occurring with the version 2.5.3 we were previously using.

Thanks!

gwhelanLD commented 4 years ago

Hi @paulfthomas,

Firstly let me thank you for your patience as we made the time to investigate this issue. At this time we are unable to replicate a manner in which the Android SDK would create multiple alarms. The referenced stack overflow post shows an issue caused by using PendingIntent.FLAG_CANCEL_CURRENT on PendingIntent.getBroadcast which prevents the AlarmManager.cancel(PendingIntent) call from removing the alarm. The Android SDK does not use this flag and cancels the alarm directly, as well as uses an identical Intent and alarmCode to prevent duplicate alarms. Due to this we suspect that the process may already have 500 alarms through some other source, causing this error to occur on the Samsung devices that enforce this limit.

The solution we'll be including in the next release is to catch this exception to prevent killing the Application, but in the case this is triggered the background polling will not have been able to set up the alarm to trigger polling. I'll leave this issue open until we release that change.

Thanks, @gwhelanLD

gwhelanLD commented 4 years ago

Thanks for your patience on this issue. We've just released version 2.9.1, which addresses this issue in the manner discussed in my previous comment.

Thanks again, @gwhelanLD