Skip to content

Commit 8226219

Browse files
Miklós FazekasMiklós Fazekas
authored andcommitted
refactor(image): extract -loaders and -decoders accessors, fix canHandleRequest: race
Extract the DCL + init logic for _loaders and _decoders into private -loaders and -decoders accessors. All three readers (-imageURLLoaderForURL:, -imageDataDecoderForData:, -canHandleRequest:) now go through the accessor, closing the race in -canHandleRequest: identified in review and eliminating the duplicated DCL blocks.
1 parent 2728511 commit 8226219

1 file changed

Lines changed: 45 additions & 37 deletions

File tree

packages/react-native/Libraries/Image/RCTImageLoader.mm

Lines changed: 45 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -163,23 +163,17 @@ - (void)setImageCache:(id<RCTImageCache>)cache
163163
_imageCache = cache;
164164
}
165165

166-
- (id<RCTImageURLLoader>)imageURLLoaderForURL:(NSURL *)URL
166+
- (NSArray<id<RCTImageURLLoader>> *)loaders
167167
{
168-
if (!_isLoaderSetup) {
169-
[self setUp];
170-
}
171-
172168
if (!_loadersReady.load(std::memory_order_acquire)) {
173169
std::lock_guard<std::mutex> guard(_loadersMutex);
174170
if (!_loadersReady.load(std::memory_order_relaxed)) {
175-
// Get loaders, sorted in reverse priority order (highest priority first)
176171
if (_loadersProvider) {
177172
_loaders = _loadersProvider(self.moduleRegistry);
178173
} else {
179174
RCTAssert(_bridge, @"Trying to find RCTImageURLLoaders and bridge not set.");
180175
_loaders = [_bridge modulesConformingToProtocol:@protocol(RCTImageURLLoader)];
181176
}
182-
183177
_loaders =
184178
[_loaders sortedArrayUsingComparator:^NSComparisonResult(id<RCTImageURLLoader> a, id<RCTImageURLLoader> b) {
185179
float priorityA = [a respondsToSelector:@selector(loaderPriority)] ? [a loaderPriority] : 0;
@@ -195,7 +189,45 @@ - (void)setImageCache:(id<RCTImageCache>)cache
195189
_loadersReady.store(YES, std::memory_order_release);
196190
}
197191
}
198-
NSArray<id<RCTImageURLLoader>> *loaders = _loaders;
192+
return _loaders;
193+
}
194+
195+
- (NSArray<id<RCTImageDataDecoder>> *)decoders
196+
{
197+
if (!_decodersReady.load(std::memory_order_acquire)) {
198+
std::lock_guard<std::mutex> guard(_loadersMutex);
199+
if (!_decodersReady.load(std::memory_order_relaxed)) {
200+
if (_decodersProvider) {
201+
_decoders = _decodersProvider(self.moduleRegistry);
202+
} else {
203+
RCTAssert(_bridge, @"Trying to find RCTImageDataDecoders and bridge not set.");
204+
_decoders = [_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)];
205+
}
206+
_decoders = [_decoders
207+
sortedArrayUsingComparator:^NSComparisonResult(id<RCTImageDataDecoder> a, id<RCTImageDataDecoder> b) {
208+
float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0;
209+
float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0;
210+
if (priorityA > priorityB) {
211+
return NSOrderedAscending;
212+
} else if (priorityA < priorityB) {
213+
return NSOrderedDescending;
214+
} else {
215+
return NSOrderedSame;
216+
}
217+
}];
218+
_decodersReady.store(YES, std::memory_order_release);
219+
}
220+
}
221+
return _decoders;
222+
}
223+
224+
- (id<RCTImageURLLoader>)imageURLLoaderForURL:(NSURL *)URL
225+
{
226+
if (!_isLoaderSetup) {
227+
[self setUp];
228+
}
229+
230+
NSArray<id<RCTImageURLLoader>> *loaders = [self loaders];
199231

200232
if (RCT_DEBUG) {
201233
// Check for handler conflicts
@@ -244,34 +276,7 @@ - (void)setImageCache:(id<RCTImageCache>)cache
244276
[self setUp];
245277
}
246278

247-
if (!_decodersReady.load(std::memory_order_acquire)) {
248-
std::lock_guard<std::mutex> guard(_loadersMutex);
249-
if (!_decodersReady.load(std::memory_order_relaxed)) {
250-
// Get decoders, sorted in reverse priority order (highest priority first)
251-
252-
if (_decodersProvider) {
253-
_decoders = _decodersProvider(self.moduleRegistry);
254-
} else {
255-
RCTAssert(_bridge, @"Trying to find RCTImageDataDecoders and bridge not set.");
256-
_decoders = [_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)];
257-
}
258-
259-
_decoders = [_decoders
260-
sortedArrayUsingComparator:^NSComparisonResult(id<RCTImageDataDecoder> a, id<RCTImageDataDecoder> b) {
261-
float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0;
262-
float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0;
263-
if (priorityA > priorityB) {
264-
return NSOrderedAscending;
265-
} else if (priorityA < priorityB) {
266-
return NSOrderedDescending;
267-
} else {
268-
return NSOrderedSame;
269-
}
270-
}];
271-
_decodersReady.store(YES, std::memory_order_release);
272-
}
273-
}
274-
NSArray<id<RCTImageDataDecoder>> *decoders = _decoders;
279+
NSArray<id<RCTImageDataDecoder>> *decoders = [self decoders];
275280

276281
if (RCT_DEBUG) {
277282
// Check for handler conflicts
@@ -1182,7 +1187,10 @@ - (BOOL)canHandleRequest:(NSURLRequest *)request
11821187
return NO;
11831188
}
11841189

1185-
for (id<RCTImageURLLoader> loader in _loaders) {
1190+
if (!_isLoaderSetup) {
1191+
[self setUp];
1192+
}
1193+
for (id<RCTImageURLLoader> loader in [self loaders]) {
11861194
// Don't use RCTImageURLLoader protocol for modules that already conform to
11871195
// RCTURLRequestHandler as it's inefficient to decode an image and then
11881196
// convert it back into data

0 commit comments

Comments
 (0)