Status Update
Comments
th...@gmail.com <th...@gmail.com> #2
Any updates on this issue? I found many more memory leaks and would like to report them but will someone look into them?
ap...@google.com <ap...@google.com> #4
Branch: androidx-master-dev
commit 8d940181071bf7c68448ae43e3cb6b931fe43970
Author: Dake Gu <dake@google.com>
Date: Wed Sep 23 16:35:33 2020
leanback: fix View leak in GuidanceStylist
Bug: 164841457
Test: sample still runs
Change-Id: Ia14415f12bf8b51b078b9a30b3eebfbe8a7961c2
M leanback/leanback/src/main/java/androidx/leanback/widget/GuidanceStylist.java
th...@gmail.com <th...@gmail.com> #5
Thanks for fixing the leak! There are many more memory leaks in leanback. I am using the following file as a workaround for the memory leaks. (call MemoryLeakFixes.onCreate(this)
in Application.onCreate
). Could these be fixed as well? If you need more details about a specific memory leak workaround let me know.
package com.superthomaslab.hueessentials
import android.app.Activity
import android.app.Application
import android.os.Bundle
import android.transition.Transition
import android.view.View
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentActivity
import androidx.fragment.app.FragmentManager
import androidx.leanback.app.BaseSupportFragment
import androidx.leanback.app.BrandedSupportFragment
import androidx.leanback.app.BrowseSupportFragment
import androidx.leanback.app.DetailsSupportFragment
import androidx.leanback.app.GuidedStepSupportFragment
import androidx.leanback.app.HeadersSupportFragment
import androidx.leanback.app.RowsSupportFragment
import androidx.leanback.widget.GuidanceStylist
import androidx.leanback.widget.VerticalGridView
/**
* Proguard rules in android/tv/proguard-rules.pro.
*/
object MemoryLeakFixes {
fun onCreate(app: Application) {
app.registerActivityLifecycleCallbacks(object : Application.ActivityLifecycleCallbacks {
override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
if (activity !is FragmentActivity) return
val supportFragmentManager = activity.supportFragmentManager
supportFragmentManager.registerFragmentLifecycleCallbacks(fragmentCallbacks, true)
}
override fun onActivityStarted(activity: Activity) {}
override fun onActivityResumed(activity: Activity) {}
override fun onActivityPaused(activity: Activity) {}
override fun onActivityStopped(activity: Activity) {}
override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) {}
override fun onActivityDestroyed(activity: Activity) {}
})
}
private val fragmentCallbacks = object : FragmentManager.FragmentLifecycleCallbacks() {
private var verticalGridViews = mutableMapOf<Fragment, VerticalGridView>()
override fun onFragmentViewCreated(
fm: FragmentManager,
f: Fragment,
v: View,
savedInstanceState: Bundle?
) {
// BaseRowSupportFragment is not public so check for the subclasses.
if (f is RowsSupportFragment) {
verticalGridViews[f] = f.verticalGridView
} else if (f is HeadersSupportFragment) {
verticalGridViews[f] = f.verticalGridView
}
}
override fun onFragmentViewDestroyed(fm: FragmentManager, f: Fragment) {
// Memory leak, clear the adapter from the grid view manually because the
// adapter instance is leaked in BaseRowSupportFragment.
verticalGridViews.remove(f)?.adapter = null
// Memory leak, rootView is not set to null in ProgressBarManager (referenced by BaseSupportFragment).
if (f is BaseSupportFragment) {
val progressBarManager = f.progressBarManager
progressBarManager.hide()
progressBarManager.setRootView(null)
}
if (f is BrowseSupportFragment) {
// Memory leak, mBrowseFrame is not set to null.
try {
BrowseSupportFragment::class.java.getDeclaredField("mBrowseFrame").apply {
isAccessible = true
set(f, null)
}
} catch (exception: Exception) {
exception.printStackTrace()
}
// Memory leak, mScaleFrameLayout is not set to null.
try {
BrowseSupportFragment::class.java.getDeclaredField("mScaleFrameLayout").apply {
isAccessible = true
set(f, null)
}
} catch (exception: Exception) {
exception.printStackTrace()
}
// Memory leak, mSceneWithHeaders (has reference to mBrowseFrame) is not set to null.
try {
BrowseSupportFragment::class.java.getDeclaredField("mSceneWithHeaders").apply {
isAccessible = true
set(f, null)
}
} catch (exception: Exception) {
exception.printStackTrace()
}
// Memory leak, mSceneWithoutHeaders (has reference to mBrowseFrame) is not set to null.
try {
BrowseSupportFragment::class.java.getDeclaredField("mSceneWithoutHeaders").apply {
isAccessible = true
set(f, null)
}
} catch (exception: Exception) {
exception.printStackTrace()
}
// Memory leak, mSceneAfterEntranceTransition (has reference to mBrowseFrame) is not set to null.
try {
BrowseSupportFragment::class.java.getDeclaredField("mSceneAfterEntranceTransition").apply {
isAccessible = true
set(f, null)
}
} catch (exception: Exception) {
exception.printStackTrace()
}
}
// Memory leak, mGuidanceContainer is not set to null.
// https://issuetracker.google.com/issues/164841457
if (f is GuidedStepSupportFragment) {
try {
GuidanceStylist::class.java.getDeclaredField("mGuidanceContainer").apply {
isAccessible = true
set(f.guidanceStylist, null)
}
} catch (exception: Exception) {
exception.printStackTrace()
}
}
// Memory leak, mTitleView is not set to null.
if (f is BrandedSupportFragment) {
f.titleView = null
}
}
override fun onFragmentDestroyed(fm: FragmentManager, f: Fragment) {
// Memory leak, the transition listeners (with references to the fragment) are added to the
// activity window but never removed, so remove them manually in the Frament onDestroy.
if (f is DetailsSupportFragment) {
fun removeTransitionListener(fieldName: String, activityTransition: Transition) {
try {
val transitionListener = DetailsSupportFragment::class.java
.getDeclaredField(fieldName).let {
it.isAccessible = true
it.get(f)
}
transitionListener::class.java.superclass!!.getDeclaredField("mImpl").apply {
isAccessible = true
val implValue = get(transitionListener) as Transition.TransitionListener?
if (implValue != null) {
activityTransition.removeListener(implValue)
set(transitionListener, null)
}
}
} catch (exception: Exception) {
exception.printStackTrace()
}
}
val window = f.requireActivity().window
removeTransitionListener("mEnterTransitionListener", window.enterTransition)
removeTransitionListener("mReturnTransitionListener", window.returnTransition)
}
}
}
}
da...@google.com <da...@google.com> #6
BrowseSupportFragment etc are not designed for fragment transition, leanback is explicitly designed with activity transition (from browse activty to details activity), they are expected to be same lifecycle as activity. So holding a View in BrowseSupportFragment shouldn't cause View leak. The View will be gone when the activity closes.
I also see some of the leaking detection is related with activity transition implementation. These warnings are invalid, it's unnecessary to remove View instance for activity transition. The detection doesn't know fragment is tightly bound to activity lifecycle in this case.
The real leaking is GuidanceStylist fixed above: GuidedStepSupportFragment can be in a multi-steps fragment flow, it should not hold View when replaced by another Fragment.
th...@gmail.com <th...@gmail.com> #7
Thanks for the explanation!
I am using the leanback fragments (including BrowseSupportFragment) in a single Activity. I did that primarily because a single Activity is recommended by Google. Furthermore, the Leanback sample app also uses a single Activity (
Description
There is a memory leak in
GuidanceStepSupportFragment
. In theonDestroyView
function,mGuidanceStylist.onDestroyView()
is called. When looking at the implementation for that function, you can see that views are set tonull
. However, the field calledmGuidanceContainer
is not set to null here. That causes a memory leak.