From 2728511bd61dd9202c477774330e89a96e2f8848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Sat, 20 Jun 2026 19:30:45 +0200 Subject: [PATCH 1/2] fix(image): fix data race in imageURLLoaderForURL and imageDataDecoderForData imageURLLoaderForURL: had a broken double-checked lock on _loaders: the outer if (!_loaders) guard and the for-loop iteration both ran without _loadersMutex, while the assignment ran inside it. imageDataDecoderForData: had no lock at all on _decoders. Both methods write the ivar in two steps (raw list then sorted), so a concurrent reader could observe the intermediate value and iterate a concurrently-released array, crashing with EXC_BAD_ACCESS in objc_release. Fix mirrors the setUp() pattern from #46153: add std::atomic flags (_loadersReady / _decodersReady) as acquire/release guards, protect the slow path with _loadersMutex, and iterate a local strong ref captured under the lock so the shared ivar is never read outside it. Verified with a reproducer (crashes 5/5 without the fix, 0/10 with it): https://github.com/mfazekas/rn-rctimageloader-dcl-repro Fixes: #57296 See also: #46115, #46153 --- .../Libraries/Image/RCTImageLoader.mm | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/packages/react-native/Libraries/Image/RCTImageLoader.mm b/packages/react-native/Libraries/Image/RCTImageLoader.mm index 5cba225f0164..d500d99635ac 100644 --- a/packages/react-native/Libraries/Image/RCTImageLoader.mm +++ b/packages/react-native/Libraries/Image/RCTImageLoader.mm @@ -73,7 +73,9 @@ @implementation RCTImageLoader { NSArray> * (^_loadersProvider)(RCTModuleRegistry *); NSArray> * (^_decodersProvider)(RCTModuleRegistry *); NSArray> *_loaders; + std::atomic _loadersReady; NSArray> *_decoders; + std::atomic _decodersReady; NSOperationQueue *_imageDecodeQueue; dispatch_queue_t _URLRequestQueue; id _imageCache; @@ -167,9 +169,9 @@ - (void)setImageCache:(id)cache [self setUp]; } - if (!_loaders) { - std::unique_lock guard(_loadersMutex); - if (!_loaders) { + if (!_loadersReady.load(std::memory_order_acquire)) { + std::lock_guard guard(_loadersMutex); + if (!_loadersReady.load(std::memory_order_relaxed)) { // Get loaders, sorted in reverse priority order (highest priority first) if (_loadersProvider) { _loaders = _loadersProvider(self.moduleRegistry); @@ -190,14 +192,16 @@ - (void)setImageCache:(id)cache return NSOrderedSame; } }]; + _loadersReady.store(YES, std::memory_order_release); } } + NSArray> *loaders = _loaders; if (RCT_DEBUG) { // Check for handler conflicts float previousPriority = 0; id previousLoader = nil; - for (id loader in _loaders) { + for (id loader in loaders) { float priority = [loader respondsToSelector:@selector(loaderPriority)] ? [loader loaderPriority] : 0; if (previousLoader && priority < previousPriority) { return previousLoader; @@ -224,7 +228,7 @@ - (void)setImageCache:(id)cache } // Normal code path - for (id loader in _loaders) { + for (id loader in loaders) { if ([loader canLoadImageURL:URL]) { return loader; } @@ -240,35 +244,40 @@ - (void)setImageCache:(id)cache [self setUp]; } - if (!_decoders) { - // Get decoders, sorted in reverse priority order (highest priority first) + if (!_decodersReady.load(std::memory_order_acquire)) { + std::lock_guard guard(_loadersMutex); + if (!_decodersReady.load(std::memory_order_relaxed)) { + // Get decoders, sorted in reverse priority order (highest priority first) - if (_decodersProvider) { - _decoders = _decodersProvider(self.moduleRegistry); - } else { - RCTAssert(_bridge, @"Trying to find RCTImageDataDecoders and bridge not set."); - _decoders = [_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)]; - } + if (_decodersProvider) { + _decoders = _decodersProvider(self.moduleRegistry); + } else { + RCTAssert(_bridge, @"Trying to find RCTImageDataDecoders and bridge not set."); + _decoders = [_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)]; + } - _decoders = [_decoders - sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { - float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0; - float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0; - if (priorityA > priorityB) { - return NSOrderedAscending; - } else if (priorityA < priorityB) { - return NSOrderedDescending; - } else { - return NSOrderedSame; - } - }]; + _decoders = [_decoders + sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { + float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0; + float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0; + if (priorityA > priorityB) { + return NSOrderedAscending; + } else if (priorityA < priorityB) { + return NSOrderedDescending; + } else { + return NSOrderedSame; + } + }]; + _decodersReady.store(YES, std::memory_order_release); + } } + NSArray> *decoders = _decoders; if (RCT_DEBUG) { // Check for handler conflicts float previousPriority = 0; id previousDecoder = nil; - for (id decoder in _decoders) { + for (id decoder in decoders) { float priority = [decoder respondsToSelector:@selector(decoderPriority)] ? [decoder decoderPriority] : 0; if (previousDecoder && priority < previousPriority) { return previousDecoder; @@ -297,7 +306,7 @@ - (void)setImageCache:(id)cache } // Normal code path - for (id decoder in _decoders) { + for (id decoder in decoders) { if ([decoder canDecodeImageData:data]) { return decoder; } From 822621965eede8a6059f3e38c4a44b1d7a506d4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Mon, 22 Jun 2026 22:38:02 +0200 Subject: [PATCH 2/2] 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. --- .../Libraries/Image/RCTImageLoader.mm | 82 ++++++++++--------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/packages/react-native/Libraries/Image/RCTImageLoader.mm b/packages/react-native/Libraries/Image/RCTImageLoader.mm index d500d99635ac..dee2685a1211 100644 --- a/packages/react-native/Libraries/Image/RCTImageLoader.mm +++ b/packages/react-native/Libraries/Image/RCTImageLoader.mm @@ -163,23 +163,17 @@ - (void)setImageCache:(id)cache _imageCache = cache; } -- (id)imageURLLoaderForURL:(NSURL *)URL +- (NSArray> *)loaders { - if (!_isLoaderSetup) { - [self setUp]; - } - if (!_loadersReady.load(std::memory_order_acquire)) { std::lock_guard guard(_loadersMutex); if (!_loadersReady.load(std::memory_order_relaxed)) { - // Get loaders, sorted in reverse priority order (highest priority first) if (_loadersProvider) { _loaders = _loadersProvider(self.moduleRegistry); } else { RCTAssert(_bridge, @"Trying to find RCTImageURLLoaders and bridge not set."); _loaders = [_bridge modulesConformingToProtocol:@protocol(RCTImageURLLoader)]; } - _loaders = [_loaders sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { float priorityA = [a respondsToSelector:@selector(loaderPriority)] ? [a loaderPriority] : 0; @@ -195,7 +189,45 @@ - (void)setImageCache:(id)cache _loadersReady.store(YES, std::memory_order_release); } } - NSArray> *loaders = _loaders; + return _loaders; +} + +- (NSArray> *)decoders +{ + if (!_decodersReady.load(std::memory_order_acquire)) { + std::lock_guard guard(_loadersMutex); + if (!_decodersReady.load(std::memory_order_relaxed)) { + if (_decodersProvider) { + _decoders = _decodersProvider(self.moduleRegistry); + } else { + RCTAssert(_bridge, @"Trying to find RCTImageDataDecoders and bridge not set."); + _decoders = [_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)]; + } + _decoders = [_decoders + sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { + float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0; + float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0; + if (priorityA > priorityB) { + return NSOrderedAscending; + } else if (priorityA < priorityB) { + return NSOrderedDescending; + } else { + return NSOrderedSame; + } + }]; + _decodersReady.store(YES, std::memory_order_release); + } + } + return _decoders; +} + +- (id)imageURLLoaderForURL:(NSURL *)URL +{ + if (!_isLoaderSetup) { + [self setUp]; + } + + NSArray> *loaders = [self loaders]; if (RCT_DEBUG) { // Check for handler conflicts @@ -244,34 +276,7 @@ - (void)setImageCache:(id)cache [self setUp]; } - if (!_decodersReady.load(std::memory_order_acquire)) { - std::lock_guard guard(_loadersMutex); - if (!_decodersReady.load(std::memory_order_relaxed)) { - // Get decoders, sorted in reverse priority order (highest priority first) - - if (_decodersProvider) { - _decoders = _decodersProvider(self.moduleRegistry); - } else { - RCTAssert(_bridge, @"Trying to find RCTImageDataDecoders and bridge not set."); - _decoders = [_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)]; - } - - _decoders = [_decoders - sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { - float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0; - float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0; - if (priorityA > priorityB) { - return NSOrderedAscending; - } else if (priorityA < priorityB) { - return NSOrderedDescending; - } else { - return NSOrderedSame; - } - }]; - _decodersReady.store(YES, std::memory_order_release); - } - } - NSArray> *decoders = _decoders; + NSArray> *decoders = [self decoders]; if (RCT_DEBUG) { // Check for handler conflicts @@ -1182,7 +1187,10 @@ - (BOOL)canHandleRequest:(NSURLRequest *)request return NO; } - for (id loader in _loaders) { + if (!_isLoaderSetup) { + [self setUp]; + } + for (id loader in [self loaders]) { // Don't use RCTImageURLLoader protocol for modules that already conform to // RCTURLRequestHandler as it's inefficient to decode an image and then // convert it back into data