permissions-dispatcher / PermissionsDispatcher

A declarative API to handle Android runtime permissions.
https://github.com/permissions-dispatcher/PermissionsDispatcher
Apache License 2.0
11.22k stars 1.44k forks source link

ktx version leaking when permission is denied #735

Closed SamYStudiO closed 3 years ago

SamYStudiO commented 3 years ago

Overview

Environment

Looking at code source i don't understand how PermissionsRequester is cleanup when fragment is destroyed. From PermissionsRequesterImpl it looks that a observer is added to an activity viewmodel but it's never removed when fragment is detroyed. https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/c8876729dfd3cd2158d57098e56e02514792fd62/ktx/src/main/java/permissions/dispatcher/ktx/PermissionsRequesterImpl.kt#L20 This has also the side effect of onNeverAskAgain to be never called with the last fragment created since only the first leaked fragment is called. https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/c8876729dfd3cd2158d57098e56e02514792fd62/ktx/src/main/java/permissions/dispatcher/ktx/PermissionRequestViewModel.kt#L31

Here is the leakcanary log

┬───
│ GC Root: Local variable in native code
│
├─ dalvik.system.PathClassLoader instance
│    Leaking: NO (InternalLeakCanary↓ is not leaking and A ClassLoader is never
│    leaking)
│    ↓ ClassLoader.runtimeInternalObjects
├─ java.lang.Object[] array
│    Leaking: NO (InternalLeakCanary↓ is not leaking)
│    ↓ Object[].[1711]
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary.resumedActivity
├─ com.hellobirdie.android.ui.main.MainActivity instance
│    Leaking: NO (Activity#mDestroyed is false)
│    mApplication instance of com.hellobirdie.android.HelloBirdieApplication
│    mBase instance of androidx.appcompat.view.ContextThemeWrapper
│    ↓ ComponentActivity.mLifecycleRegistry
│                        ~~~~~~~~~~~~~~~~~~
├─ androidx.lifecycle.LifecycleRegistry instance
│    Leaking: UNKNOWN
│    Retaining 1.3 kB in 50 objects
│    ↓ LifecycleRegistry.mObserverMap
│                        ~~~~~~~~~~~~
├─ androidx.arch.core.internal.FastSafeIterableMap instance
│    Leaking: UNKNOWN
│    Retaining 1.1 kB in 45 objects
│    ↓ SafeIterableMap.mEnd
│                      ~~~~
├─ androidx.arch.core.internal.SafeIterableMap$Entry instance
│    Leaking: UNKNOWN
│    Retaining 40 B in 2 objects
│    ↓ SafeIterableMap$Entry.mPrevious
│                            ~~~~~~~~~
├─ androidx.arch.core.internal.SafeIterableMap$Entry instance
│    Leaking: UNKNOWN
│    Retaining 40 B in 2 objects
│    ↓ SafeIterableMap$Entry.mKey
│                            ~~~~
├─ androidx.lifecycle.LiveData$LifecycleBoundObserver instance
│    Leaking: UNKNOWN
│    Retaining 32 B in 1 objects
│    mOwner instance of com.hellobirdie.android.ui.main.MainActivity with
│    mDestroyed = false
│    ↓ LiveData$ObserverWrapper.mObserver
│                               ~~~~~~~~~
├─ permissions.dispatcher.ktx.PermissionRequestViewModel$observe$1 instance
│    Leaking: UNKNOWN
│    Retaining 6.7 MB in 52596 objects
│    Anonymous class implementing androidx.lifecycle.Observer
│    ↓ PermissionRequestViewModel$observe$1.$onNeverAskAgain
│                                           ~~~~~~~~~~~~~~~~
├─ com.hellobirdie.android.ui.main.round.RoundTrackingFragment$onAttach$3
│  instance
│    Leaking: UNKNOWN
│    Retaining 40 B in 1 objects
│    Anonymous subclass of kotlin.jvm.internal.FunctionReferenceImpl
│    ↓ CallableReference.receiver
│                        ~~~~~~~~
╰→ com.hellobirdie.android.ui.main.round.RoundTrackingFragment instance
​     Leaking: YES (ObjectWatcher was watching this because com.hellobirdie.
​     android.ui.main.round.RoundTrackingFragment received Fragment#onDestroy()
​     callback and Fragment#mFragmentManager is null)
​     Retaining 6.7 MB in 52591 objects
​     key = d2ed8f1f-0233-49fa-9dc8-d17175476a11
​     watchDurationMillis = 21023
​     retainedDurationMillis = 14095
​     componentContext instance of dagger.hilt.android.internal.managers.
​     ViewComponentManager$FragmentContextWrapper, wrapping activity com.
​     hellobirdie.android.ui.main.MainActivity with mDestroyed = false

METADATA

Build.VERSION.SDK_INT: 30
Build.MANUFACTURER: Google
LeakCanary version: 2.7
App process name: com.hellobirdie.android
Stats: LruCache[maxSize=3000,hits=9950,misses=176479,hitRate=5%]
RandomAccess[bytes=10156740,reads=176479,travel=166782822934,range=53708121,size
=59863985]
Heap dump reason: user request
Analysis duration: 55326 ms
SamYStudiO commented 3 years ago

Maybe making these weakreferences may fix the pb? https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/c8876729dfd3cd2158d57098e56e02514792fd62/ktx/src/main/java/permissions/dispatcher/ktx/FragmentExtensions.kt#L28-L31

hotchemi commented 3 years ago

Thx, I think https://github.com/permissions-dispatcher/PermissionsDispatcher/pull/741 would address the issue.

hotchemi commented 3 years ago

ktx 1.1.1 has been released.