You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add missing lock around assignment of cancellation handler in RCTImageLoader (#45454)
Summary:
When running the entire unit test suite of `RNTesterPods` with `TSan`, I saw that occasionally a data race was detected on line 843 of `RCTImageLoader`. It seems the completion handler that does contain the lock around `cancelLoad` is called concurrently with the value being assigned on line 843. Here there is no lock in place.
Here is the output of `TSan` when I comment out my fix:
```
WARNING: ThreadSanitizer: data race (pid=72490)
Write of size 8 at 0x000144151ce8 by main thread:
#0 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:priority:attribution:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16e8030)
#1 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:priority:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16df8dc)
#2 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16df534)
#3 -[RCTImageLoaderTests testImageLoaderUsesImageURLLoaderWithHighestPriority] <null> (RNTesterUnitTests:arm64+0x7cb8)
#4 __invoking___ <null> (CoreFoundation:arm64+0x13371c)
Previous write of size 8 at 0x000144151ce8 by thread T4 (mutexes: write M0):
#0 __140-[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:priority:attribution:progressBlock:partialLoadBlock:completionBlock:]_block_invoke_2 <null> (RNTesterUnitTests:arm64+0x16e8894)
#1 __139-[RCTImageLoader _loadImageOrDataWithURLRequest:size:scale:resizeMode:priority:attribution:progressBlock:partialLoadBlock:completionBlock:]_block_invoke <null> (RNTesterUnitTests:arm64+0x16e3430)
#2 __139-[RCTImageLoader _loadImageOrDataWithURLRequest:size:scale:resizeMode:priority:attribution:progressBlock:partialLoadBlock:completionBlock:]_block_invoke_3 <null> (RNTesterUnitTests:arm64+0x16e52a8)
#3 __75-[RCTImageLoaderTests testImageLoaderUsesImageURLLoaderWithHighestPriority]_block_invoke_2 <null> (RNTesterUnitTests:arm64+0x7f24)
#4 -[RCTConcreteImageURLLoader loadImageForURL:size:scale:resizeMode:progressHandler:partialLoadHandler:completionHandler:] <null> (RNTesterUnitTests:arm64+0x6c470)
#5 __139-[RCTImageLoader _loadImageOrDataWithURLRequest:size:scale:resizeMode:priority:attribution:progressBlock:partialLoadBlock:completionBlock:]_block_invoke.172 <null> (RNTesterUnitTests:arm64+0x16e4964)
#6 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x77ee0)
#7 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x3974)
Location is heap block of size 48 at 0x000144151cc0 allocated by main thread:
#0 malloc <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x4fa48)
#1 _malloc_type_malloc_outlined <null> (libsystem_malloc.dylib:arm64+0xf3ec)
#2 _call_copy_helpers_excp <null> (libsystem_blocks.dylib:arm64+0x10b4)
#3 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:priority:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16df8dc)
#4 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16df534)
#5 -[RCTImageLoaderTests testImageLoaderUsesImageURLLoaderWithHighestPriority] <null> (RNTesterUnitTests:arm64+0x7cb8)
#6 __invoking___ <null> (CoreFoundation:arm64+0x13371c)
Mutex M0 (0x000108f316e8) created at:
#0 pthread_mutex_init <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x2cc98)
#1 -[NSLock init] <null> (Foundation:arm64+0x5ca14c)
#2 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:priority:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16df8dc)
#3 -[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:progressBlock:partialLoadBlock:completionBlock:] <null> (RNTesterUnitTests:arm64+0x16df534)
#4 -[RCTImageLoaderTests testImageLoaderUsesImageURLLoaderWithHighestPriority] <null> (RNTesterUnitTests:arm64+0x7cb8)
#5 __invoking___ <null> (CoreFoundation:arm64+0x13371c)
Thread T4 (tid=10935088, running) is a GCD worker thread
```
## Changelog:
[iOS][Fixed] Data race in `RCTImageLoader` related to assignment of cancellation block.
Pull Request resolved: #45454
Test Plan: There are already tests in place for `RCTImageLoader`. I hope these will cover the fix.
Reviewed By: realsoelynn
Differential Revision: D59816000
Pulled By: zeyap
fbshipit-source-id: f959d472eb60f83f39ced6711ee395949ab37e7c
0 commit comments