Fix data race related to RCTNetworkTask.status (#44694)

Summary:
Fix entails using non-synthesized getter, such that underlying backing is an std::atomic<RCTNetworkTaskStatus>.

In the greater scheme of things, I believe `RCTNetworkTask` should be improved as it has several `nonatomic` properties that are read and written to on different threads. Thread safety of this class seems to have been addressed on a per property basis, judging from the employment of `std::mutex` elsewhere in the implementation.

This is an attempt at fixing https://github.com/facebook/react-native/issues/44687.

## Changelog:

[iOS][FIXED] - Fix data race related to access on `RCTNetworkTask.status`.

Pull Request resolved: https://github.com/facebook/react-native/pull/44694

Test Plan: Added unit test in class `RCTNetworkTaskTests`.

Reviewed By: cortinico

Differential Revision: D59217353

Pulled By: javache

fbshipit-source-id: 1af77238ddd99db21e2e53f174a81e207d5832b2
This commit is contained in:
Håkon Knutzen
2024-07-03 08:22:26 -07:00
committed by Facebook GitHub Bot
parent c0977c39b9
commit b1ec698dc4
4 changed files with 67 additions and 10 deletions
@@ -35,7 +35,7 @@ typedef NS_ENUM(NSInteger, RCTNetworkTaskStatus) {
@property (nonatomic, copy) RCTURLRequestResponseBlock responseBlock;
@property (nonatomic, copy) RCTURLRequestProgressBlock uploadProgressBlock;
@property (nonatomic, readonly) RCTNetworkTaskStatus status;
@property (atomic, readonly) RCTNetworkTaskStatus status;
- (instancetype)initWithRequest:(NSURLRequest *)request
handler:(id<RCTURLRequestHandler>)handler
@@ -17,7 +17,7 @@
id<RCTURLRequestHandler> _handler;
dispatch_queue_t _callbackQueue;
std::mutex _mutex;
std::atomic<RCTNetworkTaskStatus> _atomicStatus;
RCTNetworkTask *_selfReference;
}
@@ -36,7 +36,7 @@ static auto currentRequestId = std::atomic<NSUInteger>(0);
_request = request;
_handler = handler;
_callbackQueue = callbackQueue;
_status = RCTNetworkTaskPending;
_atomicStatus = RCTNetworkTaskPending;
dispatch_queue_set_specific(callbackQueue, (__bridge void *)self, (__bridge void *)self, NULL);
}
@@ -45,6 +45,11 @@ static auto currentRequestId = std::atomic<NSUInteger>(0);
RCT_NOT_IMPLEMENTED(-(instancetype)init)
- (RCTNetworkTaskStatus)status
{
return _atomicStatus;
}
- (void)invalidate
{
_selfReference = nil;
@@ -67,7 +72,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
- (void)start
{
if (_status != RCTNetworkTaskPending) {
if (_atomicStatus != RCTNetworkTaskPending) {
RCTLogError(@"RCTNetworkTask was already started or completed");
return;
}
@@ -76,18 +81,17 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
id token = [_handler sendRequest:_request withDelegate:self];
if ([self validateRequestToken:token]) {
_selfReference = self;
_status = RCTNetworkTaskInProgress;
_atomicStatus = RCTNetworkTaskInProgress;
}
}
}
- (void)cancel
{
if (_status == RCTNetworkTaskFinished) {
if (_atomicStatus.exchange(RCTNetworkTaskFinished) == RCTNetworkTaskFinished) {
return;
}
_status = RCTNetworkTaskFinished;
id token = _requestToken;
if (token && [_handler respondsToSelector:@selector(cancelRequest:)]) {
[_handler cancelRequest:token];
@@ -108,7 +112,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
}
if (!valid) {
_status = RCTNetworkTaskFinished;
_atomicStatus = RCTNetworkTaskFinished;
if (_completionBlock) {
RCTURLRequestCompletionBlock completionBlock = _completionBlock;
[self dispatchCallback:^{
@@ -168,7 +172,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
@try {
[_data appendData:data];
} @catch (NSException *exception) {
_status = RCTNetworkTaskFinished;
_atomicStatus = RCTNetworkTaskFinished;
if (_completionBlock) {
RCTURLRequestCompletionBlock completionBlock = _completionBlock;
[self dispatchCallback:^{
@@ -205,7 +209,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
return;
}
_status = RCTNetworkTaskFinished;
_atomicStatus = RCTNetworkTaskFinished;
if (_completionBlock) {
RCTURLRequestCompletionBlock completionBlock = _completionBlock;
NSData *dataCopy = nil;
@@ -15,6 +15,7 @@
5C60EB1C226440DB0018C04F /* AppDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5C60EB1B226440DB0018C04F /* AppDelegate.mm */; };
8145AE06241172D900A3F8DA /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 8145AE05241172D900A3F8DA /* LaunchScreen.storyboard */; };
832F45BB2A8A6E1F0097B4E6 /* SwiftTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 832F45BA2A8A6E1F0097B4E6 /* SwiftTest.swift */; };
A975CA6C2C05EADF0043F72A /* RCTNetworkTaskTests.m in Sources */ = {isa = PBXBuildFile; fileRef = A975CA6B2C05EADE0043F72A /* RCTNetworkTaskTests.m */; };
BEB82277FE76227A15DED9EF /* libPods-RNTesterIntegrationTests.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 77E101C7D8E22A8E70EE76DF /* libPods-RNTesterIntegrationTests.a */; };
CD10C7A5290BD4EB0033E1ED /* RCTEventEmitterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CD10C7A4290BD4EB0033E1ED /* RCTEventEmitterTests.m */; };
D626973C1F0D4ABDE2F9028A /* libPods-RNTesterUnitTests.a in Frameworks */ = {isa = PBXBuildFile; fileRef = D8DE57E3292D41E28251988A /* libPods-RNTesterUnitTests.a */; };
@@ -98,6 +99,7 @@
832F45BA2A8A6E1F0097B4E6 /* SwiftTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = SwiftTest.swift; path = RNTester/SwiftTest.swift; sourceTree = "<group>"; };
8BFB9C61D7BDE894E24BF24F /* Pods-RNTesterUnitTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RNTesterUnitTests.release.xcconfig"; path = "Target Support Files/Pods-RNTesterUnitTests/Pods-RNTesterUnitTests.release.xcconfig"; sourceTree = "<group>"; };
9B8542B8C590B51BD0588751 /* Pods-RNTester.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RNTester.release.xcconfig"; path = "Target Support Files/Pods-RNTester/Pods-RNTester.release.xcconfig"; sourceTree = "<group>"; };
A975CA6B2C05EADE0043F72A /* RCTNetworkTaskTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RCTNetworkTaskTests.m; sourceTree = "<group>"; };
AC474BFB29BBD4A1002BDAED /* RNTester.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; name = RNTester.xctestplan; path = RNTester/RNTester.xctestplan; sourceTree = "<group>"; };
CD10C7A4290BD4EB0033E1ED /* RCTEventEmitterTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTEventEmitterTests.m; sourceTree = "<group>"; };
D74056A5352F0925816E50E0 /* libPods-RNTester.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-RNTester.a"; sourceTree = BUILT_PRODUCTS_DIR; };
@@ -341,6 +343,7 @@
E7DB20B022B2BAA4005AC45F /* RCTModuleInitTests.m */,
E7DB20CB22B2BAA5005AC45F /* RCTModuleMethodTests.mm */,
E7DB20CF22B2BAA5005AC45F /* RCTMultipartStreamReaderTests.m */,
A975CA6B2C05EADE0043F72A /* RCTNetworkTaskTests.m */,
E7DB20BE22B2BAA4005AC45F /* RCTNativeAnimatedNodesManagerTests.m */,
E7DB20AD22B2BAA3005AC45F /* RCTPerformanceLoggerTests.m */,
E7DB20C422B2BAA4005AC45F /* RCTShadowViewTests.m */,
@@ -724,6 +727,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
A975CA6C2C05EADF0043F72A /* RCTNetworkTaskTests.m in Sources */,
E7DB20DF22B2BAA6005AC45F /* RCTImageLoaderTests.m in Sources */,
E7DB20D222B2BAA6005AC45F /* RCTModuleInitNotificationRaceTests.m in Sources */,
E7DB20D522B2BAA6005AC45F /* RCTPerformanceLoggerTests.m in Sources */,
@@ -0,0 +1,49 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#import <React/RCTNetworkTask.h>
#import <React/RCTURLRequestHandler.h>
#import <XCTest/XCTest.h>
@interface TestHandler<RCTURLRequestHandler> : NSObject
@end
@implementation TestHandler
- (BOOL)canHandleRequest:(NSURLRequest *)request
{
return YES;
}
- (id)sendRequest:(NSURLRequest *)request withDelegate:(id<RCTURLRequestDelegate>)delegate
{
return [[NSUUID UUID] UUIDString];
}
@end
@interface RCTNetworkTaskTests : XCTestCase
@end
@implementation RCTNetworkTaskTests
- (void)testCanReadTaskStatus
{
NSURL *url = [[NSURL alloc] initWithString:@"https://developers.facebook.com"];
NSURLRequest *request = [[NSURLRequest alloc] initWithURL:url];
dispatch_queue_t callbackQueue =
dispatch_queue_create("RCTNetworkTaskTests-testCanReadTaskStatus", DISPATCH_QUEUE_SERIAL);
id<RCTURLRequestHandler> testHandler = (id<RCTURLRequestHandler>)[[TestHandler alloc] init];
RCTNetworkTask *task = [[RCTNetworkTask alloc] initWithRequest:request
handler:testHandler
callbackQueue:callbackQueue];
XCTAssertEqual(task.status, RCTNetworkTaskPending);
[task start];
XCTAssertEqual(task.status, RCTNetworkTaskInProgress);
[task cancel];
XCTAssertEqual(task.status, RCTNetworkTaskFinished);
}
@end