android / architecture-samples

A collection of samples to discuss and showcase different architectural tools and patterns for Android apps.
Apache License 2.0
44.37k stars 11.62k forks source link

Memory leak in TasksFragment #714

Open hijamoya opened 4 years ago

hijamoya commented 4 years ago

Detected leak path by LeakCanary:

┬───
│ 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 (RecyclerView↓ is not leaking and View attached)
│    mContext instance of com.android.internal.policy.DecorContext, wrapping activity com.example.android.architecture.blueprints.todoapp.tasks.TasksActivity 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.mAttachInfo
├─ android.view.View$AttachInfo instance
│    Leaking: NO (RecyclerView↓ is not leaking)
│    ↓ View$AttachInfo.mScrollContainers
├─ java.util.ArrayList instance
│    Leaking: NO (RecyclerView↓ is not leaking)
│    ↓ ArrayList.elementData
├─ java.lang.Object[] array
│    Leaking: NO (RecyclerView↓ is not leaking)
│    ↓ Object[].[1]
├─ androidx.recyclerview.widget.RecyclerView instance
│    Leaking: NO (View attached)
│    mContext instance of com.example.android.architecture.blueprints.todoapp.tasks.TasksActivity with mDestroyed = false
│    View.parent android.widget.LinearLayout attached as well
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mID = R.id.tasks_list
│    View.mWindowAttachCount = 1
│    ↓ RecyclerView.mAdapter
│                   ~~~~~~~~
├─ com.example.android.architecture.blueprints.todoapp.tasks.TasksAdapter instance
│    Leaking: UNKNOWN
│    ↓ TasksAdapter.viewModel
│                   ~~~~~~~~~
├─ com.example.android.architecture.blueprints.todoapp.tasks.TasksViewModel instance
│    Leaking: UNKNOWN
│    ↓ TasksViewModel._snackbarText
│                     ~~~~~~~~~~~~~
├─ androidx.lifecycle.MutableLiveData instance
│    Leaking: UNKNOWN
│    ↓ MutableLiveData.mObservers
│                      ~~~~~~~~~~
├─ androidx.arch.core.internal.SafeIterableMap instance
│    Leaking: UNKNOWN
│    ↓ SafeIterableMap.mStart
│                      ~~~~~~
├─ androidx.arch.core.internal.SafeIterableMap$Entry instance
│    Leaking: UNKNOWN
│    ↓ SafeIterableMap$Entry.mNext
│                            ~~~~~
├─ androidx.arch.core.internal.SafeIterableMap$Entry instance
│    Leaking: UNKNOWN
│    ↓ SafeIterableMap$Entry.mKey
│                            ~~~~
├─ com.example.android.architecture.blueprints.todoapp.util.ViewExtKt$setupSnackbar$1 instance
│    Leaking: UNKNOWN
│    Anonymous class implementing androidx.lifecycle.Observer
│    ↓ ViewExtKt$setupSnackbar$1.$this_setupSnackbar
│                                ~~~~~~~~~~~~~~~~~~~
╰→ androidx.coordinatorlayout.widget.CoordinatorLayout instance
​     Leaking: YES (ObjectWatcher was watching this because com.example.android.architecture.blueprints.todoapp.tasks.TasksFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks))
​     key = 080d1590-6d2b-47da-8b64-4be379e522aa
​     watchDurationMillis = 8201
​     retainedDurationMillis = 3178
​     key = 4e988abe-ac80-43b1-8004-e3c84d5d8bcd
​     watchDurationMillis = 8205
​     retainedDurationMillis = 3179
​     mContext instance of com.example.android.architecture.blueprints.todoapp.tasks.TasksActivity with mDestroyed = false
​     View#mParent is null
​     View#mAttachInfo is null (view detached)
​     View.mID = R.id.coordinator_layout
​     View.mWindowAttachCount = 1

The reason is that the sample keeps a lateinit listAdapter in TasksFragment. When we navigate to AddEditTaskFragment, the RecyclerView destroyed but the adapter is still keep the reference of it which causes the leak.

Do you have any suggestion to fix this? Thanks!

rajBopche commented 4 years ago

If this is the case, then in the onDestroyView() method of TasksFragment we can set the viewDataBinding.tasksList.adapter = null. Gotta still test to make sure nothing breaks 💯

gs-ts commented 3 years ago

is there any more elegant way to fix this memory leak?