AltBeacon / android-beacon-library

Allows Android apps to interact with BLE beacons
Apache License 2.0
2.84k stars 836 forks source link

java.lang.IllegalStateException in android.bluetooth.le.BluetoothLeUtils.checkAdapterStateOn #414

Closed catchmartin closed 7 years ago

catchmartin commented 8 years ago

Expected behavior

Exception to be caught by library, in event of unusual IllegalStateException

Actual behavior

Exception not caught and causes app crash.

java.lang.IllegalStateException: BT Adapter is not turned ON
at android.bluetooth.le.BluetoothLeUtils.checkAdapterStateOn(BluetoothLeUtils.java:136)
at android.bluetooth.le.BluetoothLeScanner$1.handleMessage(BluetoothLeScanner.java:85)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:168)
at android.app.ActivityThread.main(ActivityThread.java:5885)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:797)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:687)

Steps to reproduce this behavior

Exact sequence not found but appears to be happening predominantly on HTC devices on app start. This has been reported from two apps we are involved with. Additionally, found the same issue reported on http://stackoverflow.com/q/37965687/5580666

Mobile device model and OS version

Description from one of the app teams:

"It seems that there is some problem with some new HTC phones (like htc one_e8 dual sim) that are running latest Android 6.0. Possibly this is bug in their firmware. This bug is on the 6th place with total crashes of 147.

It probably causes app to crash on start, so It's not possible for users to use the app when bluetooth is turned on. It's reported even in Google Play."

Android Beacon Library version

Seen in version 2.6.1 in the two apps we are involved with and also reported in version 2.7 on http://stackoverflow.com/q/37965687/5580666

IMPORTANT: This forum is reserved for feature requests or reproducible bugs with the library itself. If you need help with using the library with your project, please open a new question on StackOverflow.com.

AlexLardschneider commented 8 years ago

The same issue happens in my app.

It seems to only happen on HTC devices running 6.0.1 and as soon as the user disables bluetooth while the app is scanning for beacons. The beacon handler in my app gets unbound as soon as bluetooth is disabled (i.e. the bluetooth receiver is called), but it still crashes.

The issue happens with AltBeacon 2.7 - 2.9.

Stacktrace from Crashlytics:

Fatal Exception: java.lang.IllegalStateException: BT Adapter is not turned ON
       at android.bluetooth.le.BluetoothLeUtils.checkAdapterStateOn(BluetoothLeUtils.java:136)
       at android.bluetooth.le.BluetoothLeScanner$1.handleMessage(BluetoothLeScanner.java:85)
       at android.os.Handler.dispatchMessage(Handler.java:102)
       at android.os.Looper.loop(Looper.java:168)
       at android.app.ActivityThread.main(ActivityThread.java:5821)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:797)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:687)
davidgyoung commented 8 years ago

According to the stack trace, there is code inside an anonymous inner class in BluetoothLeScanner that throws this exception on a bluetooth scan callback if bluetooth is turned off. Note that there is no such code in the open source version of this class shown here:

https://github.com/android/platform_frameworks_base/blob/master/core/java/android/bluetooth/le/BluetoothLeScanner.java

This means that this is probably an HTC-specific customization to that class. And because the exception is thrown inside the lower-level Android scanning classes, I don't think there is any way to automatically catch the exception in the Android Beacon Library. If you look at the stack trace, you don't see any library or application classes where it could be caught.

One possibility for a workaround (until HTC hopefully fixes this bug) is to use a Thread.UncaughtExceptionHandler to catch this and keep the app from crashing if the uncaught exception matches this pattern ("BT Adapter is not turned ON"). The trick is to figure out a way to attache the handler to the right thread. There is a discussion of how to do this here: http://stackoverflow.com/a/21338414/1461050

@AlexLardschneider and @catchmartin, can one of you experiment with this solution in your own app code since you can obviously reproduce the problem? If we can find something that works to fix this, we could potentially add it to the library as an automatic registration or an optional utility.

AlexLardschneider commented 8 years ago

@davidgyoung as I cannot reproduce the issue myself and only have reports on Crashlytics about this issue, the only thing I can do is try to attach the Thread.UncaughtExceptionHandler to the right thread, release an app update and hope that it fixes the crash on the affected devices.

Might take a while for all the people to download the newest version and test it out though... If I can get my hands on a HTC device which is affected by this issue, I will test it ASAP, thanks!

catchmartin commented 8 years ago

Thanks @davidgyoung https://github.com/davidgyoung for the extra info here. That UncaughtExceptionHandler solution sounds like it would be worth a look alright.

I'm in the same boat, unfortunately, regarding not being able to reproduce the crash myself. @AlexLardschneider https://github.com/AlexLardschneider, can you provide a list of affected HTC device models from your crashlytics? I don't have direct access to the crashlytics reports from the apps we're involved with, so this would be a big help for trying to get a device to replicate the issue on.

