Memory leak in PreferenceFragmentCompat adapter handling [121006469]

Fixed

Bug

Status Update

No update yet.

Description

js...@gmail.com

created issue #1

Dec 14, 2018 07:54PM

RecyclerView registers an observer (

https://android.googlesource.com/platform/frameworks/support/+/androidx-master-dev/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/RecyclerView.java#1201) that is only unregistered manually, by either setting the adapter again, or calling unregisterAdapterDataObserver externally.

PreferenceFragmentCompat sets this adapter (PreferenceGroupAdapter), which implicitly registers an observer. PreferenceFragmentCompat never sets the adapter to null when the view is destroyed, so the observer keeps references to the RecyclerView and doesn't get gc'ed.

I've worked around this by explicitly setting the adapter to null in an override of onDestroyView, but since my component doesn't explicitly have ownership of the list handling, I'd argue that PreferenceFragmentCompat should do this cleanup.

It would be solved by setting the RecyclerView adapter to null in the fragment's onDestroyView.

Here's a memory dump.

2018-12-14_10-38-54_921.hprof

23 MB

Download

Comments

lp...@google.com <lp...@google.com> #2Jan 10, 2019 11:35PM

Jeremy, is this still an issue? I think the problem was that you had two transitions targeting the same View for the same action (e.g. two Slide() transitions).

lp...@google.com <lp...@google.com> #3Jan 14, 2019 08:41PM

Marked as fixed.

I have a similar issue with plain AnimatorSet:

set.start()
set.pause()
set.setCurrentPlayTime(100)
set.setCurrentPlayTime(0)
set.setCurrentPlayTime(100)
set.resume()

doesn't play animation in resume().

ma...@gmail.com <ma...@gmail.com> #4Feb 28, 2020 01:26PM

Should clarify that if I filter out setCurrentPlayTime(0) (or replace it with setCurrentPlayTime(1)) it works well.

Also even with setCurrentPlayTime(0), onAnimationEnd is notified with correct delay (as if the animation has played).

jb...@google.com <jb...@google.com> #5Feb 28, 2020 05:57PM

@ #4:

I think that is intended for Animator. If you set the currentPlayTime to 0 or the total duration the animator completes. We do some special checks in fragments to handle it.

ma...@gmail.com <ma...@gmail.com> #6Feb 29, 2020 09:16PM

Did some investigation on the Fragment side and it seems like the merged transition is targeting correctly.

Exiting Transition: Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
>>>>> ExitingViews <<<<<
View: android.widget.LinearLayout{f9add3d}
Entering Transition: Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})
>>>>> EnteringViews <<<<<
View: android.widget.LinearLayout{b7f24bc}
Final merged transition: TransitionSet@7bc1c45: 
      TransitionSet@e133f9a: 
        Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
      Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})
merged transition passed to controlDelayedTransition: TransitionSet@7bc1c45: 
      TransitionSet@e133f9a: 
        Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
      Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})

Still digging.

Issue 121006469