square / leakcanary

A memory leak detection library for Android.
https://square.github.io/leakcanary
Apache License 2.0
29.38k stars 3.97k forks source link

LeakCanary beta 4 fails to find leak that beta 3 does #1634

Closed msfjarvis closed 4 years ago

msfjarvis commented 4 years ago

Description

LeakCanary beta 3 correctly sees BiometricPrompt holding on to the mAuthenticationCallback field. Logcat snippets showing both versions processing the leaking event are here.

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.
  3. Now switch to master and repeat step 2, LeakCanary will dump heap and analyze it again, but will fail to find any leaks.

Expected behavior: LeakCanary beta 4 should also see the leak

Version Information

Additional Information

This exact leak also seems to exist in FingerprintManager as well and was never fixed. I haven't found any fixes or workarounds yet so an update to framework leaks might be in order.

pyricau commented 4 years ago

@msfjarvis Thanks for the detailed report! Can you share the hprof file from the leak found in Beta 3? That way I can repro automatically & git bissect.

msfjarvis commented 4 years ago

@msfjarvis Thanks for the detailed report! Can you share the hprof file from the leak found in Beta 3? That way I can repro automatically & git bissect.

Absolutely, there you go. 2019-11-25_21-47-40_879.hprof

pyricau commented 4 years ago

Thanks, reproduced, bisecting now.

Edit: according to git bisect, bad commit is 879ab7ce807c3d6a3c3a7674cd559df772276d8b . Looking into it.

pyricau commented 4 years ago

Thanks a ton for the bug report! This is pretty bad, I'll make a new release ASAP.

msfjarvis commented 4 years ago

Glad to be of assistance :-)

I want to follow up about the potential framework leaks in BiometricPrompt and FingerprintManager, should I start another issue for those?

pyricau commented 4 years ago

Definitely!