Skip to content

Commit

Permalink
Fix a race condition during progressive animation load in SDAnimatedI…
Browse files Browse the repository at this point in the history
…mageView.

When the coder was updated, currentData may not be the same instance as previousdData. We should check the that current data is appended by previous data
  • Loading branch information
dreampiggy committed Aug 10, 2018
1 parent 802e19b commit 6e79ef5
Showing 1 changed file with 20 additions and 12 deletions.
32 changes: 20 additions & 12 deletions SDWebImage/SDAnimatedImageView.m
Expand Up @@ -621,20 +621,28 @@ - (void)updateIsProgressiveWithImage:(UIImage *)image
return;
}
if ([image conformsToProtocol:@protocol(SDAnimatedImage)] && image.sd_isIncremental) {
NSData *currentData = [((UIImage<SDAnimatedImage> *)image) animatedImageData];
if (currentData) {
NSData *previousData;
if ([self.image conformsToProtocol:@protocol(SDAnimatedImage)]) {
previousData = [((UIImage<SDAnimatedImage> *)self.image) animatedImageData];
}
// Check whether to use progressive coding
if (!previousData) {
// If previous data is nil
self.isProgressive = YES;
} else if ([currentData isEqualToData:previousData]) {
// If current data is equal to previous data
UIImage *previousImage = self.image;
if ([previousImage conformsToProtocol:@protocol(SDAnimatedImage)] && previousImage.sd_isIncremental) {
NSData *previousData = [((UIImage<SDAnimatedImage> *)previousImage) animatedImageData];
NSData *currentData = [((UIImage<SDAnimatedImage> *)image) animatedImageData];
// Check whether to use progressive rendering or not

// Warning: normally the `previousData` is same instance as `currentData` because our `SDAnimatedImage` class share the same `coder` instance internally. But there may be a race condition, that later retrived `currentData` is already been updated and it's not the same instance as `previousData`.
// And for protocol extensible design, we should not assume `SDAnimatedImage` protocol implementations always share same instance. So both of two reasons, we need that `rangeOfData` check.
if ([currentData isEqualToData:previousData]) {
// If current data is the same data (or instance) as previous data
self.isProgressive = YES;
} else if (currentData.length > previousData.length) {
// If current data is appended by previous data, use `NSDataSearchAnchored`
NSRange range = [currentData rangeOfData:previousData options:NSDataSearchAnchored range:NSMakeRange(0, previousData.length)];
if (range.location == 0 && range.length == previousData.length) {
// Contains hole previous data and they start with the same beginning
self.isProgressive = YES;
}
}
} else {
// Previous image is not progressive, so start progressive rendering
self.isProgressive = YES;
}
}
}
Expand Down

0 comments on commit 6e79ef5

Please sign in to comment.