Authored by Bogdan Poplauschi
Committed by GitHub

Merge pull request #2274 from dreampiggy/fix_coders_array

Fix the thread-safe issue for coders manager.
@@ -39,7 +39,7 @@ @@ -39,7 +39,7 @@
39 /** 39 /**
40 All coders in coders manager. The coders array is a priority queue, which means the later added coder will have the highest priority 40 All coders in coders manager. The coders array is a priority queue, which means the later added coder will have the highest priority
41 */ 41 */
42 -@property (nonatomic, strong, readwrite, nullable) NSArray<id<SDWebImageCoder>> *coders; 42 +@property (nonatomic, copy, readwrite, nullable) NSArray<id<SDWebImageCoder>> *coders;
43 43
44 /** 44 /**
45 Add a new coder to the end of coders array. Which has the highest priority. 45 Add a new coder to the end of coders array. Which has the highest priority.
@@ -13,10 +13,12 @@ @@ -13,10 +13,12 @@
13 #import "SDWebImageWebPCoder.h" 13 #import "SDWebImageWebPCoder.h"
14 #endif 14 #endif
15 15
  16 +#define LOCK(lock) dispatch_semaphore_wait(lock, DISPATCH_TIME_FOREVER);
  17 +#define UNLOCK(lock) dispatch_semaphore_signal(lock);
  18 +
16 @interface SDWebImageCodersManager () 19 @interface SDWebImageCodersManager ()
17 20
18 -@property (strong, nonatomic, nonnull) NSMutableArray<SDWebImageCoder>* mutableCoders;  
19 -@property (strong, nonatomic, nullable) dispatch_queue_t mutableCodersAccessQueue; 21 +@property (nonatomic, strong, nonnull) dispatch_semaphore_t codersLock;
20 22
21 @end 23 @end
22 24
@@ -34,11 +36,12 @@ @@ -34,11 +36,12 @@
34 - (instancetype)init { 36 - (instancetype)init {
35 if (self = [super init]) { 37 if (self = [super init]) {
36 // initialize with default coders 38 // initialize with default coders
37 - _mutableCoders = [@[[SDWebImageImageIOCoder sharedCoder]] mutableCopy]; 39 + NSMutableArray<id<SDWebImageCoder>> *mutableCoders = [@[[SDWebImageImageIOCoder sharedCoder]] mutableCopy];
38 #ifdef SD_WEBP 40 #ifdef SD_WEBP
39 - [_mutableCoders addObject:[SDWebImageWebPCoder sharedCoder]]; 41 + [mutableCoders addObject:[SDWebImageWebPCoder sharedCoder]];
40 #endif 42 #endif
41 - _mutableCodersAccessQueue = dispatch_queue_create("com.hackemist.SDWebImageCodersManager", DISPATCH_QUEUE_CONCURRENT); 43 + _coders = [mutableCoders copy];
  44 + _codersLock = dispatch_semaphore_create(1);
42 } 45 }
43 return self; 46 return self;
44 } 47 }
@@ -46,36 +49,36 @@ @@ -46,36 +49,36 @@
46 #pragma mark - Coder IO operations 49 #pragma mark - Coder IO operations
47 50
48 - (void)addCoder:(nonnull id<SDWebImageCoder>)coder { 51 - (void)addCoder:(nonnull id<SDWebImageCoder>)coder {
49 - if ([coder conformsToProtocol:@protocol(SDWebImageCoder)]) {  
50 - dispatch_barrier_sync(self.mutableCodersAccessQueue, ^{  
51 - [self.mutableCoders addObject:coder];  
52 - }); 52 + if (![coder conformsToProtocol:@protocol(SDWebImageCoder)]) {
  53 + return;
  54 + }
  55 + LOCK(self.codersLock);
  56 + NSMutableArray<id<SDWebImageCoder>> *mutableCoders = [self.coders mutableCopy];
  57 + if (!mutableCoders) {
  58 + mutableCoders = [NSMutableArray array];
53 } 59 }
  60 + [mutableCoders addObject:coder];
  61 + self.coders = [mutableCoders copy];
  62 + UNLOCK(self.codersLock);
54 } 63 }
55 64
56 - (void)removeCoder:(nonnull id<SDWebImageCoder>)coder { 65 - (void)removeCoder:(nonnull id<SDWebImageCoder>)coder {
57 - dispatch_barrier_sync(self.mutableCodersAccessQueue, ^{  
58 - [self.mutableCoders removeObject:coder];  
59 - });  
60 -}  
61 -  
62 -- (NSArray<id<SDWebImageCoder>> *)coders {  
63 - __block NSArray<id<SDWebImageCoder>> *sortedCoders = nil;  
64 - dispatch_sync(self.mutableCodersAccessQueue, ^{  
65 - sortedCoders = (NSArray<id<SDWebImageCoder>> *)[[[self.mutableCoders copy] reverseObjectEnumerator] allObjects];  
66 - });  
67 - return sortedCoders;  
68 -}  
69 -  
70 -- (void)setCoders:(NSArray<id<SDWebImageCoder>> *)coders {  
71 - dispatch_barrier_sync(self.mutableCodersAccessQueue, ^{  
72 - self.mutableCoders = [coders mutableCopy];  
73 - }); 66 + if (![coder conformsToProtocol:@protocol(SDWebImageCoder)]) {
  67 + return;
  68 + }
  69 + LOCK(self.codersLock);
  70 + NSMutableArray<id<SDWebImageCoder>> *mutableCoders = [self.coders mutableCopy];
  71 + [mutableCoders removeObject:coder];
  72 + self.coders = [mutableCoders copy];
  73 + UNLOCK(self.codersLock);
74 } 74 }
75 75
76 #pragma mark - SDWebImageCoder 76 #pragma mark - SDWebImageCoder
77 - (BOOL)canDecodeFromData:(NSData *)data { 77 - (BOOL)canDecodeFromData:(NSData *)data {
78 - for (id<SDWebImageCoder> coder in self.coders) { 78 + LOCK(self.codersLock);
  79 + NSArray<id<SDWebImageCoder>> *coders = self.coders;
  80 + UNLOCK(self.codersLock);
  81 + for (id<SDWebImageCoder> coder in coders.reverseObjectEnumerator) {
79 if ([coder canDecodeFromData:data]) { 82 if ([coder canDecodeFromData:data]) {
80 return YES; 83 return YES;
81 } 84 }
@@ -84,7 +87,10 @@ @@ -84,7 +87,10 @@
84 } 87 }
85 88
86 - (BOOL)canEncodeToFormat:(SDImageFormat)format { 89 - (BOOL)canEncodeToFormat:(SDImageFormat)format {
87 - for (id<SDWebImageCoder> coder in self.coders) { 90 + LOCK(self.codersLock);
  91 + NSArray<id<SDWebImageCoder>> *coders = self.coders;
  92 + UNLOCK(self.codersLock);
  93 + for (id<SDWebImageCoder> coder in coders.reverseObjectEnumerator) {
88 if ([coder canEncodeToFormat:format]) { 94 if ([coder canEncodeToFormat:format]) {
89 return YES; 95 return YES;
90 } 96 }
@@ -93,10 +99,10 @@ @@ -93,10 +99,10 @@
93 } 99 }
94 100
95 - (UIImage *)decodedImageWithData:(NSData *)data { 101 - (UIImage *)decodedImageWithData:(NSData *)data {
96 - if (!data) {  
97 - return nil;  
98 - }  
99 - for (id<SDWebImageCoder> coder in self.coders) { 102 + LOCK(self.codersLock);
  103 + NSArray<id<SDWebImageCoder>> *coders = self.coders;
  104 + UNLOCK(self.codersLock);
  105 + for (id<SDWebImageCoder> coder in coders.reverseObjectEnumerator) {
100 if ([coder canDecodeFromData:data]) { 106 if ([coder canDecodeFromData:data]) {
101 return [coder decodedImageWithData:data]; 107 return [coder decodedImageWithData:data];
102 } 108 }
@@ -110,7 +116,10 @@ @@ -110,7 +116,10 @@
110 if (!image) { 116 if (!image) {
111 return nil; 117 return nil;
112 } 118 }
113 - for (id<SDWebImageCoder> coder in self.coders) { 119 + LOCK(self.codersLock);
  120 + NSArray<id<SDWebImageCoder>> *coders = self.coders;
  121 + UNLOCK(self.codersLock);
  122 + for (id<SDWebImageCoder> coder in coders.reverseObjectEnumerator) {
114 if ([coder canDecodeFromData:*data]) { 123 if ([coder canDecodeFromData:*data]) {
115 return [coder decompressedImageWithImage:image data:data options:optionsDict]; 124 return [coder decompressedImageWithImage:image data:data options:optionsDict];
116 } 125 }
@@ -122,7 +131,10 @@ @@ -122,7 +131,10 @@
122 if (!image) { 131 if (!image) {
123 return nil; 132 return nil;
124 } 133 }
125 - for (id<SDWebImageCoder> coder in self.coders) { 134 + LOCK(self.codersLock);
  135 + NSArray<id<SDWebImageCoder>> *coders = self.coders;
  136 + UNLOCK(self.codersLock);
  137 + for (id<SDWebImageCoder> coder in coders.reverseObjectEnumerator) {
126 if ([coder canEncodeToFormat:format]) { 138 if ([coder canEncodeToFormat:format]) {
127 return [coder encodedDataWithImage:image format:format]; 139 return [coder encodedDataWithImage:image format:format];
128 } 140 }