lostzen / lost

A drop-in replacement for Google Play services location APIs for Android
http://mapzen.github.io/lost/
Other
352 stars 70 forks source link

Context memory leak #175

Closed rwrx closed 7 years ago

rwrx commented 7 years ago

I have found that context field variable inside FusedLocationProviderApiImpl is causing memory leaks for passed Context. It is on this line: https://github.com/mapzen/lost/blob/master/lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderApiImpl.java#L32. It is causing memory leaks due to FusedLocationApi static field variable inside LocationServices. I haven't much investigated which exactly was causing this, but I can look at it if you want.

rwrx commented 7 years ago

I have also found that context field varible inside GeofencingApiImpl is causing memory leak for passed context: https://github.com/mapzen/lost/blob/master/lost/src/main/java/com/mapzen/android/lost/internal/GeofencingApiImpl.java#L30 . This memory leak is caused by static variable field GeofencingApi inside LocationServices. I believe that these leaks are caused by not setting to null contexts in both classes GeofencingApiImpl and FusedLocationProviderApiImpl when they was set by connect() method. So maybe just setting these contexts to null when disconnect() is called will fix them.

ecgreb commented 7 years ago

Thanks for the report @rwrx !

We should consider using the application context and/or a WeakReference here similar to how the Context is handled in the Mapbox SDK.

https://github.com/mapbox/mapbox-java/blob/master/mapbox/libandroid-services/src/main/java/com/mapbox/services/android/location/LostLocationEngine.java

rwrx commented 7 years ago

Yes, I agree, WeakReference can help with leaking Context. How can I help? :)