On Wed, Aug 17, 2016 at 3:19 PM, Alex Lardschneider < notifications@github.com> wrote:

@davidgyoung https://github.com/davidgyoung as I cannot reproduce the issue myself and only have reports on Crashlytics about this issue, the only thing I can do is try to attach the Thread.UncaughtExceptionHandler to the right thread, release an app update and hope that it fixes the crash on the affected devices.

Might take a while for all the people to download the newest version and test it out though... If I can get my hands on a HTC device which is affected by this issue, I will test it ASAP, thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AltBeacon/android-beacon-library/issues/414#issuecomment-240426392, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLJfQqlWxx9WSlOdb8qDhCvpV85bQdpks5qgxiIgaJpZM4JlQgU .

AlexLardschneider commented 8 years ago

@catchmartin sure,

all the affected devices run Android 6.0.1

catchmartin commented 8 years ago

Brilliant, thanks @AlexLardschneider I'll try to track one of those down.

davidgyoung commented 8 years ago

One way to do this would be to use a device farm like the one on AWS.

https://aws.amazon.com/device-farm/device-list/

I see that they have a HTC One A9 (Unlocked) (6.0.1) available for testing online.

AlexLardschneider commented 8 years ago

@davidgyoung How would you test the beacons there though? Is it possible to broadcast a beacon and locate it in the device farm?

davidgyoung commented 8 years ago

No, you cannot test beacons. But in this case you don't need to. You just need to turn off Bluetooth and see if the app crashes.

catchmartin commented 8 years ago

Gave it a quick look and it will need to be an automated test which programatically toggles the bluetooth radio on/off a few times in the app.

You can't run apps on the HTC One A9 via remote access, unfortunately, because that particular device is not available for remote access. Just available for automated tests.

I should be able to give this a look again later in the week, hopefully. If nobody else has cracked it before then I'll let you know how I get on.

On Wed, Aug 17, 2016 at 4:03 PM, David G. Young notifications@github.com wrote:

No, you cannot test beacons. But in this case you don't need to. You just need to turn off Bluetooth and see if the app crashes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AltBeacon/android-beacon-library/issues/414#issuecomment-240440580, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLJfa2ucQcJXV1BeCMQ-ckPYVDmPJWJks5qgyKpgaJpZM4JlQgU .

AlexLardschneider commented 7 years ago

So I actually tried using a UncaughtExceptionHandler in production to prevent this crash from happening.

if (Build.MANUFACTURER.equalsIgnoreCase("htc")) {
            Timber.e("Current device is a HTC device, setting uncaught exception handler");

            Thread.setDefaultUncaughtExceptionHandler((t, e) -> {
                if (e instanceof IllegalStateException && e.getMessage()
                        .contains("BT Adapter is not turned ON")) {

                    Timber.e("Got a BT Adapter exception.");

                    throw (RuntimeException) e; // Or maybe use System.exit(0); ?
                }
            });
        }

This code snippet seems to work fine, and detects the crash. I don't know if rethrowing the exception is needed, I'm assuming the VM is in an unstable state. Maybe it could be replaced by a call to System.exit(0);?

This code snippet needs to be called before calling BeaconManager.bind(). So what do you guys think about this solution? Has someone found a different solution?

davidgyoung commented 7 years ago

Good to hear this works, @AlexLardschneider. Did you test with the device farm or with a physical device?

I think this should be built in to the CycledScanner class and executed on start up.

I'm not sure what to do if the exception is caught. Swallowing it (do not rethrow or exit) might be the best default behavior since it emulates how other Android devices behave. Perhaps this should be combined with a way of disabling this hack.

Thoughts?

I could prepare a PR and ad-hoc build for testing if this solution sounds workable.

AlexLardschneider commented 7 years ago

I published a new version of my app, which currently runs on about 8000 devices (this workaround has been in production for almost a month and no more crashes have been detected.). According to Crashlytics logging did this workaround cause no other issues or unintended behaviour.

I used System.exit(0) to exit the app if it encounters the crash, as it only seems to happen if the app is running in the background and the user won't notice anything, whereas rethrowing would show the annoying crash dialog. I don't know what the best thing to do would be if the exception is caught, maybe notify the developer about it and swallow it?

I guess you could try and add it to CycledScanner, also a way to disable it would be good idea. If you could prepare a PR, I'm open for testing the solution afterwards.

davidgyoung commented 7 years ago

Ok, sounds like a plan. I will prepare a PR and an ad-hoc build.

davidgyoung commented 7 years ago

@AlexLardschneider, while preparing the PR I realized that the solution of using an UncheckedExceptionHandler can't work. The problem is that it does not stop a thread from terminating, it just allow you to get a callback before the termination happens. It doesn't allow you to stop it. This probably means that the code you tried with this is simply hiding the crashes from any HTC device in your crash reports by overriding Crashlytics' own UncheckedExceptionHandler. Sorry. I know this was my idea, but upon more reflection, I think it was a bad one. :(

Another way to solve this problem may be to prevent the creation of a BluetoothLeScanner at all if bluetooth is off.

davidgyoung commented 7 years ago

@AlexLardschneider, I have published a pre-release for testing here:

https://github.com/AltBeacon/android-beacon-library/releases/tag/2.9.2-beta1

I am unsure if this will solve the problem, but I think there is a good chance. If you could try an update of your app with this change, and see if you get further Crashlytics errors on HTC devices I'd appreciate it.

AlexLardschneider commented 7 years ago

@davidgyoung thanks, will test out the PR ASAP.

Doesn't setting a UncheckedExceptionHandler, which does not rethrow the uncaught exception, also hide the "Unfortunately App has stopped" dialog? If it does, that would also kinda solve the problem as the user wouldn't be notified about the crash.

You are probably right about the custom UncheckedExceptionHandler overriding the Crashlytics handler though.

davidgyoung commented 7 years ago

Good question about the "Unfortunately App has stopped" dialog. I do not know, but it would be easy to test by throwing a RuntimeException without and with an UncheckedExceptionHandler.

Regardless, in many cases folks build apps where beacon detection is secondary, and they want them to not crash even if beacon detection will not work.

AlexLardschneider commented 7 years ago

Sure, trying to prevent the error from happening at all is always better than catching it in case it happens. Just something to keep in mind should the fix in your PR not work as intended.

RapsIn4 commented 7 years ago

Any updates on this issue? Do we have an ETA for a stable build of the fix?

davidgyoung commented 7 years ago

Hi, @Rapsin4. Since I do not have a device that can reproduce the problem I am waiting to see if you or @AlexLardschneider can verify the exception does not happen with changes in 2.9.2-beta1 or 2.9.2-beta2.

Since the fix is benign and low risk, I plan to roll a new release late next week even if confirmation is not available. That said, it would be better to be able to close out this issue by confirming if possible.

AlexLardschneider commented 7 years ago

@davidgyoung the new app version (including version 2.9.2-beta1 of the library) has been out in the wild for about a week now. No single crash has been reported through Crashlytics, so I'd consider the fix you implemented a success.

RapsIn4 commented 7 years ago

Thanks guys, I guess I'll just wait for the release next week 👍

davidgyoung commented 7 years ago

Thanks for the report, @AlexLardschneider

RapsIn4 commented 7 years ago

I'm still seeing this with 2.9.2. Is anyone else experiencing this?

davidgyoung commented 7 years ago

@RapsIn4, do you have an updated stack trace?

RapsIn4 commented 7 years ago

It looks like it's the same.

Fatal Exception: java.lang.IllegalStateException: BT Adapter is not turned ON
       at android.bluetooth.le.BluetoothLeUtils.checkAdapterStateOn(BluetoothLeUtils.java:136)
       at android.bluetooth.le.BluetoothLeScanner$1.handleMessage(BluetoothLeScanner.java:85)
       at android.os.Handler.dispatchMessage(Handler.java:102)
       at android.os.Looper.loop(Looper.java:168)
       at android.app.ActivityThread.main(ActivityThread.java:5885)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:797)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:687)
davidgyoung commented 7 years ago

Yeah, that looks the same. Again, this crash is caused by an internal Android class that throws this exception when bluetooth scanning is running when Bluetooth is turned off. The modifications in 2.9.2 made it so that scans are not started if bluetooth is turned off. If this stack trace is from 2.9.2, two possibilities come to mind:

  1. Perhaps the user turned off bluetooth after the scan was started. In this case, a crash may be impossible to prevent. We could add code that listens for bluetooth being turned off and stops scans in this case, but it may not happen fast enough to prevent a crash.

  2. Perhaps there is a hole in the implementation here: https://github.com/AltBeacon/android-beacon-library/pull/452

At any rate, I really wish we could find somebody who actually has a device that exhibits this behavior. Relying on crash logs to test potential fixes is like working in the dark.

RapsIn4 commented 7 years ago

I'm currently seeing ~2000 crashes per day for this issue.

  1. How long does a scan take? I doubt that this many users are turning off BT during this time.

  2. Your fix looks fine to me.

If I can find a device, do you know how I can trigger AltBeacon to test?

Thanks!

davidgyoung commented 7 years ago

Yeah, I agree, if you are seeing 2000 crashes a day, then it is not caused by users turning off bluetooth. By default, scans are on for 10 seconds every five minutes in the background. (3% of the time), meaning that you might get a crash for 3% of the time that an affected device turns off bluetooth. 2000 per day seems way too much.

You positive these reports are coming from apps with a 2.9.2 version of the library? Are there any other libraries or code that may be performing bluetooth scanning in your app?

g-combs-hobsons commented 7 years ago

I'm glad to see this is being looked into, as I too have identified this bug occurring for HTC devices specifically. To add to this discussion, I do know that I was able to recreate this issue for the first scenario that David defined above:

Perhaps the user turned off bluetooth after the scan was started. 

Of course, this is limited in number of occurrences because it is specific to HTC devices and the above scenario.

As for the UncheckedExceptionHandler approach, we already do that should an uncaught exception occur within the app in order to present the user with a more friendly user experience and do custom capturing of the exception metadata/analytics. That approach isn't really a "solution", as it is just a callback, all context is lost, and the app must be restarted for any prompt/response to be presented to the user via the app.

jbarmanet commented 7 years ago

I still see this occurring with 2.10 The user has an HTC Desire 825 running Android 6.0.1

Stack Trace :

 java.lang.IllegalStateException: BT Adapter is not turned ON
        at android.bluetooth.le.BluetoothLeUtils.checkAdapterStateOn(BluetoothLeUtils.java:136)
        at android.bluetooth.le.BluetoothLeScanner$1.handleMessage(BluetoothLeScanner.java:85)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:168)
        at android.app.ActivityThread.main(ActivityThread.java:5885)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:797)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:687)
tekinalper commented 7 years ago

Is this crash fixed? I still receive on v2.12.1 for the devices which have Android version 6.0.1

davidgyoung commented 7 years ago

@tekinalper, I sure thought his was fixed based on this comment above. Unfortunately, I don't have any theories about what could be causing that crash, and I don't have an HTC device on which to try to reproduce it. Any chance you have one?

AlexLardschneider commented 7 years ago

@davidgyoung Since I updated AltBeacon to the bug fix version above, I don't get any error reports on Crashlytics, so I'd consider it a working fix.

davidgyoung commented 7 years ago

Good to hear. Thanks for the update.

VvoidVano commented 6 years ago

Occurring with 2.13 HTC 6.0 os Any solutions ?

davidgyoung commented 6 years ago

@VvoidVano can you please attach your stack trace and note the device model you saw it on? I am getting inconsistent reports of if this is fixed.

I worry this is a timing issue and my solutions make it impossible to proactively prevent Bluetooth from being off in all cases when asynchronous background thread inside HTC customized OS code throws an unchecked exception.

HTC really should not do this and needs to fix it. But there may be no solution for any Bluetooth app to prevent the crash in all cases on an HTC device that has this customized ROM.

The only other option is to hide the crash from the user with an UncheckedExceptionHandler. I do not want to make this automatic in the library as it effectively disables Crashlitics exception reporting for other cases.

But if you really want to do this you can put the following in the onCreate method of a custom Application class. (see my next post for sample code)

VvoidVano commented 6 years ago

@davidgyoung Hi David, here's the log and phone details

HTC_0PFJ50 HTC Desrie D530

Fatal Exception: java.lang.IllegalStateException: BT Adapter is not turned ON at android.bluetooth.le.BluetoothLeUtils.checkAdapterStateOn(BluetoothLeUtils.java:136) at android.bluetooth.le.BluetoothLeScanner$1.handleMessage(BluetoothLeScanner.java:85) at android.os.Handler.dispatchMessage(Handler.java:102) at android.os.Looper.loop(Looper.java:168) at android.app.ActivityThread.main(ActivityThread.java:5885) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:797) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:687)

davidgyoung commented 6 years ago

Do you simply want to remove this exception from your crash reports? If so, paste the following code in the onCreate method of your custom android.app.Application class:

    final Thread.UncaughtExceptionHandler existingExceptionHandler = Thread.getDefaultUncaughtExceptionHandler();
    Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
        @Override
        public void uncaughtException(Thread thread, Throwable throwable) {
            if (throwable.getMessage().equalsIgnoreCase(" BT Adapter is not turned ON")) {
                Log.e(TAG, "HTC threw an exception for scanning with bluetooth off. Suppressing crash by exiting.", throwable);
                System.exit(0);
            }
            else {
                existingExceptionHandler.uncaughtException(thread, throwable);
            }
        }
    });

Should you add this code?

YES, add this code if your primary goal is to clean up your crash reports and you don't want this crash appearing on the list. Perhaps you need to get your boss off your back. But understand if you add this code, you won't be stopping the crash, only hiding it from the report. Nobody can stop the crash besides HTC because HTC-customized Android ROMs crash apps if bluetooth is ever found to be turned off if the app is found to be performing a scan. Third party code can never guarantee this unless they never do bluetooth scans.

NO, do not add this code if you don't want to mask a real world problem that is beyond your control and the control of this library. Perhaps you have an understanding boss who will appreciate that some crashes in crash reports are unavoidable due to bad decisions made by mobile phone manufacturers like HTC.