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

AndroidLeakFixes$TEXT_LINE_POOL should not hold TextLine.sCached #2471

Closed
pyricau opened this issue Feb 8, 2023 · 1 comment · Fixed by #2472
Closed

AndroidLeakFixes$TEXT_LINE_POOL should not hold TextLine.sCached #2471

pyricau opened this issue Feb 8, 2023 · 1 comment · Fixed by #2472
Milestone

Comments

@pyricau
Copy link
Member

pyricau commented Feb 8, 2023

Not sure why but Google's CI found that the code that's attempting to clear the sCached leak ends up on the shortest path for that leak, which makes no sense. The best way to avoid that is simply to not cache sCached but instance retrieve it each time, which isn't all that costly anyway.

Related to https://android-review.googlesource.com/c/platform/frameworks/support/+/2420704/2#message-f6a1392ab5b07a2aed5111e5ca410cf0eaaf00b6

┬───
│ GC Root: System class
│
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (LeakCanaryApp↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary._application
├─ androidx.fragment.app.LeakCanaryApp instance
│    Leaking: NO (Application is a singleton)
│    mBase instance of android.app.ContextImpl
│    ↓ Application.mActivityLifecycleCallbacks
│                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 440 B in 20 objects
│    ↓ ArrayList[2]
│               ~~~
├─ leakcanary.AndroidLeakFixes$Companion$onActivityDestroyed$1 instance
│    Leaking: UNKNOWN
│    Retaining 44 B in 3 objects
│    Anonymous class implementing android.app.Application$ActivityLifecycleCallbacks
│    ↓ AndroidLeakFixes$Companion$onActivityDestroyed$1.$block
│                                                       ~~~~~~
├─ leakcanary.AndroidLeakFixes$TEXT_LINE_POOL$apply$1$3 instance
│    Leaking: UNKNOWN
│    Retaining 16 B in 1 objects
│    Anonymous subclass of kotlin.jvm.internal.Lambda
│    ↓ AndroidLeakFixes$TEXT_LINE_POOL$apply$1$3.$sCached
│                                                ~~~~~~~~
├─ android.text.TextLine[] array
│    Leaking: UNKNOWN
│    Retaining 36.6 kB in 587 objects
│    ↓ TextLine[1]
│              ~~~
pyricau added a commit that referenced this issue Feb 8, 2023
Somehow Google tests surfaced that AndroidLeakFixes was on the shortest path to TextLine.sCached which makes no sense. Let's make sure we're never there again.

Fixes #2471
@mohlson
Copy link

mohlson commented Mar 3, 2023

I'm not sure if having additional reports of this helps, but I believe we are seeing the same, in our leak detecting instrumentation tests.

┬───
│ GC Root: System class
│
├─ com.squareup.picasso.PicassoProvider class
│    Leaking: NO (CashRegisterTestApplication↓ is not leaking and a class is never leaking)
│    ↓ static PicassoProvider.context
├─ com.izettle.android.cashregister.CashRegisterTestApplication instance
│    Leaking: NO (Application is a singleton)
│    mBase instance of android.app.ContextImpl
│    ↓ Application.mActivityLifecycleCallbacks
│                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 464 B in 25 objects
│    ↓ ArrayList[4]
│               ~~~
├─ leakcanary.AndroidLeakFixes$Companion$onActivityDestroyed$1 instance
│    Leaking: UNKNOWN
│    Retaining 44 B in 3 objects
│    Anonymous class implementing android.app.Application$ActivityLifecycleCallbacks
│    ↓ AndroidLeakFixes$Companion$onActivityDestroyed$1.$block
│                                                       ~~~~~~
├─ leakcanary.AndroidLeakFixes$TEXT_LINE_POOL$apply$1$3 instance
│    Leaking: UNKNOWN
│    Retaining 16 B in 1 objects
│    Anonymous subclass of kotlin.jvm.internal.Lambda
│    ↓ AndroidLeakFixes$TEXT_LINE_POOL$apply$1$3.$sCached
│                                                ~~~~~~~~
├─ android.text.TextLine[] array
│    Leaking: UNKNOWN
│    Retaining 334.1 kB in 5478 objects
│    ↓ TextLine[0]
│              ~~~
METADATA

Please include this in bug reports and Stack Overflow questions.

Build.VERSION.SDK_INT: 21
Build.MANUFACTURER: unknown
LeakCanary version: 2.10
App process name: com.izettle.android.cashregister.test
Class count: 8676
Instance count: 372955
Primitive array count: 102783
Object array count: 51825
Thread count: 30
Heap total bytes: 38598380
Bitmap count: 416
Bitmap total bytes: 0
Large bitmap count: 0
Large bitmap total bytes: 0
Db 1: open /data/data/com.izettle.android.cashregister.test/databases/androidx.work.workdb
Stats: LruCache[maxSize=3000,hits=106555,misses=254810,hitRate=29%] RandomAccess[bytes=17822749,reads=254810,travel=340112493765,range=44325073,size=46561021]
assertionTag: BeforeActivitiesDestroyed
waitForRetainedDurationMillis: 5133
totalDurationMillis: 23506
Analysis duration: 14828 ms
Heap dump file path: /data/data/com.izettle.android.cashregister.test/files/instrumentation_tests_2023-02-20_08-05-06_878.hprof
Heap dump timestamp: 1676909125245
Heap dump duration: 656 ms

@pyricau pyricau added this to the 2.11 milestone Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants