The Wayback Machine - https://web.archive.org/web/20211229081220/https://github.com/square/leakcanary/issues/1983
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CoordinatorLayout.mPreSortedChildren retains references to Views from destroyed Fragments #1983

Open
eygraber opened this issue Oct 28, 2020 · 3 comments
Open

CoordinatorLayout.mPreSortedChildren retains references to Views from destroyed Fragments #1983

eygraber opened this issue Oct 28, 2020 · 3 comments
Labels
good first issue status: help wanted type: leak

Comments

@eygraber
Copy link
Contributor

@eygraber eygraber commented Oct 28, 2020

I have a CoordinatorLayout that I am using as a fragment container, and I started getting leak reports after fragments are getting destroyed, because their View's are referenced by the container's mPreSortedChildren.

Looking at the ViewGroup source, it seems like that gets cleared when buildOrderedChildList is called, but I'm not fully sure what would trigger that (seems like a draw will, but not sure if there are conditions).

Is there something LeakCanary (or plumber) can do about that, or would it have to be a framework fix (I can repro on a Pixel 2 running Android 11)?

Using LeakCanary 2.4

LeakTrace information

┬───
│ 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 (ViewRootImpl↓ is not leaking and InputMethodManager is a singleton)
│    ↓ InputMethodManager.mCurRootView
├─ android.view.ViewRootImpl instance
│    Leaking: NO (MyRecyclerView↓ is not leaking and ViewRootImpl#mView is not null)
│    ↓ ViewRootImpl.mAttachInfo
├─ android.view.View$AttachInfo instance
│    Leaking: NO (MyRecyclerView↓ is not leaking)
│    ↓ View$AttachInfo.mScrollContainers
├─ java.util.ArrayList instance
│    Leaking: NO (MyRecyclerView↓ is not leaking)
│    ↓ ArrayList.elementData
├─ java.lang.Object[] array
│    Leaking: NO (MyRecyclerView↓ is not leaking)
│    ↓ Object[].[3]
├─ com.myapp.MyRecyclerView instance
│    Leaking: NO (ConstraintLayout↓ is not leaking and View attached)
│    mContext instance of com.myapp.MainActivity with mDestroyed = false
│    View.parent androidx.constraintlayout.widget.ConstraintLayout attached as well
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mID = R.id.list
│    View.mWindowAttachCount = 1
│    ↓ MyRecyclerView.mParent
├─ androidx.constraintlayout.widget.ConstraintLayout instance
│    Leaking: NO (CoordinatorLayout↓ is not leaking and View attached)
│    mContext instance of com.myapp.MainActivity with mDestroyed = false
│    View.parent androidx.coordinatorlayout.widget.CoordinatorLayout attached as well
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    ↓ ConstraintLayout.mParent
├─ androidx.coordinatorlayout.widget.CoordinatorLayout instance
│    Leaking: NO (View attached)
│    mContext instance of com.myapp.MainActivity with mDestroyed = false
│    View.parent androidx.constraintlayout.widget.ConstraintLayout attached as well
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mID = R.id.fragmentContainer
│    View.mWindowAttachCount = 1
│    ↓ CoordinatorLayout.mPreSortedChildren
│                        ~~~~~~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    ↓ ArrayList.elementData
│                ~~~~~~~~~~~
├─ java.lang.Object[] array
│    Leaking: UNKNOWN
│    ↓ Object[].[2]
│               ~~~
╰→ androidx.constraintlayout.widget.ConstraintLayout instance
​     Leaking: YES (ObjectWatcher was watching this because com.myapp.ToastFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks))
​     key = 2b7e412c-ad06-4dd5-a175-2fb836a69564
​     watchDurationMillis = 5988
​     retainedDurationMillis = 959
​     mContext instance of com.myapp.MainActivity with mDestroyed = false
​     View#mParent is null
​     View#mAttachInfo is null (view detached)
​     View.mWindowAttachCount = 1
====================================
@pyricau
Copy link
Member

@pyricau pyricau commented Oct 31, 2020

Interesting! So it looks like mPreSortedChildren is a field on ViewGroup.

Can you provide the versions and manufacturers you've reproduced this on?

Looking at the latest ViewGroup sources, I can see mPreSortedChildren :

    // Temporary holder of presorted children, only used for
    // input/software draw dispatch for correctly Z ordering.
    private ArrayList<View> mPreSortedChildren;

However, looking at the source, that array list is filled by one method and call sites typically clear the list right after using it.

There are three call sites that don't clear: populateChildrenForAutofill, populateChildrenForContentCapture and buildTouchDispatchChildList(). As far as I can see these 3 scenarios could create a leak, that's pretty bad.

Can you file a bug to AOSP and link it here? Also it's interesting we haven't had reports yet, so you might run into a special case and it's worth reproducing with an example app. Once you have filed an issue, can you link it here? Then we can add it to the library leaks.
@eygraber
Copy link
Contributor Author

@eygraber eygraber commented Nov 1, 2020

I'll file the issue soon. Trying to clear up some time so I can test whether it's a special case, or if it's a general issue.
@Manabu-GT
Copy link

@Manabu-GT Manabu-GT commented Jan 21, 2021

We've encountered the same issue and filed a bug to AOSP - https://issuetracker.google.com/issues/178029590.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue status: help wanted type: leak
3 participants
@pyricau @eygraber @Manabu-GT