Weak reference instead of getActivity() (Android avoid memory leak)?

Question

To avoid a memory leak I wrote the following method that will be used in activities and mainly in fragments (using inheritance). That method is supposed to allow me to never directly refer to the activity by calling

//this or getActivity()

The method is:

private WeakReference<BaseActivity> activityWeakReference = null; 

public BaseActivity getActivityFromWeakReference(){
        activityWeakReference = activityWeakReference == null ?
                new WeakReference<BaseActivity>((BaseActivity)getActivity()) :
                activityWeakReference;
        return activityWeakReference.get();
    }

Is calling this method getActivityFromWeakReference() instead of getActivity() safe according to memory leak threat?

If it is not safe to do so, should I return the activityWeakReference and call its get() method instead, to make it safe?

I have been using it in multiple fragments and I haven't had any problem so far. I ask the question because I read this (here):

As long as the lifetime of the helper is within the lifetime of the Activity, then there's no need to use a WeakReference. If the helper can live longer than the Activity, then you should use a WeakReference to avoid retaining the Activity in your object graph when the system destroys it.

So far, I haven't faced a case where a referred element outlived the activity. Please guys if you find an error or a possible one just write it in the comments.

Answer 1

It is totally feasible. For example you have this pseudocode code:

public class MainActivity extends Activity {

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

         new DownloadTask().execute();
    }

    public void showInfo() {
    }

    class DownloadTask extends AsyncTask<Void, Void, Void> {
        @Override
        protected Void doInBackground(Void... params) {
            return null;
        }

        @Override
        protected void onPostExecute(Void data) {
            // we can call showInfo() activity because Asynctask hold an implicit reference to activity 
            showInfo();
        }
    }
}

On above code, there is a situation will caused memory leak.

Here is the explanation:

When you create DownloadTask as above example, java call DownloadTask is an inner class. An inner class will implicit holds a reference to outer class, in this case is MainActivity. Moreover, when you start an asynctask, that asynctask will be held by system until it finish. For example, you download takes 30 seconds. In that 30 seconds, you rotate your device. When you rotate your device, MainActivity is re-created and often old activity will be destroyed. But in this case old activity isn't destroyed because the old MainActivity instance is held by DownloadTask and DownloadTask is hold by system. You will leak one activity instance.

For fixing this, you should change above code to:

public class MainActivity extends Activity {

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        new DownloadTask(this).execute();
    }

    public void showInfo() {
    }
}

class DownloadTask extends AsyncTask<Void, Void, Void> {
    WeakReference<MainActivity> mainActivityWeakReference;

    public DownloadTask(MainActivity activity) {
        mainActivityWeakReference = new WeakReference<MainActivity>(activity);
    }

    @Override
    protected Void doInBackground(Void... params) {
        return null;
    }

    @Override
    protected void onPostExecute(Void data) {
        if (mainActivityWeakReference.get() != null) {
            mainActivityWeakReference.get().showInfo();
        }
    }
}

In this case, when new MainActivity is created, the old one isn't held by DownloadTask (due to weak reference attribute), so the old one will be destroyed by Android Garbage Collector in future. You also should to check every time you use a weak reference object, because you don't know exactly when GC will destroyed those object.

Here is my own blog about another situation of memory leak. Memory leak when using static inner class

Hope this help.

Answer 2

There are some case, if your fragment is set to retain instance, it'll be longer than activity, or if your fragment is leaked.