From ffc16fc18bcc44d1e98d66dbbe56fb726962fb72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=A5kon=20Knutzen?=
 <2263015+hakonk@users.noreply.github.com>
Date: Fri, 28 Jun 2024 07:37:45 -0700
Subject: [PATCH] Fix data races in `RCTImageLoader` and `RCTNetworkTask` with
 shared atomic counters (#45114)

Summary:
In order to fix the data races described in https://github.com/facebook/react-native/issues/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: https://github.com/facebook/react-native/pull/45114

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

Reviewed By: cipolleschi

Differential Revision: D59155076

Pulled By: javache

fbshipit-source-id: f73afce6a816ad3226ed8c123cb2ccf4183549a0
---
 packages/react-native/Libraries/Image/RCTImageLoader.mm   | 8 ++------
 packages/react-native/Libraries/Network/RCTNetworkTask.mm | 7 ++++---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/packages/react-native/Libraries/Image/RCTImageLoader.mm b/packages/react-native/Libraries/Image/RCTImageLoader.mm
index 46b267cccca..dac4ecc5fe4 100644
--- a/packages/react-native/Libraries/Image/RCTImageLoader.mm
+++ b/packages/react-native/Libraries/Image/RCTImageLoader.mm
@@ -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)
 {
@@ -510,7 +506,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, CGSize size, CGFloat scal
   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) {
diff --git a/packages/react-native/Libraries/Network/RCTNetworkTask.mm b/packages/react-native/Libraries/Network/RCTNetworkTask.mm
index 96a7ded6c9c..c89e2b7b350 100644
--- a/packages/react-native/Libraries/Network/RCTNetworkTask.mm
+++ b/packages/react-native/Libraries/Network/RCTNetworkTask.mm
@@ -5,6 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+#import <atomic>
 #import <mutex>
 
 #import <React/RCTLog.h>
@@ -20,6 +21,8 @@
   RCTNetworkTask *_selfReference;
 }
 
+static auto currentRequestId = std::atomic<NSUInteger>(0);
+
 - (instancetype)initWithRequest:(NSURLRequest *)request
                         handler:(id<RCTURLRequestHandler>)handler
                   callbackQueue:(dispatch_queue_t)callbackQueue
@@ -28,10 +31,8 @@
   RCTAssertParam(handler);
   RCTAssertParam(callbackQueue);
 
-  static NSUInteger requestID = 0;
-
   if ((self = [super init])) {
-    _requestID = @(requestID++);
+    _requestID = @(currentRequestId++);
     _request = request;
     _handler = handler;
     _callbackQueue = callbackQueue;