Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed race condition betwen operation cancelation and loading finish #621

Closed
wants to merge 1 commit into from

Conversation

akavalera
Copy link

When using UICollectionView that uses SDWebImage to download content and scroll it fast there are much operation cancelations and when you stop scrolling part of images would not be loaded.

It is because of race condition between connectionDidFinishLoading and cancel methods in SDWebImageDownloaderOperation that leads to NSURLErrorTimedOut instead of image loads.

The problem is 2 calls of CFRunLoopStop() by one from each of the methods. First cancels current thread and next some thread that starts download of other image.

@Lings
Copy link

Lings commented Feb 11, 2014

It's resolved the problem in iOS 7, but still exist in iOS 6.

@akavalera
Copy link
Author

I tested it on iOS 6, 6.1 and 7 and it works fine for my app. I am using it with UICollectionView cells and the only difference from standard workflow is that I am loading low resolution image first and replacing it with high resolution image right after that.

@Lings
Copy link

Lings commented Feb 11, 2014

I test it again, the problem already exist, whatever on iOS 7 and iOS 6.

I test it on my iPhone 5s (iOS 7) and iPhone4 (iOS 6), on iPhone4 the problem appear easily, on iPhone 5s, I need scroll very very fast can make the problem appear.

It always stops in this line:

        if (!self.isFinished) {
            [self.connection cancel];
            [self connection:self.connection didFailWithError:[NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorTimedOut userInfo:@{NSURLErrorFailingURLErrorKey : self.request.URL}]];
        }

I think the reason is CFRunLoopStop. another operation's cancel method trigger the CFRunLoopStop, then the first operation continue run to the code above.

But I have not get the resolution yet.

@akavalera
Copy link
Author

Have you merged my commit with the fix? https://github.com/rs/SDWebImage/pull/621/files

@Lings
Copy link

Lings commented Feb 11, 2014

YES , I merged, you can test it with the SDWebImage demo.

@akavalera
Copy link
Author

I have checked the demo app, it has issue you are talking about. I used Charles to understand what is happening. Look at the attached screenshot please. Sometimes the image hosting returns 404 error instead of image. Please try on your own image set.
404

@Lings
Copy link

Lings commented Feb 11, 2014

I used my own image set. 404 is image host problem, but my images are all correct.
You need test it on iPhone, not simulator. scroll screen as fast as possible.

@akavalera
Copy link
Author

As I sayer before it works fine on my side. Can you post a part of code where you are loading image? Are you using UIImageView+WebCache.h category?

@Lings
Copy link

Lings commented Feb 11, 2014

yes, I just use sdwebimage demo, and only replace the sample image set.

@bpoplauschi
Copy link
Member

Guys, what is the current status of this issue? Does it still exist? Is this pull request the proper fix? If so, you need to rebase it on top of master so we can merge it.

@bpoplauschi
Copy link
Member

Since this pull request didn't get updated, replaced it with #783

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants