google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.41k stars 2.01k forks source link

Leak while integrating with navigation component #2070

Closed mpierucci closed 3 years ago

mpierucci commented 4 years ago
┬───
│ GC Root: System class
│
├─ android.view.inputmethod.InputMethodManager class
│    Leaking: NO (InputMethodManager↓ is not leaking and a class is never leaking)
│    ↓ static InputMethodManager.sInstance
├─ android.view.inputmethod.InputMethodManager instance
│    Leaking: NO (DecorView↓ is not leaking and InputMethodManager is a singleton)
│    ↓ InputMethodManager.mNextServedView
├─ com.android.internal.policy.DecorView instance
│    Leaking: NO (LinearLayout↓ is not leaking and View attached)
│    mContext instance of com.android.internal.policy.DecorContext, wrapping activity com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    Parent android.view.ViewRootImpl not a android.view.View
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    ↓ DecorView.mContentRoot
├─ android.widget.LinearLayout instance
│    Leaking: NO (MainActivity↓ is not leaking and View attached)
│    mContext instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    View.parent com.android.internal.policy.DecorView attached as well
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    ↓ LinearLayout.mContext
├─ com.mpierucci.android.hiltmemleak.MainActivity instance
│    Leaking: NO (NavHostFragment↓ is not leaking and Activity#mDestroyed is false)
│    ↓ MainActivity.mFragments
├─ androidx.fragment.app.FragmentController instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentController.mHost
├─ androidx.fragment.app.FragmentActivity$HostCallbacks instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentActivity$HostCallbacks.mFragmentManager
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentManagerImpl.mPrimaryNav
├─ androidx.navigation.fragment.NavHostFragment instance
│    Leaking: NO (Fragment#mFragmentManager is not null)
│    ↓ NavHostFragment.mNavController
│                      ~~~~~~~~~~~~~~
├─ androidx.navigation.NavHostController instance
│    Leaking: UNKNOWN
│    ↓ NavHostController.mOnDestinationChangedListeners
│                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    ↓ CopyOnWriteArrayList.elements
│                           ~~~~~~~~
├─ java.lang.Object[] array
│    Leaking: UNKNOWN
│    ↓ Object[].[0]
│               ~~~
├─ androidx.navigation.ui.ToolbarOnDestinationChangedListener instance
│    Leaking: UNKNOWN
│    ↓ ToolbarOnDestinationChangedListener.mContext
│                                          ~~~~~~~~
├─ dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper instance
│    Leaking: UNKNOWN
│    ViewComponentManager$FragmentContextWrapper wraps an Activity with Activity.mDestroyed false
│    ↓ ViewComponentManager$FragmentContextWrapper.fragment
│                                                  ~~~~~~~~
╰→ com.mpierucci.android.hiltmemleak.LeakFragment instance
​     Leaking: YES (ObjectWatcher was watching this because com.mpierucci.android.hiltmemleak.LeakFragment received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
​     key = 257b183a-777d-43c3-a1b0-0d470d6d415c
​     watchDurationMillis = 19270
​     retainedDurationMillis = 14268

METADATA

Build.VERSION.SDK_INT: 28
Build.MANUFACTURER: Google
LeakCanary version: 2.4
App process name: com.mpierucci.android.hiltmemleak
Analysis duration: 7339 ms

The following leak is reported when integrating Hilt with fragments and navigation components. A full project to reproduce the leak can be found Here

Basically you just need to add @AndroidEntryPoint to a fragment that updates the toolbar ( as suggested by the official documentation) and you'll get a leak when navigating back and forth.

Since this issue does not happen if:

or

I´ll report it in both libraries

lukas1 commented 3 years ago

I'd like to at least understand which component is the faulty one. I've looked into sources of navigation library and it would seem that they're holdin Weak reference on Toolbar and therefore I'd expect it to be released. And that makes me think, that it's something in Dagger.Hilt that makes it not work that way.

I'm purely guessing here, but it also sounds like it could be related to this: https://github.com/airbnb/epoxy/issues/1025

I didn't understand what exactly in the generated Dagger.Hilt could cause that behaviour however.

bcorso commented 3 years ago

The issue is likely that views inflated by a Hilt fragment wrap the context in FragmentContextWrapper which holds an instance of the fragment (required for injection of a Hilt view).

The reason there is no leak when inflating from a non-Hilt fragment is because normally the inflator just uses the activity's context, so there is no reference to the fragment.

Possible solutions:

  1. Our initial assumption was that a view created by a fragment shouldn't outlive the fragment, so there should never be a leak. Keeping with that assumption, you can avoid the issue by explicitly using the activity context when inflating a view that is meant to outlive its parent fragment.

  2. On the other hand, our assumption is not always true -- especially given that it's technically okay for a view to outlive its parent fragment when not using Hilt. In addition, this change in Hilt is subtle, the error messages aren't great, and you only get the error at runtime. Thus, we should also look into avoiding this leak within the framework (e.g. clearing the fragment instance in onDestroy).

wbervoets commented 3 years ago

Is there a workaround possible to mitigate the memory leak until this bug is fixed? ToolbarOnDestinationChangedListener always uses toolbar.getContext() which returns the fragment context in my case.

The only workaround I see is creating my own ToolbarOnDestinationChangedListener, passing in the activity context. But then I need to duplicate NavigationUI too.

mpierucci commented 3 years ago

@wbervoets I´ve created this extension function for now:

fun Fragment.fallbackToolbarSetUp(toolbar: Toolbar) {
    val controller = findNavController()
    with(toolbar) {
        navigationIcon = `DrawerArrowDrawable`(requireContext()).apply { progress = 1f }
        setNavigationOnClickListener {
            controller.navigateUp()
        }
    }
}

It's far from ideal and I hope a fix is released soon, but for now, works for my (rather simple) use-cases.

wbervoets commented 3 years ago

@mpierucci seems like I have a problem tinting the drawer arrow drawable with a color (when the toolbar has a style applied which sets a android:theme, which sets the colorControlNormal color); it just doesn't work when one passes requiresContext() instead of toolbar.context

I hope the dagger team fixes this bug soon, is their any ETA?

bcorso commented 3 years ago

The fix for this should be submitted shortly, but unfortunately the fix won't make the cut for the next release happening this week. We'll have another release within the next 2 weeks, so the fix will be in that release.

wbervoets commented 3 years ago

Am I right to assume this fix is inside 2.31.1 but not anymore in 2.31.2 ?

bcorso commented 3 years ago

I don't believe it's in either, or at least that was the plan.

IIRC, it was committed after 2.31 was released and we reverted it before 2.31.1 was released. The reason we reverted it was because we didn't want to introduce this potentially breaking feature in a patch release (2.31.1 / 2.31.2). Instead, we'll aim to get it back in for the 2.32 release.

marcoRS commented 3 years ago

Is the fix for this in 2.32? I can't find an issue link in https://github.com/google/dagger/releases/tag/dagger-2.32

osipxd commented 3 years ago

I don't see changes related to this issue in https://github.com/google/dagger/blob/dagger-2.32/java/dagger/hilt/android/internal/managers/ViewComponentManager.java Looks like the fix is not included to 2.32

wbervoets commented 3 years ago

Maybe they accidently forgot because this issue is already closed or are waiting for Androidx Fragment 1.3 final release?

marcoRS commented 3 years ago

Any updates on this bug? 2.33 doesn't appear to contain a fix.

bcorso commented 3 years ago

@marcoRS sorry, we ran into some issues rolling this fix forward internally. It's in now, but unfortunately it didn't make it into the 2.33 release by a few days so it's only in the HEAD snapshots.

wbervoets commented 3 years ago

any update on the eta for the 2.34 release? thanks

ghost commented 3 years ago

This issue is blocking our new release, any workaround here? thanks

osipxd commented 3 years ago

Looks like this issue resolved in 2.34 🎉

mpierucci commented 3 years ago

Looks like this issue resolved in 2.34 🎉

Just tried 2.34-beta with the project I created to raise the issue, and I´m still seeing the leak.

bcorso commented 3 years ago

@mpierucci, feel free to leave more details about your leak, and we can take a look.

Note that the particular fix (tested in FragmentContextWrapperLeakTest.java) only removes the Fragment reference we store in the context, but it doesn't prevent your view from leaking the fragment in other ways. For example, if the view in question is a Hilt view with fragment bindings (i.e., it's annotated with @AndroidEntryPoint @WithFragmentBindings) it will likely leak the fragment because there's a good chance the view or one of its injected dependencies will hold a reference to the FragmentComponent and Fragment. Unfortunately, there's nothing we can do about these leaks for you.

mpierucci commented 3 years ago

@bcorso Is the same project used to raised the issue originally. https://github.com/mpierucci/Hilt-Navigation-Leak/tree/master

Same steps:

Basically you just need to add @AndroidEntryPoint to a fragment that updates the toolbar ( as suggested by the official documentation) and you'll get a leak when navigating back and forth.

bcorso commented 3 years ago

@mpierucci, if you don't mind, can you post the results of leak canary with Dagger 2.34.

I'm assuming the leak is no longer due to the FragmentContextWrapper.fragment since we release that reference, so there must be something else we're holding onto that is still causing a leak.

mpierucci commented 3 years ago

Sure! This is what I have


│ GC Root: Local variable in native code
│
├─ android.os.HandlerThread instance
│    Leaking: NO (PathClassLoader↓ is not leaking)
│    Thread name: 'LeakCanary-Heap-Dump'
│    ↓ HandlerThread.contextClassLoader
├─ dalvik.system.PathClassLoader instance
│    Leaking: NO (InternalLeakCanary↓ is not leaking and A ClassLoader is never leaking)
│    ↓ PathClassLoader.runtimeInternalObjects
├─ java.lang.Object[] array
│    Leaking: NO (InternalLeakCanary↓ is not leaking)
│    ↓ Object[].[346]
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary.resumedActivity
├─ com.mpierucci.android.hiltmemleak.MainActivity instance
│    Leaking: NO (NavHostFragment↓ is not leaking and Activity#mDestroyed is false)
│    ↓ MainActivity.mFragments
├─ androidx.fragment.app.FragmentController instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentController.mHost
├─ androidx.fragment.app.FragmentActivity$HostCallbacks instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentActivity$HostCallbacks.mFragmentManager
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentManagerImpl.mPrimaryNav
├─ androidx.navigation.fragment.NavHostFragment instance
│    Leaking: NO (Fragment#mFragmentManager is not null)
│    ↓ NavHostFragment.mNavController
│                      ~~~~~~
├─ androidx.navigation.NavHostController instance
│    Leaking: UNKNOWN
│    ↓ NavHostController.mOnDestinationChangedListeners
│                        ~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    ↓ CopyOnWriteArrayList.array
│                           ~~~
├─ java.lang.Object[] array
│    Leaking: UNKNOWN
│    ↓ Object[].[0]
│               ~
├─ androidx.navigation.ui.ToolbarOnDestinationChangedListener instance
│    Leaking: UNKNOWN
│    ↓ ToolbarOnDestinationChangedListener.mContext
│                                          ~~~~
├─ dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper instance
│    Leaking: UNKNOWN
│    ViewComponentManager$FragmentContextWrapper wraps an Activity with Activity.mDestroyed false
│    ↓ ViewComponentManager$FragmentContextWrapper.baseInflater
│                                                  ~~~~
├─ com.android.internal.policy.PhoneLayoutInflater instance
│    Leaking: UNKNOWN
│    ↓ PhoneLayoutInflater.mFactory
│                          ~~~~
├─ android.view.LayoutInflater$FactoryMerger instance
│    Leaking: UNKNOWN
│    ↓ LayoutInflater$FactoryMerger.mF1
│                                   ~
├─ androidx.fragment.app.FragmentLayoutInflaterFactory instance
│    Leaking: UNKNOWN
│    ↓ FragmentLayoutInflaterFactory.mFragmentManager
│                                    ~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: UNKNOWN
│    ↓ FragmentManagerImpl.mOnAttachListeners
│                          ~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    ↓ CopyOnWriteArrayList.array
│                           ~~~
├─ java.lang.Object[] array
│    Leaking: UNKNOWN
│    ↓ Object[].[0]
│               ~
├─ androidx.fragment.app.FragmentManager$8 instance
│    Leaking: UNKNOWN
│    Anonymous class implementing androidx.fragment.app.FragmentOnAttachListener
│    ↓ FragmentManager$8.val$parent
│                        ~~~~
╰→ com.mpierucci.android.hiltmemleak.LeakFragment instance
​     Leaking: YES (ObjectWatcher was watching this because com.mpierucci.android.hiltmemleak.LeakFragment received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)```
bcorso commented 3 years ago

@mpierucci thanks! Looks like we'll need to release the inflator reference too. I'll add a fix for that.

mpierucci commented 3 years ago

@bcorso excellent thanks! Shall we re-open this issue then until the fix is released?

bcorso commented 3 years ago

Sounds good. Reopening until we fix the inflator leak.

bcorso commented 3 years ago

@mpierucci this should be fixed in Dagger 2.34.1 if you want to verify.

I ran through your instructions on the sample app a few times with the fix and didn't get any notification from LeakCanary, so hopefully this is fixed for you now.

mpierucci commented 3 years ago

@bcorso Can´t reproduce with 2.34.1 great job! 👏🏼

ducnv3012 commented 3 years ago

@bcorso It seems this issue is still unresolved. I´m still seeing the leak on Hilt version 2.36 My LeakCanary's log is below:

LeakCanary: ====================================
    HEAP ANALYSIS RESULT
    ====================================
    1 APPLICATION LEAKS
    References underlined with "~~~" are likely causes.
    Learn more at https://squ.re/leaks.
    252840 bytes retained by leaking objects
    ┬───
    │ GC Root: System class
    │
    ├─ android.app.ActivityThread class
    │    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
    │    ↓ static ActivityThread.sCurrentActivityThread
    ├─ android.app.ActivityThread instance
    │    Leaking: NO (MainActivity↓ is not leaking)
    │    ↓ ActivityThread.mActivities
    ├─ android.util.ArrayMap instance
    │    Leaking: NO (MainActivity↓ is not leaking)
    │    ↓ ArrayMap.mArray
    ├─ java.lang.Object[] array
    │    Leaking: NO (MainActivity↓ is not leaking)
    │    ↓ Object[].[1]
    ├─ android.app.ActivityThread$ActivityClientRecord instance
    │    Leaking: NO (MainActivity↓ is not leaking)
    │    ↓ ActivityThread$ActivityClientRecord.activity
    ├─ anime.comic.main.MainActivity instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking and Activity#mDestroyed is false)
    │    ↓ MainActivity.mActivityResultRegistry
    ├─ androidx.activity.ComponentActivity$2 instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    Anonymous subclass of androidx.activity.result.ActivityResultRegistry
    │    ↓ ComponentActivity$2.mKeyToCallback
    ├─ java.util.HashMap instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    ↓ HashMap.table
    ├─ java.util.HashMap$HashMapEntry[] array
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    ↓ HashMap$HashMapEntry[].[16]
    ├─ java.util.HashMap$HashMapEntry instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    ↓ HashMap$HashMapEntry.value
    ├─ androidx.activity.result.ActivityResultRegistry$CallbackAndContract instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    ↓ ActivityResultRegistry$CallbackAndContract.mCallback
    ├─ androidx.fragment.app.FragmentManager$9 instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    Anonymous class implementing androidx.activity.result.ActivityResultCallback
    │    ↓ FragmentManager$9.this$0
    ├─ androidx.fragment.app.FragmentManagerImpl instance
    │    Leaking: NO (GenreFragmentChild↓ is not leaking)
    │    ↓ FragmentManagerImpl.mParent
    ├─ anime.comic.ui.genre.fragments.GenreFragmentChild instance
    │    Leaking: NO (Fragment#mFragmentManager is not null)
    │    Fragment.mTag=f0
    │    ↓ GenreFragmentChild.mLifecycleRegistry
    │                         ~~~~~~~~~~~~~~~~~~
    ├─ androidx.lifecycle.LifecycleRegistry instance
    │    Leaking: UNKNOWN
    │    ↓ LifecycleRegistry.mInternalScopeRef
    │                        ~~~~~~~~~~~~~~~~~
    ├─ java.util.concurrent.atomic.AtomicReference instance
    │    Leaking: UNKNOWN
    │    ↓ AtomicReference.value
    │                      ~~~~~
    ├─ androidx.lifecycle.LifecycleCoroutineScopeImpl instance
    │    Leaking: UNKNOWN
    │    ↓ LifecycleCoroutineScopeImpl.coroutineContext
    │                                  ~~~~~~~~~~~~~~~~
    ├─ kotlin.coroutines.CombinedContext instance
    │    Leaking: UNKNOWN
    │    ↓ CombinedContext.left
    │                      ~~~~
    ├─ kotlinx.coroutines.SupervisorJobImpl instance
    │    Leaking: UNKNOWN
    │    ↓ SupervisorJobImpl._state
    │                        ~~~~~~
    ├─ kotlinx.coroutines.ChildHandleNode instance
    │    Leaking: UNKNOWN
    │    ↓ ChildHandleNode.childJob
    │                      ~~~~~~~~
    ├─ kotlinx.coroutines.StandaloneCoroutine instance
    │    Leaking: UNKNOWN
    │    ↓ StandaloneCoroutine._state
    │                          ~~~~~~
    ├─ kotlinx.coroutines.ChildHandleNode instance
    │    Leaking: UNKNOWN
    │    ↓ ChildHandleNode.childJob
    │                      ~~~~~~~~
    ├─ kotlinx.coroutines.internal.ScopeCoroutine instance
    │    Leaking: UNKNOWN
    │    ↓ ScopeCoroutine.uCont
    │                     ~~~~~
    ├─ anime.comic.ui.genre.fragments.GenreFragmentChild$onViewCreated$1$1$5 instance
    │    Leaking: UNKNOWN
    │    Anonymous subclass of kotlin.coroutines.jvm.internal.SuspendLambda
    │    ↓ GenreFragmentChild$onViewCreated$1$1$5.$pagingAdapter
    │                                             ~~~~~~~~~~~~~~
    ├─ anime.comic.ui.genre.adapter.GenreListAdapter instance
    │    Leaking: UNKNOWN
    │    ↓ GenreListAdapter.mObservable
    │                       ~~~~~~~~~~~
    ├─ androidx.recyclerview.widget.RecyclerView$AdapterDataObservable instance
    │    Leaking: UNKNOWN
    │    ↓ RecyclerView$AdapterDataObservable.mObservers
    │                                         ~~~~~~~~~~
    ├─ java.util.ArrayList instance
    │    Leaking: UNKNOWN
    │    ↓ ArrayList.elementData
    │                ~~~~~~~~~~~
    ├─ java.lang.Object[] array
    │    Leaking: UNKNOWN
    │    ↓ Object[].[0]
    │               ~~~
    ├─ androidx.recyclerview.widget.NestedAdapterWrapper$1 instance
    │    Leaking: UNKNOWN
    │    Anonymous subclass of androidx.recyclerview.widget.RecyclerView$AdapterDataObserver
    │    ↓ NestedAdapterWrapper$1.this$0
    │                             ~~~~~~
    ├─ androidx.recyclerview.widget.NestedAdapterWrapper instance
    │    Leaking: UNKNOWN
    │    ↓ NestedAdapterWrapper.mCallback
    │                           ~~~~~~~~~
    ├─ androidx.recyclerview.widget.ConcatAdapterController instance
    │    Leaking: UNKNOWN
    │    ↓ ConcatAdapterController.mBinderLookup
    │                              ~~~~~~~~~~~~~
    ├─ java.util.IdentityHashMap instance
    │    Leaking: UNKNOWN
    │    ↓ IdentityHashMap.table
    │                      ~~~~~
    ├─ java.lang.Object[] array
    │    Leaking: UNKNOWN
    │    ↓ Object[].[0]
    │               ~~~
    ├─ anime.comic.ui.genre.adapter.GenreListAdapter$GenreItemViewHolder instance
    │    Leaking: UNKNOWN
    │    ↓ GenreListAdapter$GenreItemViewHolder.mOwnerRecyclerView
    │                                           ~~~~~~~~~~~~~~~~~~
    ├─ androidx.recyclerview.widget.RecyclerView instance
    │    Leaking: YES (View detached and has parent)
    │    mContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping activity anime.comic.main.MainActivity with mDestroyed = false
    │    View#mParent is set
    │    View#mAttachInfo is null (view detached)
    │    View.mID = R.id.recycler_view
    │    View.mWindowAttachCount = 1
    │    ↓ RecyclerView.mParent
    ├─ androidx.swiperefreshlayout.widget.SwipeRefreshLayout instance
    │    Leaking: YES (RecyclerView↑ is leaking and View detached and has parent)
    │    mContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping activity anime.comic.main.MainActivity with mDestroyed = false
    │    View#mParent is set
    │    View#mAttachInfo is null (view detached)
    │    View.mID = R.id.swipe_refresh
    │    View.mWindowAttachCount = 1
    │    ↓ SwipeRefreshLayout.mParent
    ╰→ androidx.constraintlayout.widget.ConstraintLayout instance
    ​     Leaking: YES (ObjectWatcher was watching this because anime.comic.ui.genre.fragments.GenreFragmentChild received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks) and View detached and has parent)
    ​     key = f1202878-6b81-490b-99fd-8b19bdc4a7d2
    ​     watchDurationMillis = 70767
    ​     retainedDurationMillis = 65766
    ​     mContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping activity anime.comic.main.MainActivity with mDestroyed = false
    ​     View#mParent is set
    ​     View#mAttachInfo is null (view detached)
    ​     View.mWindowAttachCount = 1
    ====================================
bcorso commented 3 years ago

@ducnv3012 I see that the RecyclerView is leaking which happens to have a context wrapped in FragmentContextWrapper, but it doesn't seem like the FragmentContextWrapper itself is leaking anything.

Can you confirm this is a leak due to Hilt (e.g. by swapping the Hilt view with a normal view and verifying there is no leak).

ducnv3012 commented 3 years ago

@bcorso I swapped the Hilt view with a normal view and verifying there is no leak. But i found the cause because i was using Paging3 in my fragment. Thanks for your support

marcoRS commented 3 years ago

Is this finally fixed in the latest version? There is some instructions to enable flag -Adagger.hilt.android.useFragmentGetContextFix=true but I don't understand how to set this up? Is this suppose to be set in annotationProcessorOptions in build.gradle?

bcorso commented 3 years ago

@marcoRS this has been fixed since Dagger 2.34.1 (https://github.com/google/dagger/issues/2070#issuecomment-819048843).

salberin commented 2 years ago

Could it be that this issue is re-occuring? I'm seeing the following in my LeakCanary logs (running Hilt 2.40.5):

31587` bytes retained by leaking objects
    Signature: dd7d5f2ab5287e62a17fa9ab92b96d17e3a0af32
    ┬───
    │ GC Root: Thread object
    │
    ├─ android.net.ConnectivityThread instance
    │    Leaking: NO (PathClassLoader↓ is not leaking)
    │    Thread name: 'ConnectivityThread'
    │    ↓ Thread.contextClassLoader
    ├─ dalvik.system.PathClassLoader instance
    │    Leaking: NO (ToastEventListener↓ is not leaking and A ClassLoader is never leaking)
    │    ↓ ClassLoader.runtimeInternalObjects
    ├─ java.lang.Object[] array
    │    Leaking: NO (ToastEventListener↓ is not leaking)
    │    ↓ Object[4911]
    ├─ leakcanary.ToastEventListener class
    │    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
    │    ↓ static ToastEventListener.toastCurrentlyShown
    ├─ android.widget.Toast instance
    │    Leaking: NO (MainActivity↓ is not leaking)
    │    Retaining 12,1 kB in 170 objects
    │    mContext instance of nl.vanbreda.spanextgen.ui.MainActivity with mDestroyed = false
    │    ↓ Toast.mContext
    ├─ nl.vanbreda.spanextgen.ui.MainActivity instance
    │    Leaking: NO (BottomNavigationView↓ is not leaking and Activity#mDestroyed is false)
    │    mApplication instance of nl.vanbreda.spanextgen.SPAGlobal
    │    mBase instance of androidx.appcompat.view.ContextThemeWrapper
    │    ↓ MainActivity.binding
    ├─ nl.vanbreda.spanextgen.databinding.ActivityMainBinding instance
    │    Leaking: NO (BottomNavigationView↓ is not leaking)
    │    ↓ ActivityMainBinding.navigation
    ├─ com.google.android.material.bottomnavigation.BottomNavigationView instance
    │    Leaking: NO (View attached)
    │    View is part of a window view hierarchy
    │    View.mAttachInfo is not null (view attached)
    │    View.mID = R.id.navigation
    │    View.mWindowAttachCount = 1
    │    mContext instance of nl.vanbreda.spanextgen.ui.MainActivity with mDestroyed = false
    │    ↓ NavigationBarView.selectedListener
    │                        ~~~~~~~~~~~~~~~~
    ├─ androidx.navigation.ui.NavigationUI$5 instance
    │    Leaking: UNKNOWN
    │    Retaining 12 B in 1 objects
    │    Anonymous class implementing com.google.android.material.bottomnavigation.
    │    BottomNavigationView$OnNavigationItemSelectedListener
    │    ↓ NavigationUI$5.val$navController
    │                     ~~~~~~~~~~~~~~~~~
    ├─ androidx.navigation.NavHostController instance
    │    Leaking: UNKNOWN
    │    Retaining 39,7 kB in 834 objects
    │    mActivity instance of nl.vanbreda.spanextgen.ui.MainActivity with mDestroyed = false
    │    mContext instance of nl.vanbreda.spanextgen.ui.MainActivity with mDestroyed = false
    │    ↓ NavController.mOnDestinationChangedListeners
    │                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ├─ java.util.concurrent.CopyOnWriteArrayList instance
    │    Leaking: UNKNOWN
    │    Retaining 34,3 kB in 679 objects
    │    ↓ CopyOnWriteArrayList[1]
    │                          ~~~
    ├─ nl.vanbreda.spanextgen.ui.roaming.RoamingBlockFragment$$ExternalSyntheticLambda5 instance
    │    Leaking: UNKNOWN
    │    Retaining 12 B in 1 objects
    │    ↓ RoamingBlockFragment$$ExternalSyntheticLambda5.f$0
    │                                                     ~~~
    ╰→ nl.vanbreda.spanextgen.ui.roaming.RoamingBlockFragment instance
    ​     Leaking: YES (ObjectWatcher was watching this because nl.vanbreda.spanextgen.ui.roaming.RoamingBlockFragment
    ​     received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
    ​     Retaining 31,6 kB in 588 objects
    ​     key = 4c20fe38-0ad3-4498-b4a1-a62c88b61b61
    ​     watchDurationMillis = 126675
    ​     retainedDurationMillis = 121673
    ​     componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
    ​     wrapping activity nl.vanbreda.spanextgen.ui.MainActivity with mDestroyed = false
bcorso commented 2 years ago

@salberin I don't think that's an issue with Hilt.

In this case, it looks like the RoamingBlockFragment itself is leaking. This is something that you will need to fix on your side since it's not Hilt that is holding an instance of that fragment.

To clarify, the original problem was that a Hilt view was keeping an instance of the parent fragment. This meant that if the Hilt view outlived the fragment it would leak the fragment. That problem is now solved.

patrickpoe commented 2 years ago

@bcorso looks like the same issue still appears (latest version of Dagger/Hilt 2.41) when in the fragment's xml the android:theme attribute is applied:

====================================
HEAP ANALYSIS RESULT
====================================
1 APPLICATION LEAKS

References underlined with "~~~" are likely causes.
Learn more at https://squ.re/leaks.

2371 bytes retained by leaking objects
Signature: 1a0ede954e6f91469419c087a7a63d305942c8e3
┬───
│ 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[1464]
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary.resumedActivity
├─ com.mpierucci.android.hiltmemleak.MainActivity instance
│    Leaking: NO (NavHostFragment↓ is not leaking and Activity#mDestroyed is false)
│    mApplication instance of com.mpierucci.android.hiltmemleak.App
│    mBase instance of androidx.appcompat.view.ContextThemeWrapper
│    ↓ FragmentActivity.mFragments
├─ androidx.fragment.app.FragmentController instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentController.mHost
├─ androidx.fragment.app.FragmentActivity$HostCallbacks instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    this$0 instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    mActivity instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    mContext instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    ↓ FragmentHostCallback.mFragmentManager
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: NO (NavHostFragment↓ is not leaking)
│    ↓ FragmentManager.mPrimaryNav
├─ androidx.navigation.fragment.NavHostFragment instance
│    Leaking: NO (Fragment#mFragmentManager is not null)
│    ↓ NavHostFragment.navHostController
│                      ~~~~~~~~~~~~~~~~~
├─ androidx.navigation.NavHostController instance
│    Leaking: UNKNOWN
│    Retaining 11.1 kB in 351 objects
│    activity instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    context instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    ↓ NavController.onDestinationChangedListeners
│                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 5.0 kB in 168 objects
│    ↓ CopyOnWriteArrayList[1]
│                          ~~~
├─ androidx.navigation.ui.ToolbarOnDestinationChangedListener instance
│    Leaking: UNKNOWN
│    Retaining 4.5 kB in 153 objects
│    context instance of android.view.ContextThemeWrapper, wrapping activity com.mpierucci.android.hiltmemleak.
│    MainActivity with mDestroyed = false
│    ↓ AbstractAppBarOnDestinationChangedListener.context
│                                                 ~~~~~~~
├─ android.view.ContextThemeWrapper instance
│    Leaking: UNKNOWN
│    Retaining 4.0 kB in 142 objects
│    mBase instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping
│    activity com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    ContextThemeWrapper wraps an Activity with Activity.mDestroyed false
│    ↓ ContextThemeWrapper.mInflater
│                          ~~~~~~~~~
├─ com.android.internal.policy.PhoneLayoutInflater instance
│    Leaking: UNKNOWN
│    Retaining 3.9 kB in 134 objects
│    mContext instance of android.view.ContextThemeWrapper, wrapping activity com.mpierucci.android.hiltmemleak.
│    MainActivity with mDestroyed = false
│    mPrivateFactory instance of com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
│    ↓ LayoutInflater.mFactory
│                     ~~~~~~~~
├─ android.view.LayoutInflater$FactoryMerger instance
│    Leaking: UNKNOWN
│    Retaining 3.8 kB in 132 objects
│    ↓ LayoutInflater$FactoryMerger.mF1
│                                   ~~~
├─ androidx.fragment.app.FragmentLayoutInflaterFactory instance
│    Leaking: UNKNOWN
│    Retaining 3.8 kB in 131 objects
│    ↓ FragmentLayoutInflaterFactory.mFragmentManager
│                                    ~~~~~~~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: UNKNOWN
│    Retaining 3.8 kB in 130 objects
│    ↓ FragmentManager.mOnAttachListeners
│                      ~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 2.4 kB in 85 objects
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ androidx.fragment.app.FragmentManager$6 instance
│    Leaking: UNKNOWN
│    Retaining 2.4 kB in 82 objects
│    Anonymous class implementing androidx.fragment.app.FragmentOnAttachListener
│    ↓ FragmentManager$6.val$parent
│                        ~~~~~~~~~~
╰→ com.mpierucci.android.hiltmemleak.LeakFragment instance
​     Leaking: YES (ObjectWatcher was watching this because com.mpierucci.android.hiltmemleak.LeakFragment received
​     Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
​     Retaining 2.4 kB in 81 objects
​     key = c3c0df67-4b20-4408-bc2d-ea601f99c6eb
​     watchDurationMillis = 467898
​     retainedDurationMillis = 462897
​     componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
​     wrapping activity com.mpierucci.android.hiltmemleak.MainActivity with mDestroyed = false
====================================
0 LIBRARY LEAKS

A Library Leak is a leak caused by a known bug in 3rd party code that you do not have control over.
See https://square.github.io/leakcanary/fundamentals-how-leakcanary-works/#4-categorizing-leaks
====================================
0 UNREACHABLE OBJECTS

An unreachable object is still in memory but LeakCanary could not find a strong reference path
from GC roots.
====================================
METADATA

Please include this in bug reports and Stack Overflow questions.

Build.VERSION.SDK_INT: 30
Build.MANUFACTURER: Google
LeakCanary version: 2.8.1
App process name: com.mpierucci.android.hiltmemleak
Stats: LruCache[maxSize=3000,hits=35307,misses=91490,hitRate=27%]
RandomAccess[bytes=4339230,reads=91490,travel=23627513197,range=18828077,size=24633446]
Analysis duration: 19601 ms
Heap dump file path: /storage/emulated/0/Download/leakcanary-com.mpierucci.android.hiltmemleak/2022-02-28_11-40-48_585.
hprof
Heap dump timestamp: 1646044873444
Heap dump duration: Unknown
====================================

Here some maybe useful screenshots of the heap:

With the android:theme attribute applied: image image

With out the android:theme attribute applied: image No references to LeakFragment, hence no leak.

Is there anything we can do about this?

bcorso commented 2 years ago

@patrickpoe I think this is a different issue. The trace suggests that while Hilt's FragmentContextWrapper is being leaked, it's not leaking anything itself. Thus, I don't think there's anything we can do on the Hilt side to fix your issue.

In particular, I'm looking at the direct path of the leak, which doesn't include Hilt:

...
├─ com.mpierucci.android.hiltmemleak.MainActivity instance
│    ↓ FragmentActivity.mFragments
│                       ~~~~~~~~~~
├─ androidx.fragment.app.FragmentController instance
│    ↓ FragmentController.mHost
│                         ~~~~~
├─ androidx.fragment.app.FragmentActivity$HostCallbacks instance
│    ↓ FragmentHostCallback.mFragmentManager
│                           ~~~~~~~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│    ↓ FragmentManager.mPrimaryNav
│                      ~~~~~~~~~~~
├─ androidx.navigation.fragment.NavHostFragment instance
│    ↓ NavHostFragment.navHostController
│                      ~~~~~~~~~~~~~~~~~
├─ androidx.navigation.NavHostController instance
│    ↓ NavController.onDestinationChangedListeners
│                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    ↓ CopyOnWriteArrayList[1]
│                          ~~~
├─ androidx.navigation.ui.ToolbarOnDestinationChangedListener instance
│    ↓ AbstractAppBarOnDestinationChangedListener.context
│                                                 ~~~~~~~
├─ android.view.ContextThemeWrapper instance
│    ↓ ContextThemeWrapper.mInflater
│                          ~~~~~~~~~
├─ com.android.internal.policy.PhoneLayoutInflater instance
│    ↓ LayoutInflater.mFactory
│                     ~~~~~~~~
├─ android.view.LayoutInflater$FactoryMerger instance
│    ↓ LayoutInflater$FactoryMerger.mF1
│                                   ~~~
├─ androidx.fragment.app.FragmentLayoutInflaterFactory instance
│    ↓ FragmentLayoutInflaterFactory.mFragmentManager
│                                    ~~~~~~~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│    ↓ FragmentManager.mOnAttachListeners
│                      ~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ androidx.fragment.app.FragmentManager$6 instance
│    ↓ FragmentManager$6.val$parent
│                        ~~~~~~~~~~
╰→ com.mpierucci.android.hiltmemleak.LeakFragment instance
patrickpoe commented 2 years ago

@bcorso thanks a lot for your quick answer!

You are right, but it just happens in the LeakFragment (marked with @AndroidEnryPoint + setupWithNavController called) and not in the other 2 fragments thats why i thought it has to be somehow related to Hilt.

I also forgot to mention, that it just happens after i accessed the LayoutInflater from a view context.

I quickly forked the sample project and if you like you can check it out: https://github.com/patrickpoe/Hilt-Navigation-Leak/commit/39def957bef5750cff4f622eaf12cb90be0d06c3

bcorso commented 2 years ago

Interesting, thanks for the sample project! I'll try to take a look sometime this week.

ox-grom commented 2 years ago

I have the same issue with 2.41. Without @AndroidEntryPoint problem disappears.

componentContext instance of dagger.hilt.android.internal.managers. ViewComponentManager$FragmentContextWrapper, wrapping activity

┬───
│ 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[49]
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary.resumedActivity
├─ ru.example.presentation.ui.MainActivity instance
│    Leaking: NO (Activity#mDestroyed is false)
│    mApplication instance of ru.example.presentation.GumApp
│    mBase instance of androidx.appcompat.view.ContextThemeWrapper
│    ↓ MainActivity.navController
│                   ~~~~~~~~~~~~~
├─ androidx.navigation.NavHostController instance
│    Leaking: UNKNOWN
│    Retaining 118,2 kB in 3209 objects
│    activity instance of ru.example.presentation.ui.MainActivity with
│    mDestroyed = false
│    context instance of ru.example.presentation.ui.MainActivity with
│    mDestroyed = false
│    ↓ NavController.onDestinationChangedListeners
│                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 15,5 kB in 580 objects
│    ↓ CopyOnWriteArrayList[2]
│                          ~~~
├─ androidx.navigation.ui.ToolbarOnDestinationChangedListener instance
│    Leaking: UNKNOWN
│    Retaining 4,6 kB in 170 objects
│    context instance of androidx.appcompat.view.ContextThemeWrapper, wrapping
│    activity ru.example.presentation.ui.MainActivity with mDestroyed =
│    false
│    ↓ AbstractAppBarOnDestinationChangedListener.context
│                                                 ~~~~~~~
├─ androidx.appcompat.view.ContextThemeWrapper instance
│    Leaking: UNKNOWN
│    Retaining 4,3 kB in 155 objects
│    mBase instance of android.view.ContextThemeWrapper, wrapping activity ru.
│    example.presentation.ui.MainActivity with mDestroyed = false
│    ContextThemeWrapper wraps an Activity with Activity.mDestroyed false
│    ↓ ContextThemeWrapper.mInflater
│                          ~~~~~~~~~
├─ com.android.internal.policy.PhoneLayoutInflater instance
│    Leaking: UNKNOWN
│    Retaining 54 B in 2 objects
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping
│    activity ru.example.presentation.ui.MainActivity with mDestroyed =
│    false
│    mPrivateFactory instance of ru.example.presentation.ui.MainActivity
│    with mDestroyed = false
│    ↓ LayoutInflater.mFactory
│                     ~~~~~~~~
├─ android.view.LayoutInflater$FactoryMerger instance
│    Leaking: UNKNOWN
│    Retaining 3,8 kB in 135 objects
│    ↓ LayoutInflater$FactoryMerger.mF1
│                                   ~~~
├─ androidx.fragment.app.FragmentLayoutInflaterFactory instance
│    Leaking: UNKNOWN
│    Retaining 3,8 kB in 134 objects
│    ↓ FragmentLayoutInflaterFactory.mFragmentManager
│                                    ~~~~~~~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: UNKNOWN
│    Retaining 3,8 kB in 133 objects
│    ↓ FragmentManager.mOnAttachListeners
│                      ~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 2,5 kB in 88 objects
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ androidx.fragment.app.FragmentManager$6 instance
│    Leaking: UNKNOWN
│    Retaining 2,4 kB in 85 objects
│    Anonymous class implementing androidx.fragment.app.FragmentOnAttachListener
│    ↓ FragmentManager$6.val$parent
│                        ~~~~~~~~~~
╰→ ru.example.presentation.ui.news.News instance
      Leaking: YES (ObjectWatcher was watching this because ru.example.
      presentation.ui.news.News received Fragment#onDestroy() callback and
      Fragment#mFragmentManager is null)
      Retaining 2,4 kB in 84 objects
      key = b2d22816-0309-4475-97c8-24b1d5062df4
      watchDurationMillis = 12858
      retainedDurationMillis = 7858
      componentContext instance of dagger.hilt.android.internal.managers.
      ViewComponentManager$FragmentContextWrapper, wrapping activity 
      ru.example.presentation.ui.MainActivity with mDestroyed = false

METADATA

Build.VERSION.SDK_INT: 31
Build.MANUFACTURER: Google
LeakCanary version: 2.8.1
App process name: ru.example.dev
Stats: LruCache[maxSize=3000,hits=114540,misses=213615,hitRate=34%]
RandomAccess[bytes=10551460,reads=213615,travel=65862812471,range=33862972,size=
41353108]
Analysis duration: 68029 ms
patrickpoe commented 2 years ago

@bcorso did you have a chance to look into that issue already? did you find anything?

bcorso commented 2 years ago

Yes, I was able to look into this, but unfortunately I wasn't able to track down the root cause of the issue.

I can confirm that it only happens with the Hilt fragment and only when using navigation, but the cause of the leak really doesn't make sense to me. From your example project the leak is caused here:

@AndroidEntryPoint
class LeakFragment : Fragment(R.layout.leaky_fragment) {
    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
       ...
        // leak caused by this line
        LayoutInflater.from(view.context)
    }
}

However that line doesn't seem to store any obvious state so it's not clear to me what's actually causing the leak. Also, as I mentioned before, the leak doesn't actually point to anything contained within a Hilt class, it just states that the object leaked contains the context which is wrapped by Hilt, so it's not clear what, if anything, we could do to actually fix this leak.

Sorry, I wish I had a better answer.

patrickpoe commented 2 years ago

Alright, thank you - looks like we came to the same conclusion.

Really curious what the root cause here is, anyways pls just let me know in case you find something interesting.

Lowae commented 2 years ago

I have the same issue with 2.42.

componentContext instance of dagger.hilt.android.internal.managers. ViewComponentManager$FragmentContextWrapper, wrapping activity with mDestroyed = false

Source code you can see SearchListFragment

    ====================================
    1 APPLICATION LEAKS

    References underlined with "~~~" are likely causes.
    Learn more at https://squ.re/leaks.

    3395272 bytes retained by leaking objects
    Signature: e8b38bddb49d90e713aca0f0c231f5be997a7552
    ┬───
    │ GC Root: Input or output parameters 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[4132]
    ├─ leakcanary.internal.InternalLeakCanary class
    │    Leaking: NO (SearchActivity↓ is not leaking and a class is never leaking)
    │    ↓ static InternalLeakCanary.resumedActivity
    ├─ com.lowe.wanandroid.ui.search.SearchActivity instance
    │    Leaking: NO (Activity#mDestroyed is false)
    │    mApplication instance of com.lowe.wanandroid.base.app.BaseApp
    │    mBase instance of androidx.appcompat.view.ContextThemeWrapper
    │    ↓ SearchActivity.searchListFragment
    │                     ~~~~~~~~~~~~~~~~~~
    ╰→ com.lowe.wanandroid.ui.search.result.SearchListFragment instance
    ​     Leaking: YES (ObjectWatcher was watching this because com.lowe.wanandroid.ui.search.result.SearchListFragment
    ​     received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
    ​     Retaining 3.4 MB in 60683 objects
    ​     key = dacff5cd-264b-455a-b482-6b7733968547
    ​     watchDurationMillis = 357
    ​     retainedDurationMillis = -1
    ​     key = ef6fdeaa-3e4a-441b-84d1-8f88ca991f54
    ​     watchDurationMillis = 1264
    ​     key = df38dc21-1cc2-471e-b89c-397538ca348c
    ​     watchDurationMillis = 5449
    ​     retainedDurationMillis = 409
    ​     key = f09436b5-0175-43bb-96c0-2c7317b823f8
    ​     watchDurationMillis = 6374
    ​     retainedDurationMillis = 1374
    ​     key = d4f9cb79-c9f0-4393-9c0f-98c84584e7ae
    ​     watchDurationMillis = 7792
    ​     retainedDurationMillis = 2792
    ​     componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
    ​     wrapping activity com.lowe.wanandroid.ui.search.SearchActivity with mDestroyed = false
    ====================================
kaviskhandelwal commented 1 year ago

@bcorso I have the same issue with version 2.40.

mBase instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping activity com.example.main.HomeActivity with mDestroyed = true

┬───

│ GC Root: System class
│
├─ com.mediatek.boostfwk.identify.scroll.ScrollIdentify class
│    Leaking: NO (a class is never leaking)
│    ↓ static ScrollIdentify.mContext
│                            ~~~~~~~~
├─ dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper instance
│    Leaking: YES (ViewComponentManager$FragmentContextWrapper wraps an Activity with Activity.mDestroyed true)
│    Retaining 1.8 MB in 40025 objects
│    mBase instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping activity com.example.main.HomeActivity with mDestroyed = true
│    ↓ ContextWrapper.mBase
├─ dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper instance
│    Leaking: YES (ViewComponentManager$FragmentContextWrapper↑ is leaking and ViewComponentManager$FragmentContextWrapper wraps an Activity with Activity.mDestroyed true)
│    Retaining 1.8 MB in 40023 objects
│    mBase instance of com.example.main.HomeActivity with mDestroyed = true
│    ↓ ContextWrapper.mBase
╰→ com.example.main.HomeActivity instance
​     Leaking: YES (ObjectWatcher was watching this because com.example.main.HomeActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
​     Retaining 1.8 MB in 40021 objects
​     key = d6fa8f68-a672-40b8-bca9-1af9502de2e7
​     watchDurationMillis = 19402
​     retainedDurationMillis = 14401
​     app instance of com.example.main.SupplyApplication
​     moduleInteractor instance of com.example.main.SupplyApplication
​     mApplication instance of comexample.main.SupplyApplication
​     mBase instance of androidx.appcompat.view.ContextThemeWrapper
bcorso commented 1 year ago

@kaviskhandelwal, do you own the ScrollIdentity class?

The ScrollIdentity object is storing the context in a static field, which I think is the main issue for the leak in this case. Holding any activity-level context in static state will eventually leak the activity once the activity is destroyed. You either need to avoid storing the context in a static field or you need to make sure you clear the static field once the activity is destroyed (e.g. by setting up a LifecycleEventObserver).

AFAIK, I don't think there's much we could do on the Hilt side. The FragmentContextWrapper extends ContextWrapper, which necessarily takes in the base context in the super() constructor and stores it. Thus, we don't have any way to clear mBase instance ourselves since it's not a protected field, and there aren't any public APIs to do so.