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

Not all completion blocks called when scrolling really fast #665

Closed
bparol opened this issue May 8, 2017 · 3 comments · Fixed by #666
Closed

Not all completion blocks called when scrolling really fast #665

bparol opened this issue May 8, 2017 · 3 comments · Fixed by #666

Comments

@bparol
Copy link

bparol commented May 8, 2017

Issue Description

Kingfisher doesn't call all completionBlocks when a lot of images is loaded at once in a very short period of time.

Reproduce

  1. Use "Demo" for iOS
  2. Change numer of items to greater number (i.e.120 - 200)
  3. Use other picture source, i.e. replace url in collectionView:willDisplayCell with the following
    let url = URL(string: "https://dummyimage.com/100/888888/\(indexPath.row)")!
  4. Scroll really fast to the bottom
  5. Count how many times completion block was invoked (i.e. type "finished" in console log filter)

Other Comment

In my test in most cases completion block was not called couple of times. I use Kingfisher in production app (btw. I love it!) and this issue makes impossible to properly manage network activity indicator.
In my project I didn't notice leaks - all ImageDownloaders were deallocated properly, no leaks on ImageView and URLSessions. I tried to find solution, unfortunately I wasn't able.

Thank you @onevcat !

@onevcat
Copy link
Owner

onevcat commented May 9, 2017

@parbo86 Hi, thanks for reporting this.

However, I tried your steps but I cannot reproduce the issue. All completion blocks are called for me. Here is the snippet I am using in the collection view cell willDisplay method:

print("\(indexPath.row + 1): Started")
let url = URL(string: "https://dummyimage.com/100/888888/\(indexPath.row + 1).jpg")!

 _ = (cell as! CollectionViewCell).cellImageView.kf.setImage(with: url,
                                       placeholder: nil,
                                       options: [.transition(ImageTransition.fade(1))],
                                       completionHandler: { image, error, cacheType, imageURL in
                                            print("\(indexPath.row + 1): Finished")
})

And after several try and double check, I could pair every "Started" and "Finished" without problem. Could you confirm that you have the same number of "Started" and "Finished"? Maybe the UIKit would skip some "willDisplay" method when you scrolling too fast in a device? (I am not sure about it)

Another possible reason might be the slow connection? There is a 15 seconds timeout duration. If the downloading progress gets stuck, you may need to wait for 15 seconds before the completion handler called (with a timeout error).

If none of above could explain this, does this issue happens only in a specified device type/system version? I tried it in an iPhone 7 simulator with iOS 10.3. May I know your environment?

@onevcat
Copy link
Owner

onevcat commented May 9, 2017

OK, I think I found the reason. This is due to we are not holding the base image view here.

The completion handler should be always called, however, we do not want to hold the image view since it is a waste if the image view is already get dealloced.

I will give it a patch soon. Thanks again for reporting this!

@bparol
Copy link
Author

bparol commented May 9, 2017

Thank you very much @onevcat !

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

Successfully merging a pull request may close this issue.

2 participants