From 6e79ef51f0ec950f5428770f4d503443bfd95222 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Fri, 10 Aug 2018 12:52:31 +0800 Subject: [PATCH] Fix a race condition during progressive animation load in SDAnimatedImageView. 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 --- SDWebImage/SDAnimatedImageView.m | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/SDWebImage/SDAnimatedImageView.m b/SDWebImage/SDAnimatedImageView.m index ebbffd78..99bebeee 100644 --- a/SDWebImage/SDAnimatedImageView.m +++ b/SDWebImage/SDAnimatedImageView.m @@ -621,20 +621,28 @@ static NSUInteger SDDeviceFreeMemory() { return; } if ([image conformsToProtocol:@protocol(SDAnimatedImage)] && image.sd_isIncremental) { - NSData *currentData = [((UIImage *)image) animatedImageData]; - if (currentData) { - NSData *previousData; - if ([self.image conformsToProtocol:@protocol(SDAnimatedImage)]) { - previousData = [((UIImage *)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 *)previousImage) animatedImageData]; + NSData *currentData = [((UIImage *)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; } } }