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

Activity was never GCed but no leak found #1173

Closed
chaoshuai opened this issue Jan 21, 2019 · 19 comments · Fixed by #1317
Closed

Activity was never GCed but no leak found #1173

chaoshuai opened this issue Jan 21, 2019 · 19 comments · Fixed by #1317
Milestone

Comments

@chaoshuai
Copy link

What means that 'Activity was never GCed but no leak found'? thanks

@SebRut
Copy link
Contributor

SebRut commented Jan 23, 2019

If I understand it correctly, it means that this instance wasn't cleared by the garbage collector but LeakCanary wasn't able to find an reference pointing to this instance which could have prevented the collection. You could also check the additional information available for this "leak" for more information.

@tamtom
Copy link

tamtom commented Feb 23, 2019

@JakeWharton is that a good thing or a bad thing?

@derrickrc
Copy link

I'm also interested in whether or not this is a good or bad thing - thanks.

@pyricau
Copy link
Member

pyricau commented Mar 7, 2019

@SebRut is correct. It's unclear what could be causing this. Maybe there's no issue (but this shouldn't happen often). At this point the only way to help would be to get a heap dump and the class name.

@pyricau pyricau closed this as completed Mar 7, 2019
@shadowsheep1
Copy link

shadowsheep1 commented Mar 7, 2019

@pyricau Welcome back! I've this same scenario on a LG Nexus with Android API 19

  • NO LEAK FOUND.
* Reference Key: 41bb9376-4802-46a2-bbd1-0974d1498e07
* Device: LGE google Nexus 5 hammerhead
* Android Version: 4.4 API: 19 LeakCanary: 1.6.3 31007b4
* Durations: watch=5022ms, gc=188ms, heap dump=803ms, analysis=31060ms
* Excluded Refs:
| Field: android.app.ActivityThread$ActivityClientRecord.nextIdle
| Field: android.widget.Editor$EasyEditSpanController.this$0
| Field: android.widget.Editor$SpanController.this$0
| Field: android.os.Message.obj
| Field: android.os.Message.next
| Field: android.os.Message.target
| Field: android.view.inputmethod.InputMethodManager.mNextServedView
| Field: android.view.inputmethod.InputMethodManager.mServedView
| Field: android.view.inputmethod.InputMethodManager.mServedInputConnection
| Field: android.view.inputmethod.InputMethodManager.mCurRootView
| Field: android.animation.LayoutTransition$1.val$parent
| Field: android.view.textservice.SpellCheckerSession$1.this$0
| Field: android.support.v7.internal.widget.ActivityChooserModel.mActivityChoserModelPolicy
| Field: android.widget.ActivityChooserModel.mActivityChoserModelPolicy
| Field: android.speech.SpeechRecognizer$InternalListener.this$0
| Field: android.accounts.AccountManager$AmsTask$Response.this$1
| Field: android.media.MediaScannerConnection.mContext
| Field: android.os.UserManager.mContext
| Field: android.appwidget.AppWidgetHost$Callbacks.this$0
| Field: android.media.AudioManager$1.this$0
| Field: android.widget.Editor$Blink.this$0
| Field: android.net.ConnectivityManager.sInstance
| Field: android.view.Choreographer$FrameDisplayEventReceiver.mMessageQueue (always)
| Static field: android.text.TextLine.sCached
| Static field: android.widget.BubblePopupHelper.sHelper
| Thread:FinalizerWatchdogDaemon (always)
| Thread:main (always)
| Thread:LeakCanary-Heap-Dump (always)
| Class:java.lang.ref.WeakReference (always)
| Class:java.lang.ref.SoftReference (always)
| Class:java.lang.ref.PhantomReference (always)
| Class:java.lang.ref.Finalizer (always)
| Class:java.lang.ref.FinalizerReference (always)

I could provide the heap dump, but I think is related with the use of Soft/WeakReferences.

On other devices with different OS Api Level this is not reported.

@pyricau
Copy link
Member

pyricau commented Mar 7, 2019

👍 I'll reopen if someone has a repro case where there is actually a leak and leakcanary isn't able to find it.

@Android-xiaole
Copy link

In my project,It happened many times,LinearLayout or Fragment or Activity was never GCed but no leak found.

@SebRut
Copy link
Contributor

SebRut commented Mar 13, 2019

Can confirm on @Android-xiaole experience. While running and navigating through our app we constantly get this notification leading to ignoring all LeakCanary notifications.

@pyricau
Copy link
Member

pyricau commented Mar 19, 2019

That's cool. I'll reopen if someone can provide a heap dump with a repro case where there is actually a leak and leakcanary isn't able to find it.

@saikiran91
Copy link

saikiran91 commented Apr 23, 2019

@pyricau, In my case it is: ReportFragment was never GCed, but no leak found. I don't have a ReportFragment class in my project its a class from "package androidx.lifecycle;"

Heap dump

@pyricau
Copy link
Member

pyricau commented Apr 23, 2019

Please upgrade to the latest release of LeakCanary.

@SebRut
Copy link
Contributor

SebRut commented Apr 24, 2019

Just did a quick upgrade to 2.0. Switching from 1.6.3 was easy and a known leak was detected correctly. There seems to be less freezing and notifications which is nice. But there still are non leaking retained instances every 1-2 minutes which seems to be the equivalent to the leaks reported in this issue.

@pyricau
Copy link
Member

pyricau commented Apr 24, 2019

@SebRut can you share a heap dump file of one of those, e.g. on google drive? I will take a look and see if the parser is somehow missing something.

LeakCanary 2 adds info to the heap dump so that I can import and rerun the analysis

@saikiran91
Copy link

Please upgrade to the latest release of LeakCanary.

@pyricau I have updated to 2.0-alpha 1, now it is throwing "8 nonleaking retained instance".
Dump: https://drive.google.com/file/d/15iwVeaQ-6p8eIqenOzEP671sFudUJU0j/view?usp=sharing

@pyricau
Copy link
Member

pyricau commented Apr 29, 2019

@saikiran91 Thanks! I can reproduce the issue.

I played around with excluded references, adding back weak references (but keeping KeyedWeakReference) out, and here's what I got:

 ┬
 ├─ cAv
 │    Leaking: NO (it's a GC root)
 │    ↓ cAv.a
 │          ~
 ├─ cAu
 │    Leaking: UNKNOWN
 │    ↓ cAu.d
 │          ~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ╰→ com.newsapp.article.ArticleActivity
 ​     Leaking: YES (ArticleActivity#mDestroyed is true)

 ┬
 ├─ org.chromium.content.browser.webcontents.WebContentsImpl
 │    Leaking: NO (it's a GC root)
 │    ↓ WebContentsImpl.j
 │                      ~
 ├─ aBG
 │    Leaking: UNKNOWN
 │    ↓ aBG.a
 │          ~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ org.chromium.android_webview.AwContents
 │    Leaking: UNKNOWN
 │    ↓ AwContents.c
 │                 ~
 ├─ android.webkit.WebView
 │    Leaking: YES (View#mAttachInfo is null)
 │    View.mID=2131231084 (name not found)
 │    View.mWindowAttachCount=1
 │    ↓ WebView.mParent
 ╰→ android.widget.FrameLayout
 ​     Leaking: YES (RefWatcher was watching this)

 ┬
 ├─ android.app.LoadedApk$ReceiverDispatcher$InnerReceiver
 │    Leaking: NO (it's a GC root)
 │    ↓ LoadedApk$ReceiverDispatcher$InnerReceiver.mDispatcher
 │                                                 ~~~~~~~~~~~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ android.app.LoadedApk$ReceiverDispatcher
 │    Leaking: UNKNOWN
 │    ↓ LoadedApk$ReceiverDispatcher.mReceiver
 │                                   ~~~~~~~~~
 ├─ com.bumptech.glide.manager.DefaultConnectivityMonitor$1
 │    Leaking: UNKNOWN
 │    Anonymous subclass of android.content.BroadcastReceiver
 │    ↓ DefaultConnectivityMonitor$1.this$0
 │                                   ~~~~~~
 ├─ com.bumptech.glide.manager.DefaultConnectivityMonitor
 │    Leaking: UNKNOWN
 │    ↓ DefaultConnectivityMonitor.listener
 │                                 ~~~~~~~~
 ├─ com.bumptech.glide.RequestManager$RequestManagerConnectivityListener
 │    Leaking: UNKNOWN
 │    ↓ RequestManager$RequestManagerConnectivityListener.this$0
 │                                                        ~~~~~~
 ├─ com.bumptech.glide.RequestManager
 │    Leaking: UNKNOWN
 │    ↓ RequestManager.treeNode
 │                     ~~~~~~~~
 ├─ com.bumptech.glide.manager.RequestManagerFragment$FragmentRequestManagerTreeNode
 │    Leaking: UNKNOWN
 │    ↓ RequestManagerFragment$FragmentRequestManagerTreeNode.this$0
 │                                                            ~~~~~~
 ╰→ com.bumptech.glide.manager.RequestManagerFragment
 ​     Leaking: YES (RequestManagerFragment#mFragmentManager is null)

 ┬
 ├─ android.app.LoadedApk$ReceiverDispatcher$InnerReceiver
 │    Leaking: NO (it's a GC root)
 │    ↓ LoadedApk$ReceiverDispatcher$InnerReceiver.mDispatcher
 │                                                 ~~~~~~~~~~~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ android.app.LoadedApk$ReceiverDispatcher
 │    Leaking: UNKNOWN
 │    ↓ LoadedApk$ReceiverDispatcher.mReceiver
 │                                   ~~~~~~~~~
 ├─ com.bumptech.glide.manager.DefaultConnectivityMonitor$1
 │    Leaking: UNKNOWN
 │    Anonymous subclass of android.content.BroadcastReceiver
 │    ↓ DefaultConnectivityMonitor$1.this$0
 │                                   ~~~~~~
 ├─ com.bumptech.glide.manager.DefaultConnectivityMonitor
 │    Leaking: UNKNOWN
 │    ↓ DefaultConnectivityMonitor.listener
 │                                 ~~~~~~~~
 ├─ com.bumptech.glide.RequestManager$RequestManagerConnectivityListener
 │    Leaking: UNKNOWN
 │    ↓ RequestManager$RequestManagerConnectivityListener.this$0
 │                                                        ~~~~~~
 ├─ com.bumptech.glide.RequestManager
 │    Leaking: UNKNOWN
 │    ↓ RequestManager.treeNode
 │                     ~~~~~~~~
 ├─ com.bumptech.glide.manager.SupportRequestManagerFragment$SupportFragmentRequestManagerTreeNode
 │    Leaking: UNKNOWN
 │    ↓ SupportRequestManagerFragment$SupportFragmentRequestManagerTreeNode.this$0
 │                                                                          ~~~~~~
 ╰→ com.bumptech.glide.manager.SupportRequestManagerFragment
 ​     Leaking: YES (RefWatcher was watching this)

 ┬
 ├─ cAv
 │    Leaking: NO (it's a GC root)
 │    ↓ cAv.a
 │          ~
 ├─ cAu
 │    Leaking: UNKNOWN
 │    ↓ cAu.d
 │          ~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ com.newsapp.article.ArticleActivity
 │    Leaking: YES (ArticleActivity#mDestroyed is true)
 │    ↓ ArticleActivity._$_findViewCache
 ├─ java.util.HashMap
 │    Leaking: YES (ArticleActivity↑ is leaking)
 │    ↓ HashMap.table
 ├─ java.util.HashMap$Node[]
 │    Leaking: YES (HashMap↑ is leaking)
 │    ↓ array HashMap$Node[].[14]
 ├─ java.util.HashMap$Node
 │    Leaking: YES (HashMap$Node[]↑ is leaking)
 │    ↓ HashMap$Node.value
 ├─ androidx.viewpager.widget.ViewPager
 │    Leaking: YES (View#mAttachInfo is null)
 │    View.mID=2131231078 (name not found)
 │    View.mWindowAttachCount=1
 │    ↓ ViewPager.mAdapter
 ├─ com.newsapp.article.ArticleViewPagerAdapter
 │    Leaking: YES (ViewPager↑ is leaking)
 │    ↓ ArticleViewPagerAdapter.mCurrentPrimaryItem
 ╰→ com.newsapp.article.comments.CommentsFragment
 ​     Leaking: YES (RefWatcher was watching this)

 ┬
 ├─ cAv
 │    Leaking: NO (it's a GC root)
 │    ↓ cAv.a
 │          ~
 ├─ cAu
 │    Leaking: UNKNOWN
 │    ↓ cAu.d
 │          ~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ com.newsapp.article.ArticleActivity
 │    Leaking: YES (ArticleActivity#mDestroyed is true)
 │    ↓ ArticleActivity.mFragments
 ├─ androidx.fragment.app.FragmentController
 │    Leaking: YES (ArticleActivity↑ is leaking)
 │    ↓ FragmentController.mHost
 ├─ androidx.fragment.app.FragmentActivity$HostCallbacks
 │    Leaking: YES (FragmentController↑ is leaking)
 │    ↓ FragmentActivity$HostCallbacks.mFragmentManager
 ├─ androidx.fragment.app.FragmentManagerImpl
 │    Leaking: YES (FragmentActivity$HostCallbacks↑ is leaking)
 │    ↓ FragmentManagerImpl.mAdded
 ├─ java.util.ArrayList
 │    Leaking: YES (FragmentManagerImpl↑ is leaking)
 │    ↓ ArrayList.elementData
 ├─ java.lang.Object[]
 │    Leaking: YES (ArrayList↑ is leaking)
 │    ↓ array Object[].[1]
 ╰→ com.newsapp.article.originalarticle.OriginalArticleFragment
 ​     Leaking: YES (RefWatcher was watching this)

 ┬
 ├─ cAv
 │    Leaking: NO (it's a GC root)
 │    ↓ cAv.a
 │          ~
 ├─ cAu
 │    Leaking: UNKNOWN
 │    ↓ cAu.d
 │          ~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ com.newsapp.article.ArticleActivity
 │    Leaking: YES (ArticleActivity#mDestroyed is true)
 │    ↓ ArticleActivity.mFragments
 ├─ android.app.FragmentController
 │    Leaking: YES (ArticleActivity↑ is leaking)
 │    ↓ FragmentController.mHost
 ├─ android.app.Activity$HostCallbacks
 │    Leaking: YES (FragmentController↑ is leaking)
 │    ↓ Activity$HostCallbacks.mFragmentManager
 ├─ android.app.FragmentManagerImpl
 │    Leaking: YES (Activity$HostCallbacks↑ is leaking)
 │    ↓ FragmentManagerImpl.mAdded
 ├─ java.util.ArrayList
 │    Leaking: YES (FragmentManagerImpl↑ is leaking)
 │    ↓ ArrayList.elementData
 ├─ java.lang.Object[]
 │    Leaking: YES (ArrayList↑ is leaking)
 │    ↓ array Object[].[0]
 ╰→ androidx.lifecycle.ReportFragment
 ​     Leaking: YES (ReportFragment#mFragmentManager is null)

 ┬
 ├─ android.view.autofill.AutofillManager$AutofillManagerClient
 │    Leaking: NO (it's a GC root)
 │    ↓ AutofillManager$AutofillManagerClient.mAfm
 │                                            ~~~~
 ├─ java.lang.ref.WeakReference
 │    Leaking: UNKNOWN
 │    ↓ WeakReference.referent
 │                    ~~~~~~~~
 ├─ android.view.autofill.AutofillManager
 │    Leaking: UNKNOWN
 │    ↓ AutofillManager.mServiceClientCleaner
 │                      ~~~~~~~~~~~~~~~~~~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.next
 │              ~~~~
 ├─ sun.misc.Cleaner
 │    Leaking: UNKNOWN
 │    ↓ Cleaner.referent
 │              ~~~~~~~~
 ├─ android.view.RenderNode
 │    Leaking: UNKNOWN
 │    ↓ RenderNode.mOwningView
 │                 ~~~~~~~~~~~
 ╰→ androidx.constraintlayout.widget.ConstraintLayout

@pyricau pyricau reopened this Apr 29, 2019
@pyricau pyricau added this to the 2.0 Next Release milestone Apr 29, 2019
@pyricau
Copy link
Member

pyricau commented Apr 29, 2019

Notes from the heap dump:

  • OnePlus, API 28
  • Reference.disableInstrisic=false (as expected)
  • Reference.slowPathEnabled=false (as expected)

Out of the 8 references:

  • 4 are related to ArticleActivity leaking because cAv.a points to cAu.d which has a weak reference to ArticleActivity. It's interesting that ArticleActivity isn't GCed yet seems to only be held by weak references (cAu.d and LeakCanary KeyedWeakReference).
  • 1 is WebContentsImpl.j -> aBG.a has a weak reference to AwContents which has a c field to a detached webview.
  • 2 are LoadedApk$ReceiverDispatcher$InnerReceiver has a reference to LoadedApk$ReceiverDispatcher which references a com.bumptech.glide.manager.DefaultConnectivityMonitor$1 which ends up leaking a fragment . This very much look like a glide leak.
  • The last one is android.view.autofill.AutofillManager holding on to a cleaner. Cleaners are a linked list, one of them points to a render node which points to a detached constraint layout.

@pyricau
Copy link
Member

pyricau commented Apr 29, 2019

The ReceiverDispatcher + Glide leaks are interesting. State shows that ReceiverDispatcher.mForgotten is true (LoadedApk#forgetReceiverDispatcher was called), ReceiverDispatcher.mRegistered is true (final), DefaultConnectivityMonitor.isConnected is true and DefaultConnectivityMonitor.isRegistered is false.

https://github.com/bumptech/glide/blob/master/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java

So it looks like DefaultConnectivityMonitor.connectivityReceiver has been registered, and then unregistered. InnerReceiver keeps a weak reference to it though, and that weak reference does not get cleared, despite there being no other reference to it as far as I can see.

@pyricau
Copy link
Member

pyricau commented Apr 29, 2019

Update: I've demonstrated that 7 of the 8 leaks in that heap dump go through a glide DefaultConnectivityMonitor.

This is an interesting challenge: weak references aren't supposed to create leaks, but if you keep probing a weak reference it can't ever be GCed. Including weak refs in shortest paths isn't great because they don't all behave that way, and we'd get shortest past that hide the real issues.

Ideas to play with:

  1. Maybe the weak ref in the dispatcher should not be skipped, but only that weak ref on matching android versions. Essentially add that as a "likely excluded". The exclusion API does not have access to the parser so this can't be done via APIs right now.
  2. It looks like "always exclude" might need to be split into sub priorities, so that we never get "no retained instance" but instead get a least favorable path. This is tricky to implement right.
  3. When it comes to weak refs it's hard to predict which one is more important to see / carries a bug due to reference refresh.

@pyricau
Copy link
Member

pyricau commented Apr 29, 2019

This is really tricky to nail. I've been iteratively adding more and more exclusions to move from one weak ref to the other and.. there are so many weaks refs and everything is tangled.

pyricau added a commit that referenced this issue Apr 30, 2019
* "excludedLeak" is now known as "won't fix leak", which matches its reality better. It's a leak that is known and real yet we're not fixing.
* Exclusion.alwaysExclude is replaced with Exclusion.status, with 3 possible values: NEVER_REACHABLE, WONT_FIX_LEAK, WEAKLY_REACHABLE.
* The shortest path finder will now follow weak references after all other paths have been exhausted.
* Removed class exclusion, was confusing, replaced with less lazy approach of listing fields to exclude
* VIEWLOCATIONHOLDER_ROOT should not have been always excluded, fixed.

Fixes #1173 ... well kinda. Instead of "non leaking retained instances", LeakCanary will report weakly reachable traces. Not sure if that'll help but it's a foundation we can build on.
pyricau added a commit that referenced this issue Apr 30, 2019
* Add segmented FIFO queue

Unified several queues into one segmented FIFO queue. This will enable us to add several levels of priority.

* Introducing exclusion priority

* "excludedLeak" is now known as "won't fix leak", which matches its reality better. It's a leak that is known and real yet we're not fixing.
* Exclusion.alwaysExclude is replaced with Exclusion.status, with 3 possible values: NEVER_REACHABLE, WONT_FIX_LEAK, WEAKLY_REACHABLE.
* The shortest path finder will now follow weak references after all other paths have been exhausted.
* Removed class exclusion, was confusing, replaced with less lazy approach of listing fields to exclude
* VIEWLOCATIONHOLDER_ROOT should not have been always excluded, fixed.

Fixes #1173 ... well kinda. Instead of "non leaking retained instances", LeakCanary will report weakly reachable traces. Not sure if that'll help but it's a foundation we can build on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants