Sdwebimage: Is there a BUG that sd_setImageWithURL completed with a wrong image?

[willbin]

[aActionIV sd_setImageWithURL:tryURL
             placeholderImage:holderImg
                      options:SDWebImageRetryFailed | SDWebImageLowPriority
                     progress:^(NSInteger receivedSize, NSInteger expectedSize)
     {
         if (expectedSize > 0)
         {
             float progress = receivedSize / (float)expectedSize;
             [loadingIndicator setProgress:MAX(MIN(1, progress), 0) animated:YES];
         }
     }
                        completed:^(UIImage *image, NSError *error, SDImageCacheType cacheType, NSURL *imageURL)
     {
         [loadingIndicator removeFromSuperview];

         if (error)
         {
             NSLog(@"%@", error);
         }
         else
         {
         }
     }];

I have some UIImageView that using sd_setImageWithURL to load images.
But after pod update SDWebImage to 3.7.2, sometimes one UIImageView will show wrong image that belongs to other URL.
Just like two UIImageView exchange their images. And when I change SDWebImage to 3.7.1, everything is OK.
So weird!
Now the question,
Is there a BUG that sd_setImageWithURL completed with a wrong image?

[lvpengwei]

I have the same problem.

[timkoma]

One note: progress:^(NSInteger receivedSize, NSInteger expectedSize){} - can be called from background thread - use dispatch_(a)sync(dispatch_get_main_queue(), ^{}) there if you need update UI

[lvpengwei]

@timkoma This problem did not happen in 3.7.1.

[lvpengwei]

@willbin How you reproduced?

[willbin]

@lvpengwei Changing to version 3.7.1 will fix this bug.
But @rs should do something to fix it in 3.7.2

[jjepsen]

I also have this problem. Version 3.7.2 sometimes (error rate around 1/20 for me) sets the wrong image. The issue is not reproducible with version 3.7.1.

[jiekewei]

I,m using Version 3.7.1 also have this problem

[mythodeia]

@jiekewei
people above,@willbin @jjepsen @lvpengwei, have mentioned that with 3.7.1 there is no issue.
Are you sure you are having the same issue with v3.7.1?

[jzaccone]

I have the same problem with 3.7.1. The URL that comes back in sd_setImageWithURL is correct, but the wrong image is cached to that URL.

The handler is called with cacheType = SDImageCacheTypeMemory.

[imageView sd_setImageWithURL:[NSURL URLWithString:url] placeholderImage:placeholderImage completed:^(UIImage *image, NSError *error, SDImageCacheType cacheType, NSURL *imageURL) {
// URL is as expected, but Image is wrong
}];

This problem isn't consistent, but of course once it happens, it loads the wrong Image every time until the cache is cleared

[VersionMismatch]

Did anyone found and confirmed a version were this issue is not happening?

[OnurVar]

maybe related with iOS 9 ?

[timkoma]

I wondering if it may be related to cell reusing in TableView/CollectionView.
Old image load operation completed right after the new one.
I'm aware of the [self sd_cancelCurrentImageLoad]; but maybe it is too late to cancel, and new image is cached on disk- operation is done very quickly and finished download replaces the the cached one.

Canceling is not done very correctly - I think there is a need to wait for canceled callback and then continue with processing new downloaded image

[rrallo]

I'm seeing this on 3.7.4 and the only way I've been able to get around this is to clear the cache much more often. :(

An idea my colleague suggested is to check the MD5 hashes, but this would require sending these with the payload, which will be a pretty big workaround and not very optimal.

I'm seeing exactly what @jzaccone is describing.

My theory is that maybe there's a race condition when writing the images to the cache, as I don't see threading controls in those methods.

Any insight is much appreciated.

[ppaulojr]

I'm seeing this bug with 3.7.5

    [self.badgeView sd_setImageWithURL:[NSURL URLWithString:badge.image_path]];

Sometimes get an empty image

[mythodeia]

@ppaulojr the empty image you are seeing is caused by something else which is discussed here

1401 #1440 #1454 #1457 which is fixed by this https://github.com/rs/SDWebImage/pull/1496

[bpoplauschi]

Closing based on the last comment

[bpoplauschi]

Oh, sorry. The original issue by @willbin is a different one from the one @ppaulojr reported and is fixed by #1496. Reopening

[drakon]

I think I have the same problem. Using version 3.7.5.

Was testing lazy loading images in a table view. I figured that if an image loaded, I scroll to another position in the table and back to the start, the first image is replaced with an image from further down the list. It's pretty easy to reproduce on my side when I slow down the internet connection.

I assume it has something to do with reusing cells and not correctly cancelling downloads. After I added this to the cell it didn't happen again (it works as expected):

override func prepareForReuse() {
    super.prepareForReuse()
    self.imageView.sd_cancelCurrentImageLoad()
}

[mythodeia]

In your prepareforreuse method change the image to nil
without cancelling the operation
On Παρ, 15 Απρ 2016 at 14:04, drakon [email protected] wrote:

I think I have the same problem. Using version 3.7.5.

Was testing lazy loading images in a table view. I figured that if an
image loaded, I scroll to another position in the table and back to the
start, the first image is replaced with an image from further down the
list. It's pretty easy to reproduce on my side when I slow down the
internet connection.

I assume it has something to do with reusing cells and not correctly
cancelling downloads. After I added this to the cell it didn't happen again:

override func prepareForReuse() {
super.prepareForReuse()
self.imageView.sd_cancelCurrentImageLoad()
}

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/rs/SDWebImage/issues/1123#issuecomment-210416619

[drakon]

Why do you mean?

[mythodeia]

in objc

-(void)prepareForReuse
{
    [super prepareForReuse];

   your_imageView.image = nil;
} 

[drakon]

Yeah, I got that. :)

But why do you think it makes a difference to just cancelling? Seems to work like that for me (the image itself is later changed if needed).

The important point to me seems that the download needs to be cancelled, otherwise it will overwrite the image from the current cell (setting to nil may have the same effect though). Which is what this bug is about since it shouldn't be necessary when I understand SDWebimage correctly.

[bpoplauschi]

There are a number of potential causes for this issue:

• 1st one that crosses my mind is #897 where we switched from 2 max concurrent downloads to 6 which may have surfaced an older issue
• another potential cause is #996 that changed a bit the way we load and decompress images.

But the bottom line is: if the solution @mythodeia proposed (prepareToReuse set the image to nil) works, you should use that.
I think this was a must even for older versions of SDWebImage. The reason is if you are using table views or collection views which reuse cells, due to scrolling the same imageView will start requests for different images and we must make sure only the last request is actually finalised and the imageView is always reset before being reused.

Thoughts?

[drakon]

Well, my solution also worked. :) - I don't think it should be necessary though. I'd expected SDWebImage to cancel the previous download if I start a new one on the same UIImageView (or is there a case where you wouldn't want to have this behavior?). The solution with manually resetting works good in general I think. Would need to mention it in the docs (even if it's only until it is fixed).

I think this is what happend in my case:

• Start table view
• First cell is loaded
• First image is requested
• First image is downloaded, inserted and cached (<- this is important I think for it to be reproducible)
• Scroll down
• Same cell is reused
• New image is requested
• Scroll up to first cell
• First image is requested from cache and inserted immediately
• Second image is downloaded and replaces the first image that was loaded from cache
• Now we have the second image in the first cell

But again, I don't think it's super urgent to fix this since the workaround works pretty good even without accessing the source.

[arnaudlamy]

Hi All,

I'm in 3.8 and I still have the same issue, I tried to add the @drakon method to my cellView but without any improvements. I'm a beginner with ios9, so I'm pretty sure I don't understand the workaround here. Any help will be welcome ! :smiley:

Here's my code:

_Controller_

    override func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCellWithIdentifier("MessageCell", forIndexPath: indexPath)
        if let imageView = cell.viewWithTag(104) as? UIImageView {
            if imageView.image == nil{
                print("Update Image:  ", indexPath.row)
                if  messages[indexPath.row].user!.avatarURL != nil {
                    print("AVATAR  ", indexPath.row)
                    imageView.sd_setImageWithURL(messages[indexPath.row].user!.avatarURL)
                }
                else{
                    // TODO: put no avatar image
                    print("NO AVATAR  ", indexPath.row)
                    imageView.image = UIImage(named: "no avatar")
                }
            }
            print("-----------------")
        }
        return cell
    }

_CellView_

class MessageTableViewCell: UITableViewCell {

    @IBOutlet var avatarView: UIImageView?

    override func awakeFromNib() {
        super.awakeFromNib()
        // Initialization code
    }

    override func setSelected(selected: Bool, animated: Bool) {
        super.setSelected(selected, animated: animated)
        // Configure the view for the selected state
    }

    override func prepareForReuse() {
        super.prepareForReuse()
        self.avatarView!.sd_cancelCurrentImageLoad()
    }
}

[mythodeia]

can anyone reproduce this on the latest version?

[jelenadj]

@mythodeia same issue happens to me on 4.0

[mythodeia]

@jelenadj do you have a sample project we can look at?

[MrNiceMrGood]

@jelenadj Try this:

   func setImageAnimated(imageUrl:URL, placeholderImage:UIImage) {
        self.sd_setImage(with: imageUrl, placeholderImage: placeholderImage , options:SDWebImageOptions.avoidAutoSetImage, completed: { (image, error, cacheType, url) in
            if cacheType == SDImageCacheType.none {
                UIView.transition(with: self.superview!, duration: 0.2, options:  [.transitionCrossDissolve ,.allowUserInteraction, .curveEaseIn], animations: {
                    self.image = image
                }, completion: { (completed) in
                })
            } else {
                self.image = image
            }
        })
    }

When reusing cells images download async and are set wrong if you scroll before they are downloaded.
I got 2 solutions working, stoping current image download and setting imageView.image to nil in prepareForReuse or this extension here.

[dreampiggy]

@MrNiceMrGood This is not a bug for SDWebImage. This issue is related to animation, UIView.transition is a animator and will trigger the animation applied to the view itself. But however, you capture one image instance in the block (called image1) when it current showing on cell1. When you scroll down, cell1 is resued and should be reset for showing image2, but the animtor will still use image1 to render it. You should try to fix it by updating the image as well or stop animation.

And the case that @drakon worried(a disk query and download query at the same time), this can not happend because we call sd_cancelImageLoadOperationWithKey before any request. So there should not have such a situation to let the download image replace your disk cache image.

[Omeesh]

use .refreshCached in sdWebImage option to resolve this issue

cell.imageOut.sd_setImage(with: URL(urlPath), placeholderImage: #imageLiteral(resourceName: "imgPlaceholder"), options: [.refreshCached], completed: { (image, error, cache, url) in if error == nil{ cell.imageOut.image = image } } })