Status Update
Comments
da...@google.com <da...@google.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?
da...@google.com <da...@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
[Deleted User] <[Deleted User]> #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.
da...@google.com <da...@google.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 (
da...@google.com <da...@google.com> #8
da...@google.com <da...@google.com> #9
Is it a race condition in fragment library that calling requestFocus on a detached child?
ap...@google.com <ap...@google.com> #10
Branch: androidx-master-dev
commit 031d8dc8c746ca0840630cb6d641e91405362b6f
Author: Dake Gu <dake@google.com>
Date: Mon Nov 09 11:52:57 2020
leanback: fix wrong RecyclerView in GuidedActionAdapter
GuidedActionStylist has the same lifecycle as fragment that it can
be bound to different RecyclerViews in onCreateView/onDestroyView.
GuidedActionAdapter has the same lifecycle as the current fragment's
view. It adds onFocusChange listener to child views of RecyclerView.
Using GuidedActionStylist.getRecyclerView() may get the wrong one.
The fix is adding a final field mRecylerView in GuidedActionAdapter,
The adapter is created in fragment onCreateView and released in
fragment onDestroyView.
Bug: 172000115
Test: The bug is hard to reproduce, verified with existing Test and
verified demo app is still working.
Change-Id: Ic829f9256c1bd825a04bdeb81e673aa2a75527db
M leanback/leanback/src/main/java/androidx/leanback/widget/GuidedActionAdapter.java
Description
Version used: leanback:1.1.0-alpha04 (leanback:1.1.0-alpha05)
Devices/Android versions reproduced on: NVIDIA SHILED (android 9) & Xiaomi MiBOX (android 9)
(cannot by reproduce on android TV emulator with android 10 though)
crashes once fragment-ktx version has been upgraded to beta01 and calling add():475 in GuidedStepSupportFragment
StackTrace:
java.lang.IllegalArgumentException: View androidx.leanback.widget.GuidedActionItemContainer{701d058 VFE...C.. .F....ID 0,340-576,427} is not a direct child of androidx.leanback.widget.VerticalGridView{8f45c82 V.E...... ......ID 0,96-576,1080 #7f0b0298 app:id/guidedactions_list}
at androidx.recyclerview.widget.RecyclerView.getChildViewHolder(RecyclerView.java:4944)
at androidx.leanback.widget.GuidedActionAdapter$ActionOnFocusListener.onFocusChange(GuidedActionAdapter.java:383)
at android.view.View.onFocusChanged(View.java:7265)
at android.view.View.handleFocusGainInternal(View.java:6934)
at android.view.ViewGroup.handleFocusGainInternal(ViewGroup.java:795)
at android.view.View.requestFocusNoSearch(View.java:11546)
at android.view.View.requestFocus(View.java:11520)
at android.view.ViewGroup.requestFocus(ViewGroup.java:3230)
at android.view.View.requestFocus(View.java:11487)
at android.view.View.requestFocus(View.java:11429)
at androidx.fragment.app.FragmentStateManager.resume(FragmentStateManager.java:603)
at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:310)
at androidx.fragment.app.SpecialEffectsController$FragmentStateManagerOperation.complete(SpecialEffectsController.java:723)
at androidx.fragment.app.SpecialEffectsController$Operation.completeSpecialEffect(SpecialEffectsController.java:657)
at androidx.fragment.app.DefaultSpecialEffectsController$SpecialEffectsInfo.completeSpecialEffect(DefaultSpecialEffectsController.java:739)
at androidx.fragment.app.DefaultSpecialEffectsController$9.run(DefaultSpecialEffectsController.java:615)
at androidx.transition.FragmentTransitionSupport$5.onTransitionEnd(FragmentTransitionSupport.java:280)
at androidx.transition.Transition.end(Transition.java:1965)
at androidx.transition.TransitionSet$TransitionSetListener.onTransitionEnd(TransitionSet.java:451)
at androidx.transition.Transition.end(Transition.java:1965)
at androidx.transition.Transition$3.onAnimationEnd(Transition.java:1914)
at android.animation.Animator$AnimatorListener.onAnimationEnd(Animator.java:552)
at android.animation.ValueAnimator.endAnimation(ValueAnimator.java:1232)
at android.animation.ValueAnimator.doAnimationFrame(ValueAnimator.java:1474)
at android.animation.AnimationHandler.doAnimationFrame(AnimationHandler.java:146)
at android.animation.AnimationHandler.access$100(AnimationHandler.java:37)
at android.animation.AnimationHandler$1.doFrame(AnimationHandler.java:54)
at android.view.Choreographer$CallbackRecord.run(Choreographer.java:947)
at android.view.Choreographer.doCallbacks(Choreographer.java:761)
at android.view.Choreographer.doFrame(Choreographer.java:693)
at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:935)
at android.os.Handler.handleCallback(Handler.java:873)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:193)
at android.app.ActivityThread.main(ActivityThread.java:6721)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)