Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mAuthenticationCallback field leaks in FingerprintManager and BiometricPrompt #1640

Closed
msfjarvis opened this issue Nov 25, 2019 · 7 comments
Closed

Comments

@msfjarvis
Copy link
Contributor

LeakTrace information

BiometricPrompt leaks the field from here and FingerprintManager from here.

├─ android.app.Activity$1
│    Leaking: UNKNOWN
│    Anonymous subclass of android.app.IRequestFinishCallback$Stub
│    GC Root: Global variable in native code
│    ↓ Activity$1.this$0
│                 ~~~~~~
╰→ com.stylingandroid.biometrics.MainActivity
​     Leaking: YES (Activity#mDestroyed is true and ObjectWatcher was watching this)
​     key = 87e61d6d-a54f-430e-9e56-d063fe83336c
​     watchDurationMillis = 5222
​     retainedDurationMillis = 186
, retainedHeapByteSize=95980), ApplicationLeak(className=androidx.biometric.BiometricFragment, leakTrace=
┬
├─ android.hardware.biometrics.BiometricPrompt$1
│    Leaking: UNKNOWN
│    Anonymous subclass of android.hardware.biometrics.IBiometricServiceReceiver$Stub
│    GC Root: Global variable in native code
│    ↓ BiometricPrompt$1.this$0
│                        ~~~~~~
├─ android.hardware.biometrics.BiometricPrompt
│    Leaking: UNKNOWN
│    ↓ BiometricPrompt.mAuthenticationCallback
│                      ~~~~~~~~~~~~~~~~~~~~~~~
├─ androidx.biometric.BiometricFragment$2
│    Leaking: UNKNOWN
│    Anonymous subclass of android.hardware.biometrics.BiometricPrompt$AuthenticationCallback
│    ↓ BiometricFragment$2.this$0
│                          ~~~~~~
╰→ androidx.biometric.BiometricFragment
​     Leaking: YES (Fragment#mFragmentManager is null and ObjectWatcher was watching this)
​     key = 17e49419-e52d-4068-8633-d7bf986c8eb4
​     watchDurationMillis = 5216
​     retainedDurationMillis = 185
, retainedHeapByteSize=1519)], libraryLeaks=[])
@pyricau
Copy link
Member

pyricau commented Nov 25, 2019

From #1634 :

Steps to Reproduce

Sample project: https://github.com/msfjarvis/leakcanary-test-project

  1. Checkout the sample project to d3f9bbfedf78fc3bdb2ecc889e7bfc4637835fa3, so you can be on LeakCanary beta 3.
  2. Install and launch the app, authenticate with fingerprint, and then use home to go back. LeakCanary will automatically analyze and find the leak.

Hprof: 2019-11-25_21-47-40_879.hprof

@dknchris
Copy link

dknchris commented Nov 26, 2019

A workaround is to house the AuthenticationCallback in a static class and have a LiveData for the activity to observe the callback results. Be sure to use Application context in BiometricPrompt.Builder (I'm using framework APIs instead of androidx.*)

PS: I just need the success callback in my implementation. If there's a cleaner approach, please post it here.

import android.hardware.biometrics.BiometricPrompt
import android.os.Build
import androidx.annotation.RequiresApi

@RequiresApi(Build.VERSION_CODES.P)
object BiometricLeakFix {

    private const val logTag = "BiometricLeakFix"
    private var requestCode: Int? = null
    val onBiometricSuccessRequestCode by lazy { SafeSingleEvent<Int>() }

    private val callback by lazy {
        object : BiometricPrompt.AuthenticationCallback() {
            
            override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult?) {
                onBiometricSuccessRequestCode.value = requestCode
            }

            override fun onAuthenticationError(errorCode: Int, errString: CharSequence?) {
                if (errorCode == BiometricPrompt.BIOMETRIC_ERROR_NO_DEVICE_CREDENTIAL) {
                    onBiometricSuccessRequestCode.value = requestCode
                } else {
                    Log.i(logTag, "Biometric auth error(requestCode=$requestCode)=$errorCode:$errString")
                }
            }
        }
    }

    fun getAuthCallback(requestCode: Int): BiometricPrompt.AuthenticationCallback {
        this.requestCode = requestCode
        return callback
    }
}

@msfjarvis
Copy link
Contributor Author

  1. Checkout the sample project to d3f9bbfedf78fc3bdb2ecc889e7bfc4637835fa3, so you can be on LeakCanary beta 3.

I've bumped the repository to 2.0-beta-5 (commit) so this shouldn't be needed anymore.

@pyricau
Copy link
Member

pyricau commented Jun 11, 2020

@msfjarvis sorry I meant to reply earlier and forgot. Did you file a leak on the Google issue tracker so that they can fix it in a newer version? Also, can you check if the leak still happens in Android 11?

@msfjarvis
Copy link
Contributor Author

@msfjarvis sorry I meant to reply earlier and forgot. Did you file a leak on the Google issue tracker so that they can fix it in a newer version? Also, can you check if the leak still happens in Android 11?

There's a couple filed issues [1] [2] over on the Jetpack side of things that mention this needs framework fixes so I didn't file a separate bug. The bug for FingerprintManager about this same problem was marked as fixed, then said fix was reverted before marking the issue as obsolete in a mass cleanup. Leak seems to still happen in Android 11, but AndroidX biometric has a couple fixes queued [1] [2].

@msfjarvis
Copy link
Contributor Author

This leak has been resolved in AndroidX (issue) in the latest biometric alpha and I've been able to confirm I no longer leak activities with biometric prompt.

@pyricau
Copy link
Member

pyricau commented Aug 31, 2020

🙏

Closing

Note: there's another biometric leak that I believe is still in alpha02: https://issuetracker.google.com/issues/167014923

@pyricau pyricau closed this as completed Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants