Reset weak refs in between tests

https://github.com/square/workflow-kotlin/pull/632

Leak:

┬───
│ GC Root: Thread object
│
├─ android.app.Instrumentation$InstrumentationThread instance
│    Leaking: UNKNOWN
│    Retaining 227.7 kB in 4195 objects
│    Thread name: 'Instr: androidx.test.runner.AndroidJUnitRunner'
│    ↓ Instrumentation$InstrumentationThread<Java Local>
│                                           ~~~~~~~~~~~~
├─ org.junit.runner.Result instance
│    Leaking: UNKNOWN
│    Retaining 219.2 kB in 3829 objects
│    ↓ Result.failures
│             ~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 219.1 kB in 3823 objects
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ org.junit.runner.notification.Failure instance
│    Leaking: UNKNOWN
│    Retaining 219.1 kB in 3820 objects
│    ↓ Failure.fThrownException
│              ~~~~~~~~~~~~~~~~
├─ androidx.test.espresso.NoMatchingViewException instance
│    Leaking: UNKNOWN
│    Retaining 219.1 kB in 3819 objects
│    ↓ NoMatchingViewException.rootView
│                              ~~~~~~~~
├─ com.android.internal.policy.DecorView instance
│    Leaking: YES (View.mContext references a destroyed activity)
│    Retaining 196.6 kB in 3529 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    mContext instance of android.view.ContextThemeWrapper, wrapping activity com.squareup.sample.mainactivity.TicTacToeActivity with mDestroyed = true
│    ↓ View.mContext
├─ android.view.ContextThemeWrapper instance
│    Leaking: YES (DecorView↑ is leaking and ContextThemeWrapper wraps an Activity with Activity.mDestroyed true)
│    Retaining 32 B in 1 objects
│    mBase instance of com.squareup.sample.mainactivity.TicTacToeActivity with mDestroyed = true
│    ↓ ContextWrapper.mBase
╰→ com.squareup.sample.mainactivity.TicTacToeActivity instance
     Leaking: YES (ObjectWatcher was watching this because com.squareup.sample.mainactivity.TicTacToeActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
     Retaining 7.5 kB in 258 objects
     key = 11c18440-95d4-4600-bae9-6cab076a1bcd
     watchDurationMillis = 9186
     retainedDurationMillis = 4185
     mApplication instance of android.app.Application
     mBase instance of androidx.appcompat.view.ContextThemeWrapper

The InstrumentationThread thread has a local reference (ie local variable) to a test result which includes failures, and one of these failures is an exception that keeps a reference to the root view. The failing test is the one that runs before the one that we ding as leaking. The previous test fails, so not running the rule. But during the previous test we collect weak refs. Which we probably should clear before we start the next test. We do clear them normallly. when calling assertNoLeak (after running the analysis).

We likely want a try finally around evaluate that clears the refs.

@pyricau I believe we are still seeing this (and a related?) issue on version 2.10. Do you think the the first of these is the same?

====================================
HEAP ANALYSIS RESULT
====================================
2 APPLICATION LEAKS

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

23492 bytes retained by leaking objects
Displaying only 1 leak trace out of 2 with the same signature
Signature: 288dd58c08b8c100b6214e322ae945647b024a93
┬───
│ GC Root: Thread object
│
├─ android.app.Instrumentation$InstrumentationThread instance
│    Leaking: UNKNOWN
│    Retaining 2.1 MB in 45651 objects
│    Thread name: 'Instr: com.izettle.android.test.IzettleMockTestRunner'
│    ↓ Instrumentation$InstrumentationThread<Java Local>
│                                           ~~~~~~~~~~~~
├─ org.junit.runner.Result instance
│    Leaking: UNKNOWN
│    Retaining 1.8 MB in 33909 objects
│    ↓ Result.failures
│             ~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 1.8 MB in 33903 objects
│    ↓ CopyOnWriteArrayList[1]
│                          ~~~
├─ org.junit.runner.notification.Failure instance
│    Leaking: UNKNOWN
│    Retaining 871.5 kB in 16808 objects
│    ↓ Failure.fThrownException
│              ~~~~~~~~~~~~~~~~
├─ androidx.test.espresso.NoMatchingViewException instance
│    Leaking: UNKNOWN
│    Retaining 871.4 kB in 16807 objects
│    ↓ NoMatchingViewException.rootView
│                              ~~~~~~~~
╰→ com.android.internal.policy.DecorView instance
     Leaking: YES (View.mContext references a destroyed activity)
     Retaining 11.7 kB in 217 objects
     View not part of a window view hierarchy
     View.mAttachInfo is null (view detached)
     View.mWindowAttachCount = 1
     mContext instance of com.android.internal.policy.DecorContext, wrapping activity com.izettle.android.topLevelNavigation.TopLevelNavigationActivity with mDestroyed = true

2054485 bytes retained by leaking objects
Displaying only 1 leak trace out of 10 with the same signature
Signature: d324049fdd985fde1da52af311fa8d16538feb62
┬───
│ GC Root: Thread object
│
├─ android.app.Instrumentation$InstrumentationThread instance
│    Leaking: UNKNOWN
│    Retaining 2.1 MB in 45651 objects
│    Thread name: 'Instr: com.izettle.android.test.IzettleMockTestRunner'
│    ↓ Instrumentation$InstrumentationThread<Java Local>
│                                           ~~~~~~~~~~~~
├─ org.junit.runner.Result instance
│    Leaking: UNKNOWN
│    Retaining 1.8 MB in 33909 objects
│    ↓ Result.failures
│             ~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 1.8 MB in 33903 objects
│    ↓ CopyOnWriteArrayList[1]
│                          ~~~
├─ org.junit.runner.notification.Failure instance
│    Leaking: UNKNOWN
│    Retaining 871.5 kB in 16808 objects
│    ↓ Failure.fThrownException
│              ~~~~~~~~~~~~~~~~
├─ androidx.test.espresso.NoMatchingViewException instance
│    Leaking: UNKNOWN
│    Retaining 871.4 kB in 16807 objects
│    ↓ NoMatchingViewException.adapterViews
│                              ~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 205.7 kB in 4816 objects
│    ↓ ArrayList[0]
│               ~~~
├─ androidx.appcompat.widget.AppCompatSpinner instance
│    Leaking: YES (View.mContext references a destroyed activity)
│    Retaining 205.6 kB in 4814 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    mPopupContext instance of android.view.ContextThemeWrapper, wrapping activity com.izettle.android.topLevelNavigation.TopLevelNavigationActivity with mDestroyed = true
│    mContext instance of android.view.ContextThemeWrapper, wrapping activity com.izettle.android.topLevelNavigation.TopLevelNavigationActivity with mDestroyed = true
│    ↓ View.mContext
╰→ android.view.ContextThemeWrapper instance
     Leaking: YES (ContextThemeWrapper wraps an Activity with Activity.mDestroyed true)
     Retaining 48 B in 2 objects
     mBase instance of com.izettle.android.topLevelNavigation.TopLevelNavigationActivity with mDestroyed = true
====================================

Here is our metadata too

====================================
METADATA

Please include this in bug reports and Stack Overflow questions.

Build.VERSION.SDK_INT: 33
Build.MANUFACTURER: Google
LeakCanary version: 2.10

@mohlson the leak traces do look fairly similar.

Note that these are leaks caused by Espresso which keeps test results in memory and in case of NoMatchingViewException failure the view is held by the exception until the whole test suite ends.

The fix we implemented was to clear the list of watched reference, so the leaks are still in place but LeakCanary doesn't look for these activity instances. Notice how LeakCanary says:

ObjectWatcher was watching this because com.squareup.sample.mainactivity.TicTacToeActivity received Activity#onDestroy() callback

In your leak report, there's no "ObjectWatcher was watching this". Which means it wasn't watching those objects, the weak refs had been cleared. However, you probably configured LeakCanary.Config.leakingObjectFinder with a more aggressive version that scans the whole heap dump for any view / context that looks leaking. So those leaks still show up.

We should likely get Espresso to fix this bug.

@pyricau

In your leak report, there's no "ObjectWatcher was watching this". Which means it wasn't watching those objects, the weak refs had been cleared. However, you probably configured LeakCanary.Config.leakingObjectFinder with a more aggressive version that scans the whole heap dump for any view / context that looks leaking. So those leaks still show up.

This is entirely correct, very sharp eyed! That also gives me a mental model for what was happening. Thank you!

We should likely get Espresso to fix this bug.

I'll go and lend what little weight I can to the issue that you opened. Once again, thank you!

square / leakcanary

Reset weak refs in between tests #2297