Skip to content

Commit fd36fad

Browse files
authored
Merge pull request #68 from SDWebImage/bugfix/thread_safe_incremental
Fix the thread safe issue for accessing array with index, which may happend during incremental decoding
2 parents 8a0c5e1 + 3775b89 commit fd36fad

File tree

1 file changed

+47
-17
lines changed

1 file changed

+47
-17
lines changed

SDWebImageWebPCoder/Classes/SDImageWebPCoder.m

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,10 @@ - (void)updateIncrementalData:(NSData *)data finished:(BOOL)finished {
331331
webpData.size = _imageData.length;
332332
WebPDemuxState state;
333333
_demux = WebPDemuxPartial(&webpData, &state);
334-
SD_UNLOCK(_lock);
335-
336334
if (_demux && state != WEBP_DEMUX_PARSE_ERROR) {
337335
[self scanAndCheckFramesValidWithDemuxer:_demux];
338336
}
337+
SD_UNLOCK(_lock);
339338
}
340339

341340
- (UIImage *)incrementalDecodedImageWithOptions:(SDImageCoderOptions *)options {
@@ -986,7 +985,6 @@ - (BOOL)scanAndCheckFramesValidWithDemuxer:(WebPDemuxer *)demuxer {
986985
_hasAlpha = hasAlpha;
987986
_canvasWidth = canvasWidth;
988987
_canvasHeight = canvasHeight;
989-
_frameCount = frameCount;
990988
_loopCount = loopCount;
991989

992990
// If static WebP, does not need to parse the frame blend index
@@ -1032,8 +1030,10 @@ - (BOOL)scanAndCheckFramesValidWithDemuxer:(WebPDemuxer *)demuxer {
10321030
WebPDemuxReleaseIterator(&iter);
10331031

10341032
if (frames.count != frameCount) {
1033+
// frames not match, do not override current value
10351034
return NO;
10361035
}
1036+
_frameCount = frameCount;
10371037
_frames = [frames copy];
10381038

10391039
return YES;
@@ -1052,27 +1052,57 @@ - (NSUInteger)animatedImageFrameCount {
10521052
}
10531053

10541054
- (NSTimeInterval)animatedImageDurationAtIndex:(NSUInteger)index {
1055-
if (index >= _frameCount) {
1056-
return 0;
1057-
}
1058-
if (_frameCount <= 1) {
1059-
return 0;
1055+
NSTimeInterval duration;
1056+
// Incremental Animation decoding may update frames when new bytes available
1057+
// Which should use lock to ensure frame count and frames match, ensure atomic logic
1058+
if (_idec != NULL) {
1059+
SD_LOCK(_lock);
1060+
if (index >= _frames.count) {
1061+
SD_UNLOCK(_lock);
1062+
return 0;
1063+
}
1064+
duration = _frames[index].duration;
1065+
SD_UNLOCK(_lock);
1066+
} else {
1067+
if (index >= _frames.count) {
1068+
return 0;
1069+
}
1070+
duration = _frames[index].duration;
10601071
}
1061-
return _frames[index].duration;
1072+
return duration;
10621073
}
10631074

10641075
- (UIImage *)animatedImageFrameAtIndex:(NSUInteger)index {
10651076
UIImage *image;
1066-
if (index >= _frameCount) {
1067-
return nil;
1068-
}
1069-
SD_LOCK(_lock);
1070-
if (_frameCount <= 1) {
1071-
image = [self safeStaticImageFrame];
1077+
// Incremental Animation decoding may update frames when new bytes available
1078+
// Which should use lock to ensure frame count and frames match, ensure atomic logic
1079+
if (_idec != NULL) {
1080+
SD_LOCK(_lock);
1081+
if (index >= _frames.count) {
1082+
SD_UNLOCK(_lock);
1083+
return nil;
1084+
}
1085+
if (_frames.count <= 1) {
1086+
image = [self safeStaticImageFrame];
1087+
} else {
1088+
image = [self safeAnimatedImageFrameAtIndex:index];
1089+
}
1090+
SD_UNLOCK(_lock);
10721091
} else {
1073-
image = [self safeAnimatedImageFrameAtIndex:index];
1092+
// Animation Decoding need a lock on the canvas (which is shared), but the _frames is immutable and no lock needed
1093+
if (index >= _frames.count) {
1094+
return nil;
1095+
}
1096+
if (_frames.count <= 1) {
1097+
SD_LOCK(_lock);
1098+
image = [self safeStaticImageFrame];
1099+
SD_UNLOCK(_lock);
1100+
} else {
1101+
SD_LOCK(_lock);
1102+
image = [self safeAnimatedImageFrameAtIndex:index];
1103+
SD_UNLOCK(_lock);
1104+
}
10741105
}
1075-
SD_UNLOCK(_lock);
10761106
return image;
10771107
}
10781108

0 commit comments

Comments
 (0)