Skip to content

Commit

Permalink
Fix data races in RCTImageLoader and RCTNetworkTask with shared a…
Browse files Browse the repository at this point in the history
…tomic counters (#45114)

Summary:
In order to fix the data races described in #44715, I propose a simple solution by leveraging shared counter functions wherein `std::atomic` is the backing for the integer values.

## Changelog:

[iOS] [Fixed] - Implement shared atomic counters and replace static integers in `RCTImageLoader` and `RCTNetworkTask` that were accessed concurrently, which in some cases lead to data races.

Pull Request resolved: #45114

Test Plan: Added unit tests for the counters in `RCTSharedCounterTests`.

Reviewed By: cipolleschi

Differential Revision: D59155076

Pulled By: javache

fbshipit-source-id: f73afce6a816ad3226ed8c123cb2ccf4183549a0
  • Loading branch information
hakonk authored and facebook-github-bot committed Jun 28, 2024
1 parent 9833338 commit ffc16fc
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
8 changes: 2 additions & 6 deletions packages/react-native/Libraries/Image/RCTImageLoader.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,7 @@ static NSInteger RCTImageBytesForImage(UIImage *image)
return image.images ? image.images.count * singleImageBytes : singleImageBytes;
}

static uint64_t getNextImageRequestCount(void)
{
static uint64_t requestCounter = 0;
return requestCounter++;
}
static auto currentRequestCount = std::atomic<uint64_t>(0);

static NSError *addResponseHeadersToError(NSError *originalError, NSHTTPURLResponse *response)
{
Expand Down Expand Up @@ -510,7 +506,7 @@ - (RCTImageURLLoaderRequest *)_loadImageOrDataWithURLRequest:(NSURLRequest *)req
auto cancelled = std::make_shared<std::atomic<int>>(0);
__block dispatch_block_t cancelLoad = nil;
__block NSLock *cancelLoadLock = [NSLock new];
NSString *requestId = [NSString stringWithFormat:@"%@-%llu", [[NSUUID UUID] UUIDString], getNextImageRequestCount()];
NSString *requestId = [NSString stringWithFormat:@"%@-%llu", [[NSUUID UUID] UUIDString], currentRequestCount++];

void (^completionHandler)(NSError *, id, id, NSURLResponse *) =
^(NSError *error, id imageOrData, id imageMetadata, NSURLResponse *response) {
Expand Down
7 changes: 4 additions & 3 deletions packages/react-native/Libraries/Network/RCTNetworkTask.mm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

#import <atomic>
#import <mutex>

#import <React/RCTLog.h>
Expand All @@ -20,6 +21,8 @@ @implementation RCTNetworkTask {
RCTNetworkTask *_selfReference;
}

static auto currentRequestId = std::atomic<NSUInteger>(0);

- (instancetype)initWithRequest:(NSURLRequest *)request
handler:(id<RCTURLRequestHandler>)handler
callbackQueue:(dispatch_queue_t)callbackQueue
Expand All @@ -28,10 +31,8 @@ - (instancetype)initWithRequest:(NSURLRequest *)request
RCTAssertParam(handler);
RCTAssertParam(callbackQueue);

static NSUInteger requestID = 0;

if ((self = [super init])) {
_requestID = @(requestID++);
_requestID = @(currentRequestId++);
_request = request;
_handler = handler;
_callbackQueue = callbackQueue;
Expand Down

0 comments on commit ffc16fc

Please sign in to comment.