Collect: MemoryLeak in GeoPointActivity

[grzesiek2010]

Problem description

Instance of android.location.LocationManager$GnssStatusListenerTransport 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$ifTable = java.lang.Object[6]@1880885848 (0x701c0a58) 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$classSize = 260 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$componentType = null 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$numReferenceInstanceFields = 7 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$name = "android.location.LocationManager$GnssStatusListenerTransport" 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$vtable = null 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$referenceInstanceOffsets = 2035 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$numReferenceStaticFields = 0 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$status = -536870912 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$virtualMethodsOffset = 13 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$objectSize = 52 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $classOverhead = byte[132]@1882941545 (0x703b6869) 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$classFlags = 0 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$shadow$_monitor_ = 536870912 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$copiedMethodsOffset = 18 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$dexCache = java.lang.DexCache@1880727784 (0x7019a0e8) 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$dexClassDefIndex = 6105 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$shadow$_klass_ = java.lang.Class 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$extData = null 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$dexTypeIndex = 4145 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$accessFlags = 524288 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static NMEA_RECEIVED = 1000 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$methods = 1887716816 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$sFields = 1885872588 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$clinitThreadId = 0 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$superClass = android.location.IGnssStatusListener$Stub 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$objectSizeAllocFastPath = 56 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$primitiveType = 131072 2019-11-08 21:23:29.892 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$iFields = 1885872608 2019-11-08 21:23:29.893 8628-8662/org.odk.collect.android:leakcanary D/LeakCanary: | static $class$classLoader = null

Steps to reproduce the problem

1. Open any form with GeoPointWidget
2. Start recording location
3. Cancel it or accept.

[grzesiek2010]

I took a look at this issue and it's caused because in GeoPointActivity we call:
locationClient.setListener(this);

I tried clearing it in onStop() or onDestroy() bylocationClient.setListener(null); but the leak is still visible...
any ideas @zestyping @seadowg?

[seadowg]

Looking at the code it could be that in GeoPointActivity#onClientStart we call locationClient.requestLocationUpdates(this) but we never call LocationClient#stopLocationUpdates. Assuming we're using the AndroidLocationClient that would mean the activity would still be referenced as its locationListener (which is probably worth a rename as the two listeners have very similar names). Does that sound right?

One way to avoid these kinds of leaks would be to use WeakReference for listeners like this. We could also take advantage of ViewModel as it's onCleared would also help us here.

[zestyping]

That sounds right to me, @seadowg ! This looks like another instance of the issue fixed in https://github.com/opendatakit/collect/pull/2803 (which fixed it for all the new Geo activities, but not for this old one, which dates from 2017).

[zestyping]

Oops. #3445 addresses a different memory leak, which I've added as #3446. It doesn't address this one.

[zestyping]

Okay, I've taken #3445 a little further and now it should address this memory leak as well.

[mmarciniak90]

After merge #3445 to master, I retested GeoPoint widget to check where leaks are still visible.

Case - geopoint with no appearance

• User clicks on Start GeoPoint
• Loading location dialog is opened, location is detected
• User comes back to question
• User waits a moment - leak is recorded

Leaks visible on Google Maps, Mapbox, OSM

Leak noticed on Android: 4.2, 4.4, 5.1, 6.0, 7.0, 8.1, 9.0, 10.0

| Android 6.0 and older | Android 7.1 and newer |
|---|---|
|||

---

Case - geopoint with placement-map appearance AND maps appearance

• User clicks on Start GeoPoint
• View with map is opened
• User adds location
• User comes back to question
• User waits a moment - the leak is recorded

Leaks visible on Google Maps, OSM
I did not observe leaks when I used Mapbox.

Leak noticed on Android: 4.2, 4.4, 5.1, 6.0, 8.1, 9.0, 10.0
The leak does not appear on Android 7.0!

| Android 6.0 and older | Android 8.1 and newer |
|---|---|
|||
| | Androids 8.1, 9.0, 10.0 have additionally this leak. Both leaks appear together.|
| | |

I observed also this leak a few times but I'm not able to define repro steps.

[Vivever]

@lognaturel
I have tested Geo Widgets for memory leaks.
(PS. Long Report)
Tested on Android: 9.0
_Different Leaks in different widgets_
Geo Widgets
Start any Geo Widget
Add anything or not
Save or discard
Wait for 5 sec.
Report Fragment leaked

| Geopoint Widget(with no appearance) | Geopoint Widget(with placement-map appearance) |
| ------- | ------- |
| | |

| Geopoint Widget(with map appearance) | Geo Trace |
| -------- | -------- |
| | |

| Geo Shape |
| -------- |
| Same as Geo Trace |

Common leaks
After that when you press the Back button From any widget, "Exit All Widgets" appears.
Whatever you choose(Save or Ignore)
Wait for 5 sec
These two leaks will appear.

| Report Fragment | MediaLoading Fragment |
| -------- | -------- |
| | |

Wait here for 5 sec
Press the back button
You will be out of the app.
App is running in the background.
These memory leaks will appear(Either one of them).

| FormEntryActivity | MainMenuActivity |
| -------- | -------- |
| | |

Though MainMenuActivity leak, I got only one time.

Note(Not sure about these behaviours):

1. New form and Saved form sometimes show different behaviour(Sometimes leaks don't appear in new form )(I am not sure, I got lost while tracking ).
2. Internet connectivity affects the leaks( If net is slow leaks don't appear, sometimes ).

[mohammaduvez]

Hi, I am new here. I have gone through the reported leaks and managed to solve some. I am adding a PR as a quick solution. The solution can be improved but it requires to move some of the code, need suggestions for below changes.

1. Moving GpsStatus.Listener in LocationClient and implementing it similar to LocationListener.
2. Removing the need of WeakReference by moving the LocationClient in a ViewModel.

[lognaturel]

Given @mmarciniak90's notes on #3772, I believe GeoPointActivity is now leak-free. Let me know if I misunderstood, @mmarciniak90!