34

I've read that setting .setOnRetainInstance(true) on fragments presenting UI may lead to memory leaks.

Could somebody please explain why and how this would happen? I didn't find a detailed explanation anywhere.

Ahmad
  • 69,608
  • 17
  • 111
  • 137
Zsombor Erdődy-Nagy
  • 16,864
  • 16
  • 76
  • 101

5 Answers5

97

In a Fragment with UI you often save some Views as instance state to speed up access. For example a link to your EditText so you don't have to findViewById it all the time.

The problem is that a View keeps a reference to the Activity context. Now if you retain a View you also retain a reference to that context.

That is no problem if the context is still valid but the typical retain case is restarting the Activity. Very often for a screen rotation for example. Activity recreation will create a new context and old contexts are intended to be garbage collected. But it can't be garbage collected now since your Fragment still has a reference to the old one.

Following example shows how not to do it

public class LeakyFragment extends Fragment {

    private View mLeak; // retained

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setRetainInstance(true);
    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        mLeak = inflater.inflate(R.layout.whatever, container, false);
        return mLeak;
    }

    @Override
    public void onDestroyView() {
        super.onDestroyView();
        // not cleaning up.
    }
}

To get rid of that problem, you need to clear all references to your UI in onDestroyView. Once the Fragment instance is re-used you will be asked to create a new UI on onCreateView. There is also no point in keeping the UI after onDestroyView. The Ui is not going to be used.

The fix in this example is just changing onDestroyView to

@Override
public void onDestroyView() {
    super.onDestroyView();
    mLeak = null; // now cleaning up!
}

And besides keeping references to Views you should obviously not keep references to the Activity (e.g. from onAttach - clean on onDetach) or any Context (unless it's the Application context).

zapl
  • 63,179
  • 10
  • 123
  • 154
  • Got any ideas, how to handle, the mountain of null pointers this might cause? I have several animation listeners, threads, and each one is using one of the references, that gets nulled out in onDestroyView. So whenever I use one of those references, I first have to check for null. That's very inconvenient. – Tamas Aug 08 '13 at 11:09
  • 1
    @Tamas Listeners, Threads, ... are all fine to keep referenced as long as they don't keep a reference to anything that is or references the `Activity`. If they have something like that referenced and the Activity is recreated using it will not result in anything valid so you have to update it anyways. – zapl Aug 08 '13 at 12:47
  • 1
    @Tamas example: http://pastebin.com/8A18kMym you basically need to propagate `onAttach` / `onDetach` to anything that is refering to context, and `onCreateView` / `onDestroyView` to anything that keeps references to views. – zapl Aug 08 '13 at 13:05
  • setRetainInstance(true) is not recommended imho, I don't want to keep anything in the memory, if it's not visible. Also, adapters are not really an issue. I've been running monkey a lot, and it usually failed at these parts like this: http://pastebin.com/9Gx3McWz edit:updated link – Tamas Aug 08 '13 at 13:23
  • @Tamas retain instance is fine imo if you know that a fragment is going to be re-used soon like fragments in tabs, after rotation, ... Regarding your link: what should happen if there is no more textview because the view is gone? You'll either need to kill the task or pause it or let it check that there is something and continue the next time there is something. If you keep using the same text view you set text on some invisible textview that's just kept in memory for no reason. And yes, thats painful with `AsyncTask` – zapl Aug 08 '13 at 13:37
  • Great guide zapl. Just a nitpick: From the docs - `onCreate(Bundle) will not be called since the fragment is not being re-created.` So no use overriding that method. – Jannie Theunissen Aug 12 '13 at 18:48
  • @JannieT "lifecycle will be slightly different when an activity is *recreated*" - `onCreate` will still be called when the fragment is created initially. And you need to override it if you want to do something that happens just once upon creation, e.g. `setRetainInstance` since you only need to do that once. – zapl Aug 12 '13 at 18:56
  • 1
    In the first example, I'm just wondering how bad it is to not clean up the view onDestroyView. I guess there are two cases: onConfigChange, so the onDestroy is called, followed by a onCreateView (which will will reset the mLeak variable), and when the activity is finished, the fragment is detached and destroyed. But I do see your point, and it seems be better just to never have any views retained – Pedro Lopes Mar 08 '14 at 12:50
  • @PedroLopes take for example fragments in a `ViewPager`. The pager will keep up to 3 fragments (the one visible and one to the right, one to the left) with views. If you flip the page it will `onDestroyView` the fragment that's no longer within range. If that `View` contains a big image in an `ImageView` you leak a lot of memory although that image is not going to be reused. The next time it apprears you'll have to create a new one via `onCreateView`. Leaking `Context` through a rotation will leak everything but just 1 `View` can be too big already and lead to `OutOfMemoryError`s – zapl Mar 08 '14 at 14:45
  • It's in my experience also simpler if you don't care about the cases and simply always clean up. Symmetric create / destroy methods exist so the framework has to do the thinking while you just need to implement the right thing. – zapl Mar 08 '14 at 14:52
  • 3
    @zapl what is the point of doing manual clean up of the variables instantiated in onCreateView in onDestroyView when onCreateView will be called again in few milliseconds and rewrite those references with new objects, so old objects will have no reference and will be GC? – Happy Dev Sep 23 '14 at 05:21
  • @Happydev it is not guaranteed that there will be `onCreateView` directly after `onDestroyView`. Apps can go into background, fragments can be hidden, ... Cleaning up in destroy guarantees that you're not filling the memory with unnecessary stuff and potentially oom-exception because of that. – zapl Oct 22 '14 at 12:10
  • @zapl so then every view call should be preceded with null check? because I have issues when onClickListener called after onDestroyView... – Happy Dev Oct 23 '14 at 22:48
  • @Happydev click listener should never have that problem because there are no more buttons to be pressed after the view is destroyed. If you have async tasks or other events that are somehow delayed you will have to check if there is currently some UI to modify. If not, save the result / new state in a variable and let the next onCreateView use it to update the ui instead – zapl Oct 24 '14 at 09:09
  • Setting the reference to my views to null made no difference with my leaky activity (that contained fragments). Was another problem. – Diederik Jul 04 '16 at 05:40
  • Can we use WeakReference ? – slaviboy Apr 19 '20 at 11:05
3

setRetainInstance(true) is used to retain instances of dynamic Fragments during an Activity recreation, such as a screen rotation or other config changes. This does not mean the Fragment will be retained forever by the system though.

When an Activity is terminated for other reasons, such as the user finishing the Activity (i.e. pressing back), the Fragment should be eligible for garbage collection.

wsanville
  • 37,158
  • 8
  • 76
  • 101
3

Be careful when retaining certain objects that are coupled to the Activity.

Caution: While you can return any object, you should never pass an object that is tied to the Activity, such as a Drawable, an Adapter, a View or any other object that's associated with a Context. If you do, it will leak all the views and resources of the original activity instance. (Leaking resources means that your application maintains a hold on them and they cannot be garbage-collected, so lots of memory can be lost.)

http://developer.android.com/guide/topics/resources/runtime-changes.html#RetainingAnObject

Emanuel Canha
  • 441
  • 5
  • 11
0

The "setRetainInstance" is used to maintain the state of the fragment when the activity is recreated. According to the official documentation: if we use "setRetainInstance", 2 methods of the fragment's lifecycle will not be executed (onCreate, onDestroy). However, the views contained in the fragment will be recreated, and that is because the lifecycle will be executed from the "onCreateView". In these cases, if we have saved some data in "onSaveInstanceState", we should ask for it in the "onActivityCreated" instead of in the "onCreate".

Oficial info: https://developer.android.com/reference/android/app/Fragment.html#setRetainInstance(boolean)

More info: https://inthecheesefactory.com/blog/fragment-state-saving-best-practices/en

FacuArg
  • 64
  • 4
-12

you can overide onDestroy() and invoke garbage collector.

 @Override
public void onDestroy() {
    super.onDestroy();
    System.gc();
    System.gc();
}
Metro Polinet
  • 132
  • 1
  • 3