Activity leak when using FirebaseRecyclerAdapter

[talosdev]

I am noticing an activity leak with LeakCanary when using a FirebaseRecyclerAdapter.

Here's the trace from LeakCanary:

My adapter does not hold a reference to a context.

The leak seems to go away if I explicitly remove the adapter from the recycler in the onDestroy method.

@Override
public void onDestroy() {
    super.onDestroy();
    recycler.setAdapter(null);
}

Any ideas if this is normal behaviour?

• Google Play Services version: 9.4.0
• Firebase/Play Services SDK version: 9.4.0
• FirebaseUI version: 0.4.4

[samtstern]

@talosdev thanks for the report, a few questions:

1. Does recyclerAdapter.cleanup() avoid the leak just like setAdapter(null) does?
2. Can you show the code of your FirebaseRecyclerAdapter?

[talosdev]

1. Yes it does.
2. Here it is

public class MessagesAdapter extends FirebaseRecyclerAdapter<Message, MessagesAdapter.MessageViewHolder> {

    private final int layoutOutgoingMessage;
    private final int layoutIncomingMessage;
    private int layoutSystemMessage;
    private BaseConversationContract.Presenter presenter;
    private final String userId;
    private MessagesHelper messagesHelper;

    public MessagesAdapter(Class<Message> modelClass,
                           int layoutOutgoingMessage,
                           int resourceIncomingMessage,
                           int layoutSystemMessage,
                           Class<MessageViewHolder> viewHolderClass,
                           BaseConversationContract.Presenter presenter,
                           String userId,
                           MessagesHelper messagesHelper) {

        super(modelClass, layoutOutgoingMessage, viewHolderClass, presenter.getChatMessagesReference());
        this.layoutOutgoingMessage = layoutOutgoingMessage;
        this.layoutIncomingMessage = resourceIncomingMessage;
        this.layoutSystemMessage = layoutSystemMessage;
        this.presenter = presenter;
        this.userId = userId;
        this.messagesHelper = messagesHelper;
    }

    @Override
    protected void populateViewHolder(MessageViewHolder viewHolder, Message message, int position) {

        // system messages
        if (message.getSenderId().equals(Message.SYSTEM)) {
            viewHolder.messageView.setText(messagesHelper.buildSystemMessage(message));
        } else {
            // other messages
            viewHolder.messageView.setText(message.getMessage());
        }

        // if group, show the sender
        if (presenter.isGroup() && getItemViewType(position) == layoutIncomingMessage) {
            viewHolder.senderNameView.setVisibility(View.VISIBLE);
            // TODO any better ways to avoid the casting?
            viewHolder.senderNameView.setText(
                   ((GroupConversationContract.Presenter) presenter)
                   .getUserNameFromId(message.getSenderId()));
        }

        viewHolder.dateView.setText(DateUtils.formatTimestamp(message.getTimestamp(),
                Constants.CHAT_DATE_FORMAT));
    }

    /**
     * The {@link FirebaseRecyclerAdapter} actually uses the resource id as the layout to
     * inflate for each item.
     * @param position The adapter position
     * @return The resource id of the layout to use in the position.
     */
    @Override
    public int getItemViewType(int position) {
        Message message = getItem(position);
        if (message.getSenderId().equals(Message.SYSTEM)) {
            return layoutSystemMessage;
        } else if(message.getSenderId().equals(userId)) {
            return layoutOutgoingMessage;
        } else {
            return layoutIncomingMessage;
        }
    }

    /**
     * Must be static for firebase-ui to work correctly
     */
    static class MessageViewHolder extends RecyclerView.ViewHolder {

        @BindView(R.id.message)
        TextView messageView;

        @BindView(R.id.date)
        TextView dateView;

        @BindView(R.id.sender_name_text_view)
        TextView senderNameView;

        public MessageViewHolder(View itemView) {
            super(itemView);

            ButterKnife.bind(this, itemView);
        }
    }

}

[samtstern]

I don't see any obvious issues here, although I am not familiar with the use of ButterKnife in a RecyclerView.

Can you reproduce this leak in the provided sample app by adding LeakCanary? That will tell us if it's something in the library or something in your code specifically causing the leak.

[talosdev]

OK, first of all, there is another leak in the sample app, caused by the AuthStateListener not being properly removed.

I could provide a PR solving this (by removing the listener in the onDestroy() method, is that OK with you, or would you prefer to add/remove it onStart()/onStop().

Once this leak was fixed (for the moment by not adding the listener at all), I was able to reproduce the leak. As before, the leak only happens if the recycler cleanup method is not called.
I believe I can live with knowing that the FirebaseRecyclerAdapter needs to have its cleanup method called at some point.

You can check my fork here
https://github.com/talosdev/FirebaseUI-Android

[samtstern]

@talosdev thanks for all the digging! We do expect cleanup to be called, despite the leakiness of the sample app. If you'd like to submit a PR fixing the leaks I'd be happy to accept it, and for bonus points if you'd like to add a more prominent mention of cleanup() to the documentation I'd accept that as well.