BottomSheetDialogFragment shows a memory leak, in LeakCanary 2, but I'm not sure why?

Question

I've been looking through a number of posts and I have not found anything to help me understand why LeakCanary is reporting a leak. I have a main activity with a com.google.android.material.bottomappbar.BottomAppBar and that shows a BottomSheetDialogFragment. When you select an item in the bottom sheet it updates some text then dismisses the dialog fragment. Now running this with LeakCanary shows a leak when the dialog is presented and an item is selected.

The leak looks like:

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

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

    152090 bytes retained by leaking objects
    Signature: 3841703253a9bf9893936b1dd318c9dd54bf5a8
    ┬───
    │ 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
    ├─ com.example.testleak.MainActivity instance
    │    Leaking: NO (Activity#mDestroyed is false)
    │    ↓ MainActivity.bottomNavFragment
    │                   ~~~~~~~~~~~~~~~~~
    ╰→ com.example.testleak.BottomNavigationDrawerFragment instance
    ​     Leaking: YES (ObjectWatcher was watching this because com.example.testleak.BottomNavigationDrawerFragment received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
    ​     key = 74db5e32-a816-418a-9f83-2f50a05f37a4
    ​     watchDurationMillis = 7224
    ​     retainedDurationMillis = 2210
    ​     key = 92744a08-2122-4fbe-9841-08f805fcf6e5
    ​     retainedDurationMillis = 2212
    ====================================
    0 LIBRARY LEAKS

    Library Leaks are leaks coming from the Android Framework or Google libraries.
    ====================================
    METADATA

    Please include this in bug reports and Stack Overflow questions.

    Build.VERSION.SDK_INT: 26
    Build.MANUFACTURER: motorola
    LeakCanary version: 2.1
    App process name: com.example.testleak
    Analysis duration: 4182 ms
    Heap dump file path: /data/user/0/com.example.testleak/files/leakcanary/2020-02-20_10-42-59_515.hprof
    Heap dump timestamp: 1582213384806
    ====================================

So it appears that BottomNavigationDrawerFragment should be cleaning up something in it's onDestroy() method.

Here are the main files involved.

MainActivity.kt

package com.example.testleak

import android.os.Bundle
import android.view.Menu
import android.view.MenuItem
import androidx.appcompat.app.AppCompatActivity
import com.example.testleak.BottomNavigationDrawerFragment.OnItemClickListener
import kotlinx.android.synthetic.main.activity_main.*
import kotlinx.android.synthetic.main.content_main.*

class MainActivity : AppCompatActivity() {
    private var bottomNavFragment: BottomNavigationDrawerFragment? = null

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)

        setSupportActionBar(bottom_app_bar)
        bottomNavFragment = BottomNavigationDrawerFragment(clickListener)
    }

    override fun onCreateOptionsMenu(menu: Menu): Boolean {
        return true
    }

    private var clickListener = object : OnItemClickListener {
        override fun onItemClick(item: Int?) {
            // Based on the item clicked show that fragment
            bottomNavFragment?.dismiss()
            when (item) {
                R.id.nav_item_1 -> {
                    screen_label.text = resources.getString(R.string.item_1)
                }
                R.id.nav_item_2 -> {
                    screen_label.text = resources.getString(R.string.item_2)
                }
                R.id.nav_item_3 -> {
                    screen_label.text = resources.getString(R.string.item_3)
                }
                R.id.nav_item_4 -> {
                    screen_label.text = resources.getString(R.string.item_4)
                }
                R.id.preferences -> {
                    screen_label.text = resources.getString(R.string.preferences)
                }
            }
        }
    }


    override fun onOptionsItemSelected(item: MenuItem?): Boolean {
        when (item!!.itemId) {
            android.R.id.home -> {
                bottomNavFragment!!.show(supportFragmentManager, bottomNavFragment!!.tag)
            }
        }
        return true
    }
}

activity_main.xml

<?xml version="1.0" encoding="utf-8"?>
<androidx.coordinatorlayout.widget.CoordinatorLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:id="@+id/coordinatorLayout"
    android:layout_width="match_parent"
    android:layout_height="match_parent">

    <include layout="@layout/content_main" />

    <com.google.android.material.bottomappbar.BottomAppBar
        android:id="@+id/bottom_app_bar"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_gravity="bottom"
        app:backgroundTint="?attr/colorPrimary"
        app:fabAlignmentMode="center"
        app:hideOnScroll="true"
        app:layout_scrollFlags="scroll|enterAlways"
        app:navigationIcon="@drawable/ic_menu_white_24dp"/>

    <com.google.android.material.floatingactionbutton.FloatingActionButton
        android:id="@+id/fab"
        style="@style/Widget.MaterialComponents.FloatingActionButton"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:src="@drawable/ic_add_circle_outline_white_24dp"
        app:layout_anchor="@id/bottom_app_bar" />

</androidx.coordinatorlayout.widget.CoordinatorLayout>

content_main.xml

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    android:orientation="vertical"
    android:layout_width="match_parent"
    android:layout_height="match_parent">
    <androidx.coordinatorlayout.widget.CoordinatorLayout
        android:id="@+id/coordinatorLayout2"
        android:layout_width="match_parent"
        android:layout_height="wrap_content" >

        <androidx.constraintlayout.widget.ConstraintLayout
            android:id="@+id/constraintLayout"
            android:layout_width="match_parent"
            android:layout_height="wrap_content">

            <TextView
                android:id="@+id/screen_label"
                android:layout_width="wrap_content"
                android:layout_height="wrap_content"
                android:layout_marginTop="120dp"
                android:text="Primary"
                android:textSize="18sp"
                android:textStyle="bold"
                app:layout_constraintBottom_toBottomOf="parent"
                app:layout_constraintEnd_toEndOf="parent"
                app:layout_constraintStart_toStartOf="parent"
                app:layout_constraintTop_toTopOf="parent" />

        </androidx.constraintlayout.widget.ConstraintLayout>

    </androidx.coordinatorlayout.widget.CoordinatorLayout>

</LinearLayout>

BottomNavigationDrawerFragment.kt

package com.example.testleak

import android.os.Bundle
import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import com.google.android.material.bottomsheet.BottomSheetDialogFragment
import com.google.android.material.navigation.NavigationView

class BottomNavigationDrawerFragment(private val clickListener: OnItemClickListener) : BottomSheetDialogFragment() {

    lateinit var mapNavView: NavigationView

    interface OnItemClickListener {
        fun onItemClick(item: Int?)
    }

    override fun onDestroy() {
        Log.d("BottomNavFragment", "onDestroy")
        super.onDestroy()
    }

    override fun onDestroyView() {
        Log.d("BottomNavFragment", "onDestroyView")
        super.onDestroyView()
    }

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        Log.d("BottomNavFragment", "onCreateView")
        val v = inflater.inflate(R.layout.fragment_bottomsheet, container, false)
        mapNavView = v.findViewById(R.id.nav_view)
        mapNavView.setNavigationItemSelectedListener { menuItem ->
            // Bottom Navigation Drawer menu item clicks
            clickListener.onItemClick(menuItem.itemId)
            true
        }
        return v
    }

}

fragment_bottomsheet.xml

<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:app="http://schemas.android.com/apk/res-auto"
  xmlns:android="http://schemas.android.com/apk/res/android"
  android:fitsSystemWindows="true"
  android:layout_width="match_parent"
  android:layout_height="wrap_content">


  <com.google.android.material.navigation.NavigationView
    android:id="@+id/nav_view"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_gravity="bottom"
    app:layout_constraintEnd_toEndOf="parent"
    app:layout_constraintStart_toStartOf="parent"
    app:layout_constraintTop_toTopOf="parent"
    app:menu="@menu/bottom_nav_drawer_menu" />

</androidx.constraintlayout.widget.ConstraintLayout>

bottom_nav_drawer_menu.xml

<?xml version="1.0" encoding="utf-8"?>
<menu xmlns:android="http://schemas.android.com/apk/res/android">
  <group android:checkableBehavior="none">
    <item
      android:id="@+id/nav_item_1"
      android:title="@string/item_1" />
    <item
      android:id="@+id/nav_item_2"
      android:title="@string/item_2" />
    <item
      android:id="@+id/nav_item_3"
      android:title="@string/item_3" />
    <item
      android:id="@+id/nav_item_4"
      android:title="@string/item_4" />
    <item
      android:id="@+id/preferences"
      android:title="@string/preferences" />
  </group>
</menu>

Any help would be greatly appreciated. I'm sure there is something I'm overlooking but I'm guessing I've looked at it so long I'm looking past it.

-Rindress

Answer 1

I am not sure if this is still needed but there are two things I realised:

1. You are initialising the fragment with an argument, this will lead to crashes in your users phones, you might not see that yourself, and if you see it crashing you won't understand why because the crash message is very not clear unless you are expecting this crash to happen. This crash happens when for example you try to rotate the phone or do anything that does not allow the UI to survive, your data in liveData objs will survive but the UI won't and your app will try to remake the BottomNavFragment and won't know what to pass to the argument, so it will lead to unexpected behaviour could be crashing or accessing not initialised latinit variable in kotlin.
2. I think your app is having MemoryLeak because you are not calling dismiss anywhere that is guaranteed to get executed. From what I can realise in your MainActivity, you called the dismiss only inside the onClickListener, am I right? What if the item that is being listened to when clicked it never clicked? The dismiss will never be called.

Solution: Don't make an instance of the BottomNavFragment, just create an instance without a name and handle button clicks inside the class of the fragment, then make inform the activity back. For how to communicate between the activity and fragment, please check https://developer.android.com/guide/fragments/communicate

Answer 2

The key part of the leaktrace to look at is where the ~~~ are :

├─ com.example.testleak.MainActivity instance
│    Leaking: NO (Activity#mDestroyed is false)
│    ↓ MainActivity.bottomNavFragment
│                   ~~~~~~~~~~~~~~~~~
╰→ com.example.testleak.BottomNavigationDrawerFragment instance
​     Leaking: YES (ObjectWatcher was watching this because
com.example.testleak.BottomNavigationDrawerFragment received Fragment#onDestroy() callback
and Fragment#mFragmentManager is null)

This tells us that MainActivity is not destroyed, but BottomNavigationDrawerFragment is. When BottomNavigationDrawerFragment becomes destroyed, it should be garbage collected. However it cannot be garbage collected because MainActivity is keeping a reference to it in MainActivity.bottomNavFragment

When MainActivity.clickListener calls bottomNavFragment?.dismiss() it should also set bottomNavFragment to null. And instead of setting MainActivity.bottomNavFragment to a new instance in MainActivity.onCreate(), the new instance should be created when the fragment is shown, e.g. in MainActivity.onOptionsItemSelected