firebase / firebase-android-sdk

Firebase Android SDK
https://firebase.google.com
Apache License 2.0
2.26k stars 572 forks source link

Add defaultHandler check for null #5998

Open mrblrrd opened 4 months ago

mrblrrd commented 4 months ago

Problem

 java.lang.NullPointerException: Attempt to invoke interface method 'void java.lang.Thread$UncaughtExceptionHandler.uncaughtException(java.lang.Thread, java.lang.Throwable)' on a null object reference
  at com.google.firebase.crashlytics.internal.common.CrashlyticsUncaughtExceptionHandler.uncaughtException(CrashlyticsUncaughtExceptionHandler.java:62)
  at com.example.CompositeUncaughtExceptionHandler.uncaughtException(CompositeUncaughtExceptionHandler.kt:37)
  at org.chromium.base.JavaExceptionReporter.uncaughtException(chromium-TrichromeWebViewGoogle6432.aab-stable-636718033:27)
  at java.lang.ThreadGroup.uncaughtException(ThreadGroup.java:1071)
  at java.lang.ThreadGroup.uncaughtException(ThreadGroup.java:1066)
  at java.lang.Thread.dispatchUncaughtException(Thread.java:2306)

Steps to reproduce

Apply bytecode transformation to substitute Thread.setDefaultUncaughtExceptionHandler() and Thread.getDefaultUncaughtExceptionHandler() calls with custom ones.

If you really want to 😅 If you don't please proceed to Relevant Code. The fix is trivial and safe.

@JvmStatic
fun setDefaultUncaughtExceptionHandler(exceptionHandler: Thread.UncaughtExceptionHandler) {
    if (compositeUncaughtExceptionHandlerIsAttached.compareAndSet(false, true)) {
        CompositeUncaughtExceptionHandler.systemUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler()
        Thread.setDefaultUncaughtExceptionHandler(CompositeUncaughtExceptionHandler)
    }
    CompositeUncaughtExceptionHandler.addExceptionHandler(exceptionHandler)
}

@JvmStatic
fun getDefaultUncaughtExceptionHandler(): Thread.UncaughtExceptionHandler? {
    return null
}

The idea is to take control of app-internal UncaughtExceptionHandlers execution, add additional custom keys, etc.

uncaught-exception-handler-Page-1

So that the processing chain looks like

uncaught-exception-handler-Page-2

Relevant Code

I believe this code

    } finally {
      Logger.getLogger().d("Completed exception processing. Invoking default exception handler.");
      defaultHandler.uncaughtException(thread, ex);
      isHandlingException.set(false);
    }

should be

    } finally {
      Logger.getLogger().d("Completed exception processing. Invoking default exception handler.");
      if (defaultHandler != null) {
        defaultHandler.uncaughtException(thread, ex);
      }
      isHandlingException.set(false);
    }

to meet the Thread contract

Screenshot 2024-05-28 at 12 23 20
google-cla[bot] commented 4 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.