Fix LeakCanary startup in debug builds and fix a memory leak #10232
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is it?
Description of the changes in your PR
PlayerService
-related leak and I decided to fix it, for more info see https://stackoverflow.com/q/63787707 and the leak report below. You can verify that this was fixed by opening the player, closing it and then taking a heap dump from LeakCanary. Note that these steps did not always lead to the leak before, idk why. Remember to close the app after you enable LeakCanary in debug settings!debugImplementation
only, as it does not make sense to include those in release builds. Previously they were included just becauseBaseFragment.onDestroy()
calledAppWatcher.expectWeaklyReachable(this)
, but I don't think we need to do it manually, since LeakCanary already takes care of detecting fragments being destroyed. After removing this manual call, I still saw some leaks being detected because "[...] MainFragment received onDestroyView", so I guess the automatic detection works. The release builds fine, here is a release APK signed by me:The leak report I fixed
┬───
│ GC Root: Global variable in native code
│
├─ org.schabi.newpipe.player.PlayerService$LocalBinder instance
│ Leaking: UNKNOWN
│ Retaining 6.8 kB in 103 objects
│ this$0 instance of org.schabi.newpipe.player.PlayerService
│ ↓ PlayerService$LocalBinder.this$0
│ ~~~~~~
╰→ org.schabi.newpipe.player.PlayerService instance
Leaking: YES (ObjectWatcher was watching this because org.schabi.newpipe.
player.PlayerService received Service#onDestroy() callback and Service
not held by ActivityThread)
Retaining 6.3 kB in 102 objects
key = cd33af62-1bd6-4297-bc44-2f05fab9fa16
watchDurationMillis = 19136
retainedDurationMillis = 14120
mApplication instance of org.schabi.newpipe.DebugApp
mBase instance of org.schabi.newpipe.player.AudioServiceLeakFix
METADATA
Build.VERSION.SDK_INT: 33
Build.MANUFACTURER: Google
LeakCanary version: 2.12
App process name: org.schabi.newpipe.debug
Class count: 30345
Instance count: 297963
Primitive array count: 185241
Object array count: 45771
Thread count: 74
Heap total bytes: 40216348
Bitmap count: 55
Bitmap total bytes: 10275621
Large bitmap count: 0
Large bitmap total bytes: 0
Db 1: open /data/user/0/org.schabi.newpipe.debug/databases/newpipe.db
Db 2: open /data/user/0/org.schabi.newpipe.debug/databases/exoplayer_internal.db
Db 3: open /data/user/0/org.schabi.newpipe.debug/no_backup/androidx.work.workdb
Stats: LruCache[maxSize=3000,hits=136480,misses=286718,hitRate=32%]
RandomAccess[bytes=15166205,reads=286718,travel=153801922092,range=44549734,size
=57029441]
Analysis duration: 24471 ms
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence