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

CredentialManager leaking MainActivity on screen rotation #56

Closed
simon-st opened this issue Nov 19, 2023 · 8 comments
Closed

CredentialManager leaking MainActivity on screen rotation #56

simon-st opened this issue Nov 19, 2023 · 8 comments

Comments

@simon-st
Copy link

simon-st commented Nov 19, 2023

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

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

@niharika2810
Copy link
Contributor

Thank you for sharing, we are looking into it.

@jom16antonio
Copy link

was this resolved?

@niharika2810
Copy link
Contributor

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

simon-st commented Apr 29, 2024

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

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

simon-st commented May 2, 2024

@niharika2810
Copy link
Contributor

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

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

No branches or pull requests

3 participants