FusedLocationProviderClient.removeLocationUpdates always returns failure

Question

I have an activity that extends a base class called LocationAwareActivity all this LocationAwareActivity activity does is creates a location service client

LocationServices.getFusedLocationProviderClient and listens to location updates.

Source for this activity is here https://github.com/snijsure/MultiActivity/blob/master/app/src/main/java/com/example/subodhnijsure/multiactivity/LocationAwareActivity.java

And when activity is destroyed it calls removeLocationUpdates . What I am finding is

• removeLocationUpdate returns a task that always returns not-successful
• More concerning is because location activities is not removed, the activity is not getting being garbage collected.

- So if I start the any activity that inherits from LocationAwareActivity that activity always stays on heap.

So the question is what is the correct way to stop receiving location updates thus allowing activity to be garbage collected.

Entire source for this project can be accessed here - https://github.com/snijsure/MultiActivity

Answer 1

In removeLocationUpdates you should pass locationCallback, current implementation is wrong.

Still, there is chance of memory leak somewhere else. You should try integrating Leakcanary in your app and it can give you reference tree and will tell you which field or listener is causing this memory leak. You can refer one of my only blog post here

public void stopLocationUpdates() {
        if (locationProviderClient != null) {
            try {
                final Task<Void> voidTask = locationProviderClient.removeLocationUpdates(locationCallback);
                if (voidTask.isSuccessful()) {
                    Log.d(TAG,"StopLocation updates successful! ");
                } else {
                    Log.d(TAG,"StopLocation updates unsuccessful! " + voidTask.toString());
                }
            }
            catch (SecurityException exp) {
                Log.d(TAG, " Security exception while removeLocationUpdates");
            }
        }
    }

Answer 2

Hi @Subodh Nijsure Please check below code and paste into your code and after checked it:

final Task<Void> voidTask = locationProviderClient.removeLocationUpdates(locationCallback);
                voidTask.addOnCompleteListener(new OnCompleteListener<Void>() {
                    @Override
                    public void onComplete(@NonNull Task<Void> task) {
                        Log.e(TAG, "addOnCompleteListener: "+task.isComplete());
                    }
                });

                voidTask.addOnSuccessListener(new OnSuccessListener<Void>() {
                    @Override
                    public void onSuccess(Void aVoid) {
                        Log.e(TAG, "addOnSuccessListener: " );
                    }
                });

                voidTask.addOnFailureListener(new OnFailureListener() {
                    @Override
                    public void onFailure(@NonNull Exception e) {
                        Log.e(TAG, "addOnFailureListener: ");
                    }
                });

I think voidTask.isSuccessful() this method is not working when you put this listener at that time it working fine and i also see into memory it's release all memory when come to previous Activity.

And when you are redirecting to any activity then please stopLocationUpdates() called once into onPause() and remove from other method like onDestroy(),onStop() because it stop once so why should we call multiple time.

Hope this helps you.

Answer 3

By looking at the code in the repository I discovered some issues in your design that maybe cause the leaking of your Activity.

1) You are using two different LocationCallbacks. One in the start and one in the stop method, but you should actually use the same. So one time instantiating it would be sufficient and would lead probably also to a successful result of your Task when removing the LocationCallback.

2) Since your instantiating the LocationCallback twice with an Anonymous Class you are keeping a non-static reference of an inner class even if you finish the containing class and this causes your Memory Leak. You can read more about this here.

3) IMHO it is better to use a separate manager class for handling your location requests than abstracting an Activity.

That said here is my...

Solution

GpsManager.java

public class GpsManager extends LocationCallback {

    private FusedLocationProviderClient client;
    private Callback callback;

    public interface Callback {
        void onLocationResult(LocationResult locationResult);
    }

    public boolean start(Context context, Callback callback) {
        this.callback = callback;
        client = LocationServices.getFusedLocationProviderClient(context);
        if (!checkLocationPermission(context)) return false;
        client.requestLocationUpdates(getLocationRequest(), this, null);
        return true;
    }

    public void stop() {
        client.removeLocationUpdates(this);
    }

    @Override
    public void onLocationResult(LocationResult locationResult) {
        callback.onLocationResult(locationResult);
    }

    private boolean checkLocationPermission(Context context) {
        int permissionCheck = ContextCompat.checkSelfPermission(
                context, android.Manifest.permission.ACCESS_FINE_LOCATION);
        return permissionCheck == PackageManager.PERMISSION_GRANTED;
    }

    private LocationRequest getLocationRequest() {
        return LocationRequest.create()
                .setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY)
                .setInterval(30_000L)
                .setFastestInterval(20_000L);
    }
}

and calling this from your Activity like this

YourActivity.java

public class MapsActivity extends AppCompatActivity implements GpsManager.Callback  {

    private static final int PERMISSION_REQUEST_FINE_LOCATION = 1;
    private GpsManager mGpsManager;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        ...
        mGpsManager = new GpsManager(getApplicationContext(), this);

        // check if user gave permissions, otherwise ask via dialog
        if (!checkPermission()) {
            getLocationPermissions();
            return;
        }

        mGpsManager.start();
        ...
    }

    @Override
    protected void onStop() {
        super.onStop();
        mGpsManager.stop();
    }

    @Override
    public void onLocationResult(LocationResult locationResult) {
        // do something with the locationResult
    }

    // CHECK PERMISSIONS PART

    private boolean checkPermission() {
        return isGranted(ActivityCompat.checkSelfPermission(this, ACCESS_FINE_LOCATION)) &&
                isGranted(ActivityCompat.checkSelfPermission(this, ACCESS_COARSE_LOCATION));
    }

    @TargetApi(Build.VERSION_CODES.M)
    private void getLocationPermissions() {
        requestPermissions(new String[] {Manifest.permission.ACCESS_FINE_LOCATION},
                PERMISSION_REQUEST_FINE_LOCATION);
    }

    @Override
    public void onRequestPermissionsResult(int code, @Nullable String permissions[], @Nullable int[] results) {
        switch (code) {
            case PERMISSION_REQUEST_FINE_LOCATION:
                if (isPermissionGranted(results)) {
                    getLocationRequest();
                }
        }
    }

    private boolean isPermissionGranted(int[] results) {
        return results != null && results.length > 0 && isGranted(results[0]);
    }

    private boolean isGranted(int permission) {
        return permission == PackageManager.PERMISSION_GRANTED;
    }
}

This is just a guess because I didn't try your code but the solution should help you anyways. Please correct me if I'm wrong ;)

Answer 4

The reason why the Task object returns false is in your stopLocationUpdates method, you are again creating a local **LocationCallback** reference and then using this reference to as an argument in locationProviderClient.removeLocationUpdates(cL); where your local LocationCallBack is never present in the locationProviderClient

So what you have to do is , instead of creating another LocationCallBack object ,you have to pass the same global object which you are instantiating in your startLocationUpdates method

your code should be like this

final Task<Void> voidTask = locationProviderClient.removeLocationUpdates(locationCallback);