Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why callback method for asynchronous network call can't cause memory leak when activity finished?

We know that anonymous inner class may cause memory leak. But Why it does't work when asynchronous network call.
For example:

OkHttpClient client = new OkHttpClient();

 Request request = new Request.Builder()
                .get()
                .url(url)
                .build();
        client.newCall(request).enqueue(new Callback() {
            @Override
            public void onFailure(Call call, IOException e) {

            }
            @Override
            public void onResponse(Call call, Response response) throws IOException {
                if (response.isSuccessful()) {
                // String str = response.body().string();
                // do sth to our View, but those views may be null when activity finished
                }

            }
        });

We will change our view's states when callback method call, but those views always be null when activity finished. Why this anonymous inner class instance for callback can't cause activity leak.

like image 223
Cyrus Avatar asked Mar 02 '18 07:03

Cyrus


4 Answers

why this anonymous inner class instance for callback can't cause activity leak

I'm assuming that what you mean here is that it doesn't cause a memory leak, but it most certainly could, given that the scope in which you're instantiating the anonymous Callback is an Activity.

If you instantiate an inner class within an Android Activity and then pass a reference to this instance to some other component, as long as this component is reachable, so will be the instance of the inner class. For instance, consider this:

class MemorySink {

    static private List<Callback> callbacks = new ArrayList<>();

    public static void doSomething(Callback callback){
        callbacks.add(callback);
    }
}

If you created instances of Callbacks from some activities and pass them to doSomething(callback), when one of the Activity is destroyed the system will not use that instance anymore, it is expected that the garbage collector will release that instance. But, if MemorySink here has a reference to a Callback that has a reference to that Activity, the instance of that Activity will stay in memory even after being destroyed. Bam, memory leak.

So you say that your sample is not causing the memory leak, I will first suggest you to try out MemorySink, create a simple Activity "MainActivity" with 2 buttons and possibly some images to increase the memory footprint. In onCreate set a listener on the first button this way:

    findViewById(R.id.firstButton).setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            startActivity(new Intent(MainActivity.this, MainActivity.class));
            MemorySink.doSomething(new Callback() {
                @Override
                public void onFailure(Call call, IOException e) {

                }

                @Override
                public void onResponse(Call call, Response response) throws IOException {

                }
            });
            finish();
        }
    });

You've just created a memory leak using Callback. Every time you click on the button in MainActivity that instance of MainActivity will be destroyed and a new one will be created. But, the old instance of MainActivity will be retained in memory. I invite you to click many times the button and then dump the memory (with Android Profiler in Android Studio), or use LeakCanary.

So, we've created a memory leak with Callback, same class from OP. Now, let's add this method to MemorySink:

public static void releaseAll() {
    callbacks.clear();
}

And call it from another button on MainActivity. If you press the first button many times (better if you have images in MainActivity) you will see memory usages going up, even if you manually trigger garbage collection (Android Profile). Then you click this second button, all reference to Callback are released, trigger garbage collection, and memory goes down. No more memory leak.

So the problem is not if Callback can or cannot create a memory leak, it most certainly can. The problem is where are you passing that Callback. In that case OkHttpClient is not creating a memory leak, so you say, but it is not guaranteed at all that this will always happen. In that case you would need to be sure about the implementation of OkHttpClient to say that it won't generate a memory leak.

My suggestion is to always assume that the memory leak can happen if you are passing a reference to an Activity to some external class.

like image 130
lelloman Avatar answered Sep 27 '22 23:09

lelloman


Yes you are correct, that's why you can add a

 if (response.isSuccessful()) {
                // String str = response.body().string();
                if(view != null){
                //do sth to view
                }
 }
like image 25
jazz Avatar answered Sep 28 '22 00:09

jazz


A non-static inner class has a strong reference to the instance of the outer class. On the other hand a static inner class has no strong reference to the instance of the outer class. It uses WeakReference to reference outer class. you can learn more here

like image 44
shb Avatar answered Sep 27 '22 22:09

shb


client.newCall(request).enqueue(new Callback() {...})

Here you are passing a callback object to retrofit. By this, you are telling retrofit to use it in it's process to call-back to you when the call has finished.

That process is not in the current activity and it is in a separate thread and context (conceptually, not android's context). After your activity is destroyed by something (like rotation), the retrofit service is probably still alive and is holding a reference to this object and your activity causes a memory leak (it cannot be cleared from memory, and thus pollutes the memory) because your activity's callback object is needed by the retrofit.

If you wanted to fix this memory leak, you would need to cancel the call and remove the callback in activity's onStop, but that way you would lose the call on rotation.

The better thing to do, which is highly recommended, is that you don't do the asynchronous stuff inside the activity itself, and do it in a separate object (like Presenter in a MVP pattern or ViewModel in MVVM) that has a different lifecycle than the activity (a lifecycle that does not get destroyed by rotation or ...).

like image 42
Adib Faramarzi Avatar answered Sep 27 '22 22:09

Adib Faramarzi