search

pinterest / PINRemoteImage

A thread safe, performant, feature rich image fetcher
Apache License 2.0
4.01k stars 511 forks source link

Fixes re-downloading data corrupt for the same url #514

Closed zhongwuzw closed 4 years ago

zhongwuzw commented 5 years ago

Fixes #489 , we use cacheKey to get the task, actually it would mix up data if previous task cancelled but download delegate still not finished , and next task with the same url started at the same time. This is the race condition for the same url downloading, we can keep the weak reference to task and get it in session delegate.

bolsinga commented 5 years ago

Please add a test.

garrettmoon commented 5 years ago

This is a really great fix! Would you mind adding a test as @bolsinga suggested?

zhongwuzw commented 5 years ago

@bolsinga @garrettmoon Please review again. :)

bolsinga commented 5 years ago

Thanks for the test!

garrettmoon commented 5 years ago

Can you add an entry to CHANGELOG.md for this?

ghost commented 5 years ago

🚫 CI failed with log

garrettmoon commented 5 years ago

Mind rebasing against master? I think I fixed a flakey test.

ghost commented 5 years ago

🚫 CI failed with log

zhongwuzw commented 5 years ago

@garrettmoon Tests failed like below frequently, I think we may need to add a command like rm -rf ~/Library/Developer/Xcode/DerivedData/ before build project in CI?

image
jparise commented 4 years ago

Sorry for the long delay @zhongwuzw. The CI tests should now be fixed. If you could rebase again, we can do a final review of your change.

zhongwuzw commented 4 years ago

@jparise Done.

zhongwuzw commented 4 years ago

This is looking really good, @zhongwuzw! Just a few more bits of feedback.

Done.

garrettmoon commented 4 years ago

This looks great, thank you so much for the fix!