RecyclerView doesn't unregister itself from the adapter on orientation change

Question

I used to use ListView in my android application and I recently switched to RecyclerView and observed that it introduced some memory leaks on orientation change. On further investigation, the reason became apparent

SETUP

A single activity which hosts a fragment, the instance of which is retained across config changes. The fragment contains a single RecyclerView in its layout file that is populated using a custom adapter

DRILLING DOWN

Whenever an Adapter is set for any of those 2 views, they register themselves with the adapter to monitor changes to the data and update on the UI. ListView unregisters itself on config changes by

@Override
protected void onDetachedFromWindow() {
    super.onDetachedFromWindow();
    ...

    if (mAdapter != null && mDataSetObserver != null) {
        mAdapter.unregisterDataSetObserver(mDataSetObserver);
        mDataSetObserver = null;
    }

    ...
}

Unfortunately, RecyclerView doesn't do that

@Override
protected void onDetachedFromWindow() {
    super.onDetachedFromWindow();
    if (mItemAnimator != null) {
        mItemAnimator.endAnimations();
    }
    mFirstLayoutComplete = false;

    stopScroll();
    mIsAttached = false;
    if (mLayout != null) {
        mLayout.onDetachedFromWindow(this, mRecycler);
    }
    removeCallbacks(mItemAnimatorRunner);
}

PROOF

I changed the orientation a good number of times and then took a heap dump, and read it using Eclipse's MAT. I did see that there were a good number of instances of my activity because the RecyclerView instances didn't unregister and they have strong references to my activity!!

Am I missing something? How do you guys make sure that the RecyclerView doesn't leak your activity?

FRAGMENT

public class ExampleFragment extends Fragment {

    private ExampleAdapter mAdapter = null;

    public static ExampleFragment newInstance() {
        return new ExampleFragment();
    }


    @Override
    public void onAttach(Activity activity) {
        super.onAttach(activity);
        setupAdapterIfRequired();
    }

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

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

    @Override
    public void onActivityCreated(Bundle savedInstanceState) {
        super.onActivityCreated(savedInstanceState);
        setupRecyclerView(getView());
    }

    private void setupAdapterIfRequired() {
        if (mAdapter == null) {
            mAdapter = new ExampleAdapter();
        }
    }

    private void setupRecyclerView(View rootView) {
        RecyclerView recyclerView = (RecyclerView) rootView.findViewById(R.id.list);
        recyclerView.setLayoutManager(new LinearLayoutManager(getActivity(), LinearLayoutManager.VERTICAL, false));
        recyclerView.setAdapter(mAdapter);
    }
}

I make sure by not retaing the fragment in the first place :) — Bojan Kseneman, May 08 '15 at 20:55
That would be one way to do it, but my fragments are data heavy. It would be better to keep them around — x-treme, May 08 '15 at 20:58
This issue manifested in another way for me. When adding a `Fragment` to the back stack I keep the adapter in memory as an instance variable. Every time I navigate back to that `Fragment` a new view is created but the adapter retains a reference to every `RecyclerView` that it was added to, leaking all of the old ones. The solution here worked to `setAdapter(null)` when destroying the view. — darnmason, Dec 16 '16 at 01:59

score 23 · Answer 1 · edited Mar 10 '16 at 14:28

23

Adding this to Fragment stopped leaks for me:

@Override
public void onDestroyView() {
    super.onDestroyView();
    recyclerView.setAdapter(null);
}

edited Mar 10 '16 at 14:28

Joe Van der Wee

126
9

answered Sep 30 '15 at 12:27

InTwoMinds

655
4
9

4

Any idea why this works or better yet why the recyclerview does not automatically release the adapter reference? – kldavis4 Dec 03 '15 at 18:36
Annoying this is required to trigger onDetachedFromRecyclerView - Worked though. – speedynomads Jun 23 '16 at 15:25
@kldavis4 in my case the culprit was the `AdapterDataObserver` registered internally by `RecyclerView` when calling `setAdapter` initially, it calls `registerAdapterDataObserver` which is documented stating _Components registering observers with an adapter are responsible for unregistering those observers when finished._, but it doesn't propagate that information to callers of `setAdapter` to let them know that to unregister you need to call `setAdapter` again. As to why it does not release it automatically: https://issuetracker.google.com/issues/140951437 – arekolek Sep 17 '20 at 08:44
This method crashes now on setting the adapter to null. Android 12L. – falero80s Jun 25 '22 at 06:15

score 3 · Answer 2 · answered May 08 '15 at 21:39

3

It not the problem with RecyclerView. It because you are setting setRetainInstance(true).

setRetainInstance() should only be use with fragment without view or it will cause memory leak.

When orientation change activity will be killed, but view in fragment still using context from that activity. That is why you see memory leak.

answered May 08 '15 at 21:39

Pongpat

13,248
9
38
51

2

`When orientation change activity will be killed, but view in fragment still using context from that activity. That is why you see memory leak.` - This is not the reason for my memory leak. On orientation change, a `fragment`'s views will be recreated (therefore loosing the activity's reference - as context) but the adapter I use to populate the `RecyclerView` will hold a strong reference to the view since it didn't unregister which in turn will hold a strong reference to the activity itself, thereby resulting in the leak – x-treme May 11 '15 at 17:46
2

But as you said, the leak can be prevented by not retaining the fragment containing those `RecyclerView`s – x-treme May 11 '15 at 17:47
2

when setRetainedInstance is set to true, the instance of the Fragment is kept upon orientation changes or activities recreation. However, onDestroyView() is still called and the View hierarchy should be gabage collected. The OP's problem is that their View hierarchy are leaked. That means there must be a reference to the Views that is not destroyed. The OP is right, and that reference is held via the adapter, not the fragment. Detaching the adapter from the RecyclerView solves the problem. – GaRRaPeTa Nov 09 '16 at 11:34
Setting setRetainInstance(false) in this example will also work, as with that the Fragment will be released, thus the Adapter is released (I'm assuming a reference to the Adapter from the Fragment), thus the Views will be released. BUT, it should still be passible to not to leak the Views with setRetainInstance(true), and reason why the OP experiments the leak is ultimately that the RecyclerViews does not unregister from the Adapter. – GaRRaPeTa Nov 09 '16 at 11:36

score 0 · Answer 3 · answered Jun 24 '15 at 08:41

I ran into the same issue. I make some search but don't find anything about this thread.

So I query myself about : Why did I need a context in an RecyclerView.Adapter ? (In an ArrayAdapter you use it to Infalte some layout) But in this new pattern you can use the parent context to inflate the layout :

e.g.

public class RecyclerViewAdapter extends RecyclerView.Adapter<VH> {
     @Override
     public BaseViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
          return LayoutInflater.from(parent.getContext()).inflate(R.layout.item_action, parent, false);
     }
}

Same to Bind it

@Override
public void onBindViewHolder(BaseViewHolder holder, int position) {
    String myString = holder.itemView.getContext().getString(R.string.app);
    ((Button)holder.itemView).setText(myString);
}

Hope it help

The issue is not the adapter holding on to a long lived `context`, but `RecyclerView` not unregistering itself from the adapter — x-treme, Jun 24 '15 at 16:35
have posted the relevant code. I am not retaining fragments with UI any more. That takes care of the leak — x-treme, Jun 25 '15 at 19:03

RecyclerView doesn't unregister itself from the adapter on orientation change

3 Answers3

Linked