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

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

Closed
msfjarvis opened this issue Nov 23, 2019 · 6 comments · Fixed by #1636
Closed

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

msfjarvis opened this issue Nov 23, 2019 · 6 comments · Fixed by #1636
Milestone

Comments

@msfjarvis
Copy link
Contributor

msfjarvis commented Nov 23, 2019

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

  • LeakCanary version: 2.0-beta-4
  • Android OS version: 10
  • Gradle version: 6.0.1

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
Copy link
Member

pyricau commented Nov 25, 2019

@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
Copy link
Contributor Author

@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
Copy link
Member

pyricau commented Nov 25, 2019

Thanks, reproduced, bisecting now.

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

@pyricau pyricau added this to the 2.0 Next Release milestone Nov 25, 2019
pyricau added a commit that referenced this issue Nov 25, 2019
* When adding support for native reference leaks (879ab7c) there was a missing bracket in an if/else branch that led to all native gc roots being ignored.

Fixes #1634
@pyricau
Copy link
Member

pyricau commented Nov 25, 2019

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

@msfjarvis
Copy link
Contributor Author

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
Copy link
Member

pyricau commented Nov 25, 2019

Definitely!

pyricau added a commit that referenced this issue Nov 25, 2019
* When adding support for native reference leaks (879ab7c) there was a missing bracket in an if/else branch that led to all native gc roots being ignored.

Fixes #1634
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants