android / identity-samples

Multiple samples showing the best practices in identity on Android.
Apache License 2.0
322 stars 198 forks source link

CredentialManager leaking MainActivity on screen rotation #56

Closed simon-st closed 1 month ago

simon-st commented 7 months ago

Steps to reproduce:

  1. Add LeakCanary to CredentialManagerSample app build.gradle (debugImplementation("com.squareup.leakcanary:leakcanary-android:2.12"))
  2. Run CredentialManagerSample app
  3. Click "SIGN UP"
  4. Enter username and click "Sign up with passkey"
  5. While the passkey creation dialog is open, rotate the device
  6. Observe; passkey creation dialog gets cancelled, yet LeakCanary shows a leak of MainActivity

Note: I also checked in the Android Studio profiling heap dump and it also shows the same leak.

The problem seems to be in the android CredentialManager, not in the CredentialManagerSample app.

Affected credentials library versions: 1.2.0-beta03 and 1.2.0

LeakCanary output:

┬───
│ GC Root: Global variable in native code
│
├─ android.credentials.CredentialManager$CreateCredentialTransport instance
│ Leaking: UNKNOWN
│ Retaining 57.9 kB in 1137 objects
│ mContext instance of com.google.credentialmanager.sample.MainActivity with
│ mDestroyed = true
│ ↓ CredentialManager$CreateCredentialTransport.mContext
│ ~~~~~~~~
╰→ com.google.credentialmanager.sample.MainActivity instance
     Leaking: YES (ObjectWatcher was watching this because com.google.
     credentialmanager.sample.MainActivity received Activity#onDestroy()
     callback and Activity#mDestroyed is true)
     Retaining 51.5 kB in 1000 objects
     key = d8330c9b-b0e1-4654-8630-f8917d10c9ff
     watchDurationMillis = 89005
     retainedDurationMillis = 84002
     mApplication instance of android.app.Application
     mBase instance of androidx.appcompat.view.ContextThemeWrapper

METADATA

Build.VERSION.SDK_INT: 34
Build.MANUFACTURER: Google
LeakCanary version: 2.12
App process name: com.google.credentialmanager.sample
Class count: 27499
Instance count: 199781
Primitive array count: 146738
Object array count: 29293
Thread count: 25
Heap total bytes: 27370119
Bitmap count: 0
Bitmap total bytes: 0
Large bitmap count: 0
Large bitmap total bytes: 0
Stats: LruCache[maxSize=3000,hits=111427,misses=192082,hitRate=36%]
RandomAccess[bytes=9443680,reads=192082,travel=62550215614,range=33191621,size=4
1623102]
Analysis duration: 184237 ms
simon-st commented 7 months ago

This issue seems to be similar, but not exactly the same

niharika2810 commented 5 months ago

Thank you for sharing, we are looking into it.

jom16antonio commented 2 months ago

was this resolved?

niharika2810 commented 2 months ago

Hey, bumping in to check if you still face this with GMS latest versions? Please share any video recrding and bug report if you can with latest versions.

simon-st commented 2 months ago

I tried this again with the latest main branch of identity-samples repo and running CredentialManagerSample in profile mode from AS. Rotated the screen a few times while the passkey bottom sheet was open and then took a heap dump that shows one instance being leaked per rotation. It looks like the reference to the activity is kept in CredentialManager as you can see in below screenshot.

I did this test on a Pixel 8 emulator that is completely updated.

Video: https://github.com/android/identity-samples/assets/1965839/430de945-2f8b-4d7d-a384-18d17697062f

Heap dump screenshot:

Screenshot 2024-04-29 at 9 19 11 AM
niharika2810 commented 2 months ago

Thanks, would you mind creating a bug with all details required in that template? Sharing here : https://issuetracker.google.com/issues/new?component=1301097&template=1773864

simon-st commented 1 month ago

here you go: https://issuetracker.google.com/issues/338398658

niharika2810 commented 1 month ago

Thank you, closing from here so that we can track and respond at one place. Please refer to the bug link for next steps :)