android / location-samples

Multiple samples showing the best practices in location APIs on Android.
Apache License 2.0
2.71k stars 2.77k forks source link

Google play service's v8.1.0's location listener is leaking the activity or service it is attached to #26

Open claywilkinson opened 9 years ago

claywilkinson commented 9 years ago

(Moving this issue from playgames to here on behalf of @youfacepalm)

I am not sure if this is the right place to report this since I could not find any common "Play Services" issue reporting forum anywhere.. If there is such an issue tracker, please give me the link. Thanks

The latest version of Google play service's (8.1.0 as of writing this report) LocationListener is leaking the activity or service it is attached to according to LeakCanary version 1.3.1.

Steps to reproduce

-Download the LocationUpdate Sample from google's repo (https://github.com/googlesamples/android-play-location/tree/master/LocationUpdates)

-Update the gradle to include the latest version of Google play services (8.1.0, the sample contains Google play services version 6.5.87 which had no issues during my testing)

Add a regular activity (name it "TestActivity" for example) to the project

from the MainActivity, after starting the location update, go to the newly added (startActivity()) TestActivity while finishing the MainActivity (it doesn't matter if you stop or did not stop the location updates)

after about 5 seconds you will see LeakCanary reporting that the MainActivity has leaked (if it is added to the project)

Device Info: Model: Samsung Galaxy S6 edge OS: Android Lollipop v5.1.1 API 22

Trace: D/LeakCanary﹕ In com.hammockstudio.myapplication:1.0:1.

com.hammockstudio.myapplication.MainActivity has leaked: GC ROOT com.google.android.gms.location.internal.zzd$1$1.zzaFg (anonymous class extends com.google.android.gms.location.internal.zzg$zza) references com.google.android.gms.location.internal.zzd$1.zzaFe (anonymous class extends com.google.android.gms.location.internal.zzd$zza) leaks com.hammockstudio.myapplication.MainActivity instance [ 10-02 15:32:05.124 20752:21895 D/LeakCanary ] Reference Key: c3aab681-4124-4c8c-ac2c-97d52fe63e4f Device: samsung samsung SM-G925I zeroltedv Android Version: 5.1.1 API: 22 LeakCanary: 1.3.1 Durations: watch=5016ms, gc=133ms, heap dump=2572ms, analysis=17078ms [ 10-02 15:32:05.124 20752:21895 D/LeakCanary ] Details: Instance of com.google.android.gms.location.internal.zzd$1$1 | zzaFg = com.google.android.gms.location.internal.zzd$1 [id=0x32e4f380] | mDescriptor = java.lang.String [id=0x32e563a0] | mObject = 547036342800 | mOwner = com.google.android.gms.location.internal.zzd$1$1 [id=0x32e56380] Instance of com.google.android.gms.location.internal.zzd$1 | zzaFd = com.google.android.gms.location.LocationRequest [id=0x32dedf40] | zzaFe = com.hammockstudio.myapplication.MainActivity [id=0x32c179f0] | zzaFf = com.google.android.gms.location.internal.zzd [id=0x32df8510] | zzZM = com.google.android.gms.common.api.Api$zzc [id=0x32df84d0] | zzabg = java.util.concurrent.atomic.AtomicReference [id=0x32e55150] | zzL = true | zzaaX = com.google.android.gms.common.api.Status [id=0x32cdaf20] | zzabh = java.lang.Object [id=0x32e55130] | zzabi = com.google.android.gms.internal.zzlc$zza [id=0x32e562a0] | zzabj = java.util.ArrayList [id=0x32e56240] | zzabk = null | zzabl = false | zzabm = false | zzabn = null | zzabo = null | zzabp = null | zzoS = java.util.concurrent.CountDownLatch [id=0x32e55140] Again, this leak did not happen in the Google Play Services version that came with the Location Updates sample!

raderto commented 4 years ago

The anonymous class (listener) has an implicit reference to the Activity. This is not Google's code causing the problem. The listener you register needs to be a concretely defined, static class so it won't contain an implicit reference to MainActivity.

Specifically this... https://github.com/android/location-samples/blob/master/LocationUpdates/app/src/main/java/com/google/android/gms/location/sample/locationupdates/MainActivity.java#L330

ruieduardosoares commented 3 years ago

I also have this issue, for quite some time i was digging all the layers that compose my activity + my code

Experimented with a lot of approaches to make sure the listener was not keeping my activity in memory, profiled the app process a lot of times.

I have found an instance in the app heap which has the following fields image

As you can see, we have a field called callbackIdMap, which is the one retaining the listeners the code was table to remove from the fusedlocationprovider client, but for some reason the references remain in the callbackIdMap as keys

As the google library code is offuscated, i am unable to know to that class these fields belong to. We have a constant string named LOCATION_PROVIDER_NAME also that has the string value "fused" which is not offuscated but i am unable to find any results by searching by this currently.

The fix would be to remove the listeners from this map when FusedLocationproviderClient.removeLocationUpdates is called, simple as that :see_no_evil:

This ticket is open for almost 6 years surprisingly :older_adult:

bubenheimer commented 2 years ago

Confirmed fixed in 20.0.0. Good news, but did this really have to take 7 years and 12 major releases?

chrisjenx commented 2 years ago

Maybe for you still seeing callbackIdMap leaking on latest release