From 694d39f55610ac5527c866e0d6612d64e897137f Mon Sep 17 00:00:00 2001 From: Ibraheem Zaman Date: Wed, 1 Dec 2021 15:40:18 +0500 Subject: [PATCH] Fix multiple exception handler invocation in Android The AndroidExceptionPreHandler was changed in commit 5ea9339f144f829da94752f57cdbf044d91a6402 to not invoke the uncaught exception pre-handler on Android Pie and above, and to instead invoke the thread's uncaught exception handler directly on those versions, as Android Pie had added logic in the default uncaught exception handler to ensure that the pre-handler would be invoked. However, the uncaught exception handler is already being invoked for all unhandled coroutine exceptions, so this change in the AndroidExceptionPreHandler caused the uncaught exception handler to be invoked twice in Android Pie and above. Since the default uncaught exception handler implementation in Android kills the app immediately, this normally wouldn't make a difference. However, if an app sets a custom uncaught exception handler (either at the thread or global level) that doesn't always kill the app, then the duplicate invocations might cause undesired or wrong behaviour. This is now fixed by removing the uncaught exception handler invocation from AndroidExceptionPreHandler. Since the pre-handler was introduced in Android Oreo, I have also added a minimum SDK check for Android Oreo as well. Also edited the documentation in the comments to add more details and context. --- .../src/AndroidExceptionPreHandler.kt | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt b/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt index 7d06752cba..af32191f53 100644 --- a/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt +++ b/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt @@ -34,20 +34,21 @@ internal class AndroidExceptionPreHandler : override fun handleException(context: CoroutineContext, exception: Throwable) { /* - * If we are on old SDK, then use Android's `Thread.getUncaughtExceptionPreHandler()` that ensures that - * an exception is logged before crashing the application. + * Android Oreo introduced private API for a global pre-handler for uncaught exceptions, to ensure that the + * exceptions are logged even if the default uncaught exception handler is replaced by the app. The pre-handler + * is invoked from the Thread's private dispatchUncaughtException() method, so our manual invocation of the + * Thread's uncaught exception handler bypasses the pre-handler in Android Oreo, and uncaught coroutine + * exceptions are not logged. This issue was addressed in Android Pie, which added a check in the default + * uncaught exception handler to invoke the pre-handler if it was not invoked already (see + * https://android-review.googlesource.com/c/platform/frameworks/base/+/654578/). So the issue is present only + * in Android Oreo. * - * Since Android Pie default uncaught exception handler always ensures that exception is logged without interfering with - * pre-handler, so reflection hack is no longer needed. - * - * See https://android-review.googlesource.com/c/platform/frameworks/base/+/654578/ + * We're fixing this by manually invoking the pre-handler using reflection, if running on an Android Oreo SDK + * version (26 and 27). */ - val thread = Thread.currentThread() - if (Build.VERSION.SDK_INT >= 28) { - thread.uncaughtExceptionHandler.uncaughtException(thread, exception) - } else { + if (Build.VERSION.SDK_INT in 26..27) { (preHandler()?.invoke(null) as? Thread.UncaughtExceptionHandler) - ?.uncaughtException(thread, exception) + ?.uncaughtException(Thread.currentThread(), exception) } } }