search

square / leakcanary

A memory leak detection library for Android.
https://square.github.io/leakcanary
Apache License 2.0
29.44k stars 3.97k forks source link

Drawable.mCallback + infinite ObjectAnimator cause hard to detect leaks #2116

Open Armaxis opened 3 years ago

Armaxis commented 3 years ago

Problem description

LeakCanary is ruining screenshots with its "Nothing's leaking" notifications: image

Potential solutions

A new configuration boolean can be added to disable the "nothing found" popup.

Additional information

pyricau commented 3 years ago

First we need to confirm the issue is with a notification being showed (not a popup). If yes, then.. I'm not convinced we need this. "No leak found" should be an edge case as it should be rare that we end up with non cleared weak refs yet can't find a leak. It's also a useful signal, because if we remove it people start thinking LeakCanary didn't show them the result of the analysis and dropped the ball (that's what happened when we did it that way).

dpreussler commented 3 years ago

Hi, so for me this one is the annoying one: Screenshot_20210503-100407 do we really need to show it?

I guess its a notification, cant it be less prio so its staing in notification bar?

pyricau commented 3 years ago

We can definitely lower the prioritization when nothing is found (PR welcome!)

I like to investigate those cases though, see if there's a particular reason nothing was found. I wouldn't mind taking a look at a heap dump from when that happens @dpreussler (ie the .hprof file associated with the report)

dpreussler commented 3 years ago

@pyricau sorry for the late response, here you go: https://drive.google.com/file/d/1C4rNr74X6HYJ22KTd-ooVdROsl72u5Ra/view?usp=sharing

pyricau commented 2 years ago

Ran the analysis on the latest shark-cli:

====================================
HEAP ANALYSIS RESULT
====================================
3 APPLICATION LEAKS

References underlined with "~~~" are likely causes.
Learn more at https://squ.re/leaks.

366 bytes retained by leaking objects
Signature: 00f0ab14a66b16eeae7988b30415bfdc35ce9d42
┬───
│ GC Root: Global variable in native code
│
├─ android.service.media.MediaBrowserService$ServiceBinder instance
│    Leaking: UNKNOWN
│    Retaining 894 B in 14 objects
│    this$0 instance of androidx.media.MediaBrowserServiceCompat$MediaBrowserServiceImplApi26$MediaBrowserServiceApi26
│    ↓ MediaBrowserService$ServiceBinder.this$0
│                                        ~~~~~~
╰→ androidx.media.MediaBrowserServiceCompat$MediaBrowserServiceImplApi26$MediaBrowserServiceApi26 instance
     Leaking: YES (Service not held by ActivityThread)
     Retaining 366 B in 13 objects
     mBase instance of com.soundcloud.android.playback.players.MediaService

59 bytes retained by leaking objects
Signature: 1471f4f37a91d44bfa43ecffdae717588c8e02d9
┬───
│ GC Root: Global variable in native code
│
├─ android.content.AbstractThreadedSyncAdapter$ISyncAdapterImpl instance
│    Leaking: UNKNOWN
│    Retaining 2.8 kB in 42 objects
│    ↓ AbstractThreadedSyncAdapter$ISyncAdapterImpl.this$0
│                                                   ~~~~~~
├─ com.soundcloud.android.sync.SyncAdapter instance
│    Leaking: UNKNOWN
│    Retaining 2.3 kB in 41 objects
│    mContext instance of com.soundcloud.android.app.RealSoundCloudApplication
│    ↓ SyncAdapter.looper
│                  ~~~~~~
├─ android.os.Looper instance
│    Leaking: UNKNOWN
│    Retaining 2.1 kB in 34 objects
│    ↓ Looper.mQueue
│             ~~~~~~
╰→ android.os.MessageQueue instance
     Leaking: YES (MessageQueue#mQuitting is true)
     Retaining 59 B in 2 objects

7725 bytes retained by leaking objects
Displaying only 1 leak trace out of 4 with the same signature
Signature: e98026f9815cfabf9f62b62b82bc5943858fcbbc
┬───
│ GC Root: System class
│
├─ android.provider.FontsContract class
│    Leaking: NO (RealSoundCloudApplication↓ is not leaking and a class is never leaking)
│    ↓ static FontsContract.sContext
├─ com.soundcloud.android.app.RealSoundCloudApplication instance
│    Leaking: NO (Application is a singleton)
│    mBase instance of android.app.ContextImpl
│    ↓ SoundCloudApplication.applicationComponent
│                            ~~~~~~~~~~~~~~~~~~~~
├─ com.soundcloud.android.app.DaggerRealApplicationComponent instance
│    Leaking: UNKNOWN
│    Retaining 65.0 kB in 1522 objects
│    application instance of com.soundcloud.android.app.RealSoundCloudApplication
│    ↓ DaggerRealApplicationComponent.mainNavController
│                                     ~~~~~~~~~~~~~~~~~
├─ com.soundcloud.android.listeners.navigation.MainNavController instance
│    Leaking: UNKNOWN
│    Retaining 8.7 kB in 317 objects
│    ↓ MainNavController.fragNavController
│                        ~~~~~~~~~~~~~~~~~
├─ com.ncapdevi.fragnav.FragNavController instance
│    Leaking: UNKNOWN
│    Retaining 8.5 kB in 305 objects
│    ↓ FragNavController.rootFragments
│                        ~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 7.8 kB in 283 objects
│    ↓ ArrayList[1]
│               ~~~
╰→ com.soundcloud.android.stream.StreamFragment instance
     Leaking: YES (Fragment#mFragmentManager is null)
     Retaining 2.3 kB in 91 objects
====================================
0 LIBRARY LEAKS

A Library Leak is a leak caused by a known bug in 3rd party code that you do not have control over.
See https://square.github.io/leakcanary/fundamentals-how-leakcanary-works/#4-categorizing-leaks
====================================
1 UNREACHABLE OBJECTS

An unreachable object is still in memory but LeakCanary could not find a strong reference path
from GC roots.

androidx.coordinatorlayout.widget.CoordinatorLayout instance
  Leaking: YES (ObjectWatcher was watching this because com.soundcloud.android.features.discovery.DiscoveryFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks))
  key = 7d51bdfd-404d-4c38-b305-00f00f43806e
  watchDurationMillis = 21408
  retainedDurationMillis = 16408
  View not part of a window view hierarchy
  View.mAttachInfo is null (view detached)
  View.mWindowAttachCount = 1
  mContext instance of com.soundcloud.android.main.MainActivity with mDestroyed = false
====================================
METADATA

Please include this in bug reports and Stack Overflow questions.

Build.VERSION.SDK_INT: 30
Build.MANUFACTURER: Google
LeakCanary version: 2.7
App process name: com.soundcloud.android.debug
Class count: 36199
Instance count: 282678
Primitive array count: 167096
Object array count: 42049
Thread count: 112
Heap total bytes: 41141862
Bitmap count: 31
Bitmap total bytes: 2221003
Large bitmap count: 0
Large bitmap total bytes: 0
Db 1: open /data/user/0/com.soundcloud.android.debug/databases/com.google.android.datatransport.events
Db 2: open /data/user/0/com.soundcloud.android.debug/databases/core.db
Db 3: open /data/user/0/com.soundcloud.android.debug/databases/uploads.db
Db 4: open /data/user/0/com.soundcloud.android.debug/databases/media_streams.db
Db 5: open /data/user/0/com.soundcloud.android.debug/databases/offline.db
Db 6: open /data/user/0/com.soundcloud.android.debug/databases/collections.db
Db 7: open /data/user/0/com.soundcloud.android.debug/databases/collection.db
Db 8: open /data/user/0/com.soundcloud.android.debug/databases/stream.db
Db 9: open /data/user/0/com.soundcloud.android.debug/databases/search_history.db
Db 10: open /data/user/0/com.soundcloud.android.debug/no_backup/androidx.work.workdb
Db 11: open /data/user/0/com.soundcloud.android.debug/databases/last_update.db
Db 12: open /data/user/0/com.soundcloud.android.debug/databases/discovery.db
Db 13: open /data/user/0/com.soundcloud.android.debug/databases/play_queue.db
Db 14: open /data/user/0/com.soundcloud.android.debug/databases/analytics.db
Db 15: open /data/user/0/com.soundcloud.android.debug/databases/promoted_tracking.db
Db 16: open /data/user/0/com.soundcloud.android.debug/databases/promoted_tracking.db
Db 17: open /data/user/0/com.soundcloud.android.debug/databases/promoted_tracking.db
Db 18: open /data/user/0/com.soundcloud.android.debug/databases/video_ads.db
Stats: LruCache[maxSize=3000,hits=84809,misses=232646,hitRate=26%] RandomAccess[bytes=13603184,reads=232646,travel=131077335153,range=46758623,size=57250867]
Analysis duration: 2226 ms
Heap dump file path: /Users/py/Downloads/2021-05-31_16-02-00_771.hprof
Heap dump timestamp: 1668201000482
Heap dump duration: Unknown
====================================
pyricau commented 2 years ago

This heap dump would have yielded a result, at least it would today.

However, there is the issue of the CoordinatorLayout being only weakly reachable.

I looked at the weak references reaching into it. One of the views in that detached view hierarchy is com.soundcloud.android.view.CircularProgressBar. The image below shows how that view is referenced by a weak reference held by an ObjectAnimator, through com.soundcloud.android.view.CircularProgressDrawable:

image

ObjectAnimator.mRunning is true.

So here's what was going on @dpreussler, that's actually a bug in Soundcloud :) : CircularProgressDrawable is animated by an object animator, in an infinite loop, and that animation isn't canceled when CircularProgressBar becomes detached. The object animator therefore runs forever! CircularProgressDrawable holds only a weak reference to CircularProgressBar through Drawable.mCallback (source), but unfortunately every 16ms the ObjectAnimator runs and it's likely that CircularProgressDrawable checks if the weak reference is cleared, which has the side effect of refreshing the reference, preventing it from being garbage collected.

I filed a ticket on AOSP for a very similar issue: https://issuetracker.google.com/issues/212993949

In 2.8 I added support for detecting ObjectAnimator leaks: https://square.github.io/leakcanary/changelog/#objectanimator-leaks.

I'm now adding detection of leaks caused by infinite ObjectAnimator + Drawable.mCallback in #2447. When running shark against that PR, here's the outcome, which clearly shows the infinite animator holding the drawable and indirectly the view hierarchy.

┬───
│ GC Root: Thread object
│
├─ java.lang.Thread instance
│    Leaking: NO (the main thread always runs)
│    Thread name: 'main'
│    ↓ Thread.threadLocals
│             ~~~~~~~~~~~~
├─ java.lang.ThreadLocal$ThreadLocalMap instance
│    Leaking: UNKNOWN
│    Retaining 476.2 kB in 8457 objects
│    ↓ ThreadLocal$ThreadLocalMap.table
│                                 ~~~~~
├─ java.lang.ThreadLocal$ThreadLocalMap$Entry[] array
│    Leaking: UNKNOWN
│    Retaining 476.2 kB in 8456 objects
│    ↓ ThreadLocal$ThreadLocalMap$Entry[37]
│                                      ~~~~
├─ java.lang.ThreadLocal$ThreadLocalMap$Entry instance
│    Leaking: UNKNOWN
│    Retaining 28 B in 1 objects
│    ↓ ThreadLocal$ThreadLocalMap$Entry.value
│                                       ~~~~~
├─ android.animation.AnimationHandler instance
│    Leaking: UNKNOWN
│    Retaining 131.3 kB in 2355 objects
│    ↓ AnimationHandler.mAnimationCallbacks
│                       ~~~~~~~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 131.2 kB in 2351 objects
│    ↓ ArrayList[0]
│               ~~~
├─ android.animation.ObjectAnimator instance
│    Leaking: UNKNOWN
│    Retaining 424 B in 13 objects
│    mListeners = null
│    mPropertyName = null
│    mProperty.mName = angle
│    mProperty.mType = java.lang.Float
│    mInitialized = true
│    mStarted = true
│    mRunning = true
│    mAnimationEndRequested = false
│    mDuration = 2000
│    mStartDelay = 0
│    mRepeatCount = INFINITE (-1)
│    mRepeatMode = RESTART (1)
│    ↓ ObjectAnimator.mProperty
│                     ~~~~~~~~~
├─ com.soundcloud.android.view.CircularProgressDrawable$1 instance
│    Leaking: UNKNOWN
│    Retaining 20 B in 1 objects
│    Anonymous subclass of android.util.Property
│    ↓ CircularProgressDrawable$1.this$0
│                                 ~~~~~~
├─ com.soundcloud.android.view.CircularProgressDrawable instance
│    Leaking: UNKNOWN
│    Retaining 267 B in 5 objects
│    ↓ CircularProgressDrawable.mCallback
│                               ~~~~~~~~~
├─ com.soundcloud.android.view.CircularProgressBar instance
│    Leaking: UNKNOWN
│    Retaining 2.1 kB in 33 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mID = R.id.upload_action_bar_verification_progress
│    View.mWindowAttachCount = 4
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ android.widget.FrameLayout instance
│    Leaking: UNKNOWN
│    Retaining 124.5 kB in 2210 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 4
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ com.soundcloud.android.creators.upload.ClassicTitleBarUploadView instance
│    Leaking: UNKNOWN
│    Retaining 76.8 kB in 1585 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mID = R.id.upload_action_bar_view
│    View.mWindowAttachCount = 4
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ android.widget.FrameLayout instance
│    Leaking: UNKNOWN
│    Retaining 74.6 kB in 1549 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 4
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ androidx.appcompat.widget.ActionMenuView instance
│    Leaking: UNKNOWN
│    Retaining 72.4 kB in 1515 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    mPopupContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ com.soundcloud.android.view.customfontviews.CustomFontTitleToolbar instance
│    Leaking: UNKNOWN
│    Retaining 35.7 kB in 813 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mID = R.id.toolbar_id
│    View.mWindowAttachCount = 1
│    mPopupContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    mContext instance of androidx.appcompat.view.ContextThemeWrapper, wrapping activity com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ android.widget.FrameLayout instance
│    Leaking: UNKNOWN
│    Retaining 26.6 kB in 664 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    mContext instance of com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ com.google.android.material.appbar.AppBarLayout instance
│    Leaking: UNKNOWN
│    Retaining 24.6 kB in 629 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mID = R.id.appbar
│    View.mWindowAttachCount = 1
│    mContext instance of com.soundcloud.android.main.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
╰→ androidx.coordinatorlayout.widget.CoordinatorLayout instance
     Leaking: YES (ObjectWatcher was watching this because com.soundcloud.android.features.discovery.DiscoveryFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks))
pyricau commented 1 year ago

The approach #2447 wasn't great because it made it look like all drawables had strong refs.

Maybe we need to introduce the concept of a "reachable" cache, and process this reference last, only going through if the value isn't already reachable, sort of a last resort path.