72

I'm requesting images from presenter in adapter:

  @Override
  public void onBindViewHolder(SiteAdapter.ViewHolder holder, int position)
  {
    Site site = sites.get(position);
    holder.siteName.setText(site.getName());
    requestHolderLogo(holder, site.getLinks().getLogoUrl());
  }

  private void requestHolderLogo(final ViewHolder holder, final String logoUrl)
  {
    compositeSubscription.add(
      presenter.bitmap(logoUrl)
        .subscribe(
          bitmap -> {
            holder.siteLogo.setImageBitmap(bitmap);
            holder.siteLogo.setVisibility(View.VISIBLE);
          },
          error -> {
            holder.siteName.setVisibility(View.VISIBLE);
          })
    );
  }

I should unsubscribe when ViewHolder is re-used. It is easy.

But how stop all subscription when view is destroyed? I should also probably nullify presenter reference to avoid memory leak

Eugen Martynov
  • 19,888
  • 10
  • 61
  • 114

3 Answers3

32

I think the best way to do that would be to:

  1. Keep a subscription reference in the SiteAdapter.ViewHolder
  2. unsubscribe the subscription object in onBindViewHolder (it's called when the ViewHolder is reused)
  3. Keep the CompositeSubscription object in your adapter
  4. Use the onDetachedFromRecyclerView method of your Adapter to unsubscribe the compositeSubscription

Like so:

public class SiteAdapter extends RecyclerView.Adapter<SiteAdapter.ViewHolder> {

    private CompositeSubscription compositeSubscription = new CompositeSubscription();

    // other needed SiteAdapter methods

    @Override
    public void onBindViewHolder(SiteAdapter.ViewHolder holder, int position) {
        if (holder.subscription != null && !holder.subscription.isUnsubscribed()) {
            compositeSubscription.remove(holder.subscription);
            // this will unsubscribe the subscription as well
        }
        Site site = sites.get(position);
        holder.siteName.setText(site.getName());
        requestHolderLogo(holder, site.getLinks().getLogoUrl());
    }

    private void requestHolderLogo(final SiteAdapter.ViewHolder holder, final String logoUrl) {
        holder.subscription = presenter.bitmap(logoUrl)
                .subscribe(
                        bitmap -> {
                            holder.siteLogo.setImageBitmap(bitmap);
                            holder.siteLogo.setVisibility(View.VISIBLE);
                        },
                        error -> {
                            holder.siteName.setVisibility(View.VISIBLE);
                        });
        compositeSubscription.add(holder.subscription);
    }

    @Override
    public void onDetachedFromRecyclerView(RecyclerView recyclerView) {
        compositeSubscription.unsubscribe();
    }

    public static class ViewHolder extends RecyclerView.ViewHolder {

        public Subscription subscription;

        // some holder-related stuff

        public ViewHolder(View itemView) {
            super(itemView);
            // init holder
        }
    }
}
Bartek Lipinski
  • 30,698
  • 10
  • 94
  • 132
  • 2
    Cool, so there is no method on adapter that is called when `RecyclerView` is detached from activity – Eugen Martynov Jun 07 '16 at 09:25
  • Well... actually there is one - `onDetachedFromRecyclerView` and it could easily do what the `recycle()` currently does. Good idea :) – Bartek Lipinski Jun 07 '16 at 09:29
  • 8
    `onDetachedFromRecyclerView` is being called when you changing adapters, but it is not being called when configuration changed :/ – Miha_x64 Jan 24 '17 at 10:23
  • @Miha_x64 that seems to be like a whole [another issue](http://stackoverflow.com/questions/30132643/recyclerview-doesnt-unregister-itself-from-the-adapter-on-orientation-change) – Bartek Lipinski Jan 24 '17 at 11:26
  • How will you solve the same problem if the subscription is something that keeps listening for more data? For context, i am using firebase data change listener. – Adi Nov 21 '18 at 21:37
  • @Adi and this kind of subscription is kept inside `ViewHolders`? – Bartek Lipinski Nov 21 '18 at 22:42
  • @BartekLipinski Yes because each listener has to update data from a different endpoint. In my case, the listener for a particular viewholder is listening for changes for a chat group id attached to that viewholder and updating the textview. If you have a better design in mind, i will love to know. – Adi Nov 22 '18 at 15:17
  • Where does `siteLogo` comes from? What is it's type? – Victor Queiroz Dec 02 '18 at 23:03
  • @VictorQueiroz this is something that was copied from the question and is "hidden" behind the comment `// some holder-related stuff` – Bartek Lipinski Dec 03 '18 at 10:14
7

For others which have the same problem: viewDetachedFromWindow in the adapter is only called when the adapter is set to null in the onPause (Activity, Fragment) or onDetachFromWindow (Activity, Fragment)

recyclerview.setAdapter(null)

Then you get viewDetachedFromWindow(...) where you can release your internal states and subscriptions. I would setup your subscriptions in on bind, make sure before every bind call you relase old subscriptions as a view can be recycled.

Another possibility is to inflate a custom view instead of only a layout in your factory. Then you can make the cleanup in the custom view onDetachFromWindow(). You get the onDetachedFromWindow also without setting the adapter to null.

eugstman
  • 978
  • 2
  • 15
  • 18
  • How will you solve the same problem if the subscription is something that keeps listening for more data? For context, i am using firebase data change listener. – Adi Nov 21 '18 at 21:37
  • @Adi I would make a custom view and use onDetachFromWindow() – eugstman Nov 26 '18 at 08:36
  • That wouldn't solve the problem i believe. onDetachFromWindow is called when the activity is active and a recycler view is scrolled out of window. But if you close an activity, then this method is not called for the views last visible on window. So there would be memory leak. – Adi Nov 27 '18 at 14:18
  • @Adi onDetachFromWindow is also called when the activity is destroyed for example when you press the android back button. – eugstman Nov 29 '18 at 20:21
  • So much hours wasted on this issue. Thank you. – Rubén Viguera Jun 06 '19 at 13:39
4

One can call public void onViewRecycled(@NonNull VH holder) enter image description here

Susanta
  • 319
  • 2
  • 11