Should I worry about memory leaks and using WeakReference with Volley in Android

Question

After reading this article, I started thinking about memory leaks with Volley. Usually, the listeners implemented with Volley have either an implicit or explicit reference to the outer class (the activity). for example:

JsonObjectRequest jsonObjReq = new JsonObjectRequest(Request.Method.GET,
        url, null, 
        new Response.Listener<JSONObject>() {
            @Override 
            public void onResponse(JSONObject response) {

                updateLayout(); 
            } 
        }

in this case there is an implicit reference... or I may want to create a custom JsonObjectRequest to internalize the response handling, and need to pass in a reference to the calling activity in its constructor.

Now lets say I start a web request, but before the response comes back, I navigate away from the activity that started the request. From what I understand the JsonObjectRequest object would keep a reference to my activity and prevent it from being Garbage collected.
-Am I understanding this correctly, is this a legitimate fear?
-Does the Volley library automatically deal with this?
-If am creating a custom JsonObjectRequest and passing in a "this" (reference to activity), do I need to create a WeakReference to the activity?

Rockies

Have a look at the volley documentation. It looks like as long as you make sure to call cancel on your volley requests in the onStop() method of your fragment/activity, the handler won't be called. I assume this also means the reference to the fragment/activity will also be removed, meaning the fragment/activity is no longer leaked. – Noblesse

@tmalseed thanks, you should post your comment as an answer, and I will accept it – Rockies

I'm not 100% convinced it's the correct or most appropriate answer. I followed my own advice and might have still been leaking the activity.. – Noblesse

I see, thanks for the info, and would to get any updates if you get to the bottom of it. – Rockies

The documentation you are referring to does not protect from the memory leak. Because the Listener is never actively cleared from the request (set to null), whatever you set as the listener will hang around until the request completes itself. Volley just sets a flag to canceled that they use to skip notifying the listener if the request has been "canceled". It feels like a pretty significant oversight, especially when fragments are used as listeners (which then ties them to the activity, which rapidly becomes a giant memory leak waiting to happen)... – Centralization

Answer 1

Based on looking at the volley code, calling cancel doesn't really avoid the memory leak because the reference never gets cleared and the reference isn't weak. Calling cancel only avoids Volley from delivering the response to the listener.

My solution to the problem would have to be cloning and modifying the library myself.

• One of the solutions can be to make base ErrorListener reference inside of base Request.java to be weak reference. And similarly the same can be done to the Listener inside of JsonRequest.java.

• The other solution can be to manually clear the reference upon cancel being called. inside of cancel(), set the mErrorListener and mListener to null. With this solution though, you'll have to remove the final modifier from the field declaration otherwise you wouldn't be allowed to set the reference to null.

Hope this helps.

Platinocyanide

I've implemented option 2 and so far so good. I've found another leak due to the VolleyLog but that's a different topic. – Squires

No need to clone Volley, really. Just subclass Request or JsonRequest and handle the listener yourself (by adding the part of clearing them when cancel() is called). – Seedbed

Answer 2

I hava faced memory leak when using volley just like the way you write here.everytime I new a new listener.

I used leakcanary to detect leaking.

When I read about your article, http://www.androiddesignpatterns.com/2013/01/inner-class-handler-memory-leak.html ,and used WeakReference to activity and callback interface(myself customed), it solved.

I used volley as a singleton.

public interface VolleyCallback {
    void onSuccess(JSONObject result);

    void onFailed(String error);
}

private static class SListener implements Response.Listener<JSONObject> {
    private final WeakReference<Activity> activityWeakReference;
    private final WeakReference<VolleyCallback> callbackWeakReference;

    public SListener(Activity activity, VolleyCallback callback) {
        activityWeakReference = new WeakReference<Activity>(activity);
        callbackWeakReference = new WeakReference<VolleyCallback>(callback);
    }

    @Override
    public void onResponse(JSONObject jsonObject) {
        Activity act = activityWeakReference.get();
        VolleyCallback vc = callbackWeakReference.get();
        if (act != null && vc != null) {
            LogUtil.d(TAG, act.toString() + "   " + jsonObject.toString());
            something you need to do;
            vc.onSuccess(jsonObject);
        }
    }

I also read this answer,How to use WeakReference in Java and Android development? ,the second answer give an example just like your article provide.It's good.

Avellaneda

Answer 3

I am probably late to this party by a year but I just figured a way to solve this issue. Here it is:

public interface VolleyCallback { 
    void onSuccess(JSONObject result);

    void onFailed(String error);
} 

private static class SListener implements VolleyCallback {
    private final WeakReference<MainActivity> activityWeakReference;

    public SListener(MainActivity activity, VolleyCallback callback) {
        activityWeakReference = new WeakReference<>(activity);
    } 
@Override
    void onSuccess(JSONObject result){
}
 @Override
    void onFailed(String error){
   }
  } 

here you can use activityWeakReference.get() to access all the UI elements of the MainActivity too. Found this from http://www.androiddesignpatterns.com/2013/01/inner-class-handler-memory-leak.html. This way we don't need to cancel any requests in onStop(). Remember to use activityWeakReference.get().isFinishing && activityWeakReference.get()!=null to avoid crashes when the activity does not exist.

Ethnocentrism

Hello, I am having same issue and found your answer, would you mind sharing where to put the code above, like how to implement it. Here is my sample Volley implementation in my app. gist.github.com/arvi/587b2f4cb582ad96c98645212e7b2baf Thanks a lot. – Carlin

You should create a new static inner class and implement new Response.Listener<ProfileModel> & pass in a weak reference. – Ethnocentrism

@RohitRamkumar what if I don't need to pass context, but I need to pass variable that I created using activity context in onCreate() like this SomeClass obj = new SomeClass(this)? Should it be wrapped as weak ref? – Halland