Skip to content

Commit 3a6f312

Browse files
committed
Fix the thread safe issue for accessing array with index, which may happend during incremental decoding
1 parent 8a0c5e1 commit 3a6f312

File tree

1 file changed

+48
-15
lines changed

1 file changed

+48
-15
lines changed

SDWebImageWebPCoder/Classes/SDImageWebPCoder.m

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,6 @@ - (BOOL)scanAndCheckFramesValidWithDemuxer:(WebPDemuxer *)demuxer {
986986
_hasAlpha = hasAlpha;
987987
_canvasWidth = canvasWidth;
988988
_canvasHeight = canvasHeight;
989-
_frameCount = frameCount;
990989
_loopCount = loopCount;
991990

992991
// If static WebP, does not need to parse the frame blend index
@@ -1032,9 +1031,13 @@ - (BOOL)scanAndCheckFramesValidWithDemuxer:(WebPDemuxer *)demuxer {
10321031
WebPDemuxReleaseIterator(&iter);
10331032

10341033
if (frames.count != frameCount) {
1034+
// frames not match, do not override current value
10351035
return NO;
10361036
}
1037+
_frameCount = frameCount;
1038+
SD_LOCK(_lock);
10371039
_frames = [frames copy];
1040+
SD_UNLOCK(_lock);
10381041

10391042
return YES;
10401043
}
@@ -1052,27 +1055,57 @@ - (NSUInteger)animatedImageFrameCount {
10521055
}
10531056

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

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

0 commit comments

Comments
 (0)