Asynchronous image loading in ListView: old images shown

Question

I have a ListView with the following adapter:

private class ListAdapter extends BaseAdapter {
        private List<Item> items;
        private ImageCache imageCache;

        public ListAdapter(List<Item> item) {
            this.items=items;
            imageCache=ImageCache.getInstance();
        }

        //nothing special here
        public View getView(int position, View convertView, ViewGroup parent) {
            ViewHolder viewHolder;

            if (convertView == null) {
                convertView = getLayoutInflater().inflate(R.layout.listitem, parent, false);

                viewHolder = new ViewHolder();
                viewHolder.imgView = (ImageView) convertView.findViewById(R.id.imgview);
                viewHolder.txtView = (TextView) convertView.findViewById(R.id.txtview);

                convertView.setTag(viewHolder);
            } else  {
                viewHolder = (ViewHolder) convertView.getTag();
            }

            Item item = getItem(position);

                    //this method is used to start download
            imageCache.load(item.getImageURL(), viewHolder.imgView);

            viewHolder.txtView.setText(item.getText());

            return convertView;
        }
    }

Below is my ImageCache class. It relies on android.util.LruCache and Jake Wharton's DiskLruCache; If item is not found in memory and disk cache, it is downloaded using AsyncTask.

private Map<String,WeakReference<ImageView>> references;

//...
public void load(String imageUrl, ImageView imgView) {
    final String key=getKey(imageUrl);
    references.put(key, new WeakReference<ImageView>(imgView)); 
    this.get(imageUrl, new OnImageLoadedListener() {
        public void onImageLoaded(Bitmap image) {
            WeakReference<ImageView> imgViewRef=references.get(key);
            if(imgViewRef!=null && imgViewRef.get()!=null) {
                imgViewRef.get().setImageBitmap(image);
            } else {
                Log.d("Set image", "Dead reference! setting bitmap cancelled");
            }
        }
    });
}

As you see, I'm trying to use WeakReference to my ImageViews to set image only if ImageView is still alive and to avoid leaking view references from ListView. However, This doesn't seem to work. Images are loaded normally for the first few visible items, but when I start scrolling my ListView, I see incorrect images (the ones used in previous list items), and that's annoying. How can I avoid this? As soon as download finishes, image is replaced with correct one, but download might take a while.

An obvious workaround is to hide ImageView until download is finished, and then show it, but is there any other way?

---

EDIT: Some other methods of ImageCache class:

//used to track requests for each image URL, key is URL
private Map<String, Queue<OnImageLoadedListener>> listeners;

//...

public void get(String imageUrl, OnImageLoadedListener listener) {
    final String key=this.getKey(imageUrl);
    Bitmap image;

    Log.d(LOG_TAG, "image "+key+" for "+listener.hashCode()+": requesting...");
    if (!listeners.containsKey(key)) {
        listeners.put(key, new LinkedList<OnImageLoadedListener>());
    }

    Queue<OnImageLoadedListener> imageListenersQueue=listeners.get(key);
    imageListenersQueue.add(listener);

    //check if there are other requests for the same image
    if (imageListenersQueue.size()>1) {
        Log.d(LOG_TAG, "image "+key+" for "+listener.hashCode()+": already requested...");
                    return;
    }

    //proceed only if this request is the first
            Log.d(LOG_TAG, "image "+key+" for "+listener.hashCode()+": proceeding...");
    image=memCache.get(key);
    if (image==null && diskCache!=null) {
        image=diskCache.getBitmap(key);

        if (image==null) {
            new BitmapDownloader(new OnImageLoadedListener() {
                public void onImageLoaded(Bitmap image) {
                    if (image!=null) {
                        memCache.put(key, image);
                        diskCache.put(key, image);
                    }
                    post(key, image);
                                            Log.d(LOG_TAG, "image "+key+", downloaded bitmap "+image.hashCode());
                }
            }).execute(imageUrl);
        } else {
            memCache.put(key, image);
                            Log.d(LOG_TAG, "image "+key+" found in diskcache");
            post(key, image);
        }
    } else {
                    Log.d(LOG_TAG, "image "+key+" found in memcache");
        post(key, image);
    }
}

private void post(String key, Bitmap bitmap) {
    //will now pass bitmap to all listeners
    LinkedList<OnImageLoadedListener> imageListenersQueue=(LinkedList<OnImageLoadedListener>) listeners.get(key);
    while (!imageListenersQueue.isEmpty()) {
        imageListenersQueue.remove().onImageLoaded(bitmap);
    }
}

//and the mysterious getKey()
private String getKey(String str) {
    return Integer.toHexString(str.hashCode());
}

And here's the log output for ImageCache:

First visible list items:

12-05 12:30:06.036: D/ImageCache(21138): image 40bc8de0 for 1104135776: requesting...
12-05 12:30:06.036: D/ImageCache(21138): image 40bc8de0 for 1104135776: proceeding...
12-05 12:30:06.071: D/ImageCache(21138): image 6589e90 for 1103929688: requesting...
12-05 12:30:06.071: D/ImageCache(21138): image 6589e90 for 1103929688: proceeding...
12-05 12:30:07.251: D/ImageCache(21138): image 40bc8de0 for 1104135776 ready, setting bitmap 1104282656
12-05 12:30:07.255: D/ImageCache(21138): image 40bc8de0, downloaded bitmap 1104282656
12-05 12:30:08.407: D/ImageCache(21138): image 6589e90 for 1103929688 ready, setting bitmap 1104663160
12-05 12:30:08.411: D/ImageCache(21138): image 6589e90, downloaded bitmap 1104663160

When scrolling to the next item:

12-05 12:30:44.669: D/ImageCache(21138): image 2e762e6c for 1104686624: requesting...
12-05 12:30:44.673: D/ImageCache(21138): image 2e762e6c for 1104686624: proceeding...
12-05 12:30:45.997: D/ImageCache(21138): image 2e762e6c for 1104686624 ready, setting bitmap 1104089312
12-05 12:30:45.997: D/ImageCache(21138): image 2e762e6c, downloaded bitmap 1104089312

Seems like the problem is not in the caching mechanism, since the only load() call for this item gives me correct image set exactly to requested ImageView. It's just that incorrect image is displayed until it is downloaded and set.

Answer 1

Two things need to be taken account for here. First, when you do not find the target Bitmap in your cache, you should set the ImageView source to some sort of loading drawable. This is pretty straight forward.

Second, you need to take view recycling into account. There is nothing in your code that prevents the scenario where you begin a download for an ImageView, but before onImageLoaded() is called, the View from your adapter gets recycled to display some other image. Then, the download completes, resulting in the totally wrong image being displayed.

A typical solution to this is to call setTag() on your ImageView, and set the URL that it should load. Do this in the beginning of your load() method. Then, you probably want to add an extra parameter to onImageLoaded(), for the URL. Check if this URL matches the ImageView tag, only calling setImageBitmap() if the URL matches.