Skip to content

Commit 6bc5dde

Browse files
Saadnajmifacebook-github-bot
authored andcommitted
fix: avoid race condition crash in [RCTDataRequestHandler invalidate] (#49705)
Summary: Upstreaming a fix by ntre that fixes a crash we saw internally related to `[_queue cancelAllOperations]`. >Calling [_queue cancelAllOperations] will release all references to any active operations. >If the blocks of those operations have a reference to itself, it will result in dangling pointers, which could conceptually trigger a later crash if there's a race between the operation completing and it being pulled out of the queue. > >Add explicit strong reference while block is running. >For good measure, fix same pattern also in RCTFileRequestHandler. > >Note: separately, that this code is passing the op itself as a requestToken to [delegate URLRequest:] methods is suspect. That delegate can retain said token. ## Changelog: [IOS] [FIXED] - avoid race condition crash in [RCTDataRequestHandler invalidate] Pull Request resolved: #49705 Test Plan: Tested internally, we no longer saw the crash after this fix. Reviewed By: javache Differential Revision: D70314889 Pulled By: cipolleschi fbshipit-source-id: ebcecb4675bd1dda3d9ee60d69967feb4e05e11b
1 parent 13ac1a9 commit 6bc5dde

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

packages/react-native/Libraries/Network/RCTDataRequestHandler.mm

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,13 @@ - (NSOperation *)sendRequest:(NSURLRequest *)request withDelegate:(id<RCTURLRequ
4949
_queue.maxConcurrentOperationCount = 2;
5050
}
5151

52-
__weak __block NSBlockOperation *weakOp;
53-
__block NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{
52+
__weak NSBlockOperation *weakOp;
53+
NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{
54+
NSBlockOperation *strongOp = weakOp; // Strong reference to avoid deallocation during execution
55+
if (strongOp == nil || [strongOp isCancelled]) {
56+
return;
57+
}
58+
5459
// Get mime type
5560
NSRange firstSemicolon = [request.URL.resourceSpecifier rangeOfString:@";"];
5661
NSString *mimeType =
@@ -62,15 +67,15 @@ - (NSOperation *)sendRequest:(NSURLRequest *)request withDelegate:(id<RCTURLRequ
6267
expectedContentLength:-1
6368
textEncodingName:nil];
6469

65-
[delegate URLRequest:weakOp didReceiveResponse:response];
70+
[delegate URLRequest:strongOp didReceiveResponse:response];
6671

6772
// Load data
6873
NSError *error;
6974
NSData *data = [NSData dataWithContentsOfURL:request.URL options:NSDataReadingMappedIfSafe error:&error];
7075
if (data) {
71-
[delegate URLRequest:weakOp didReceiveData:data];
76+
[delegate URLRequest:strongOp didReceiveData:data];
7277
}
73-
[delegate URLRequest:weakOp didCompleteWithError:error];
78+
[delegate URLRequest:strongOp didCompleteWithError:error];
7479
}];
7580

7681
weakOp = op;

packages/react-native/Libraries/Network/RCTFileRequestHandler.mm

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,19 @@ - (NSOperation *)sendRequest:(NSURLRequest *)request withDelegate:(id<RCTURLRequ
5353
_fileQueue.maxConcurrentOperationCount = 4;
5454
}
5555

56-
__weak __block NSBlockOperation *weakOp;
57-
__block NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{
56+
__weak NSBlockOperation *weakOp;
57+
NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{
58+
NSBlockOperation *strongOp = weakOp; // Strong reference to avoid deallocation during execution
59+
if (strongOp == nil || [strongOp isCancelled]) {
60+
return;
61+
}
62+
5863
// Get content length
5964
NSError *error = nil;
6065
NSFileManager *fileManager = [NSFileManager new];
6166
NSDictionary<NSString *, id> *fileAttributes = [fileManager attributesOfItemAtPath:request.URL.path error:&error];
6267
if (!fileAttributes) {
63-
[delegate URLRequest:weakOp didCompleteWithError:error];
68+
[delegate URLRequest:strongOp didCompleteWithError:error];
6469
return;
6570
}
6671

@@ -77,14 +82,14 @@ - (NSOperation *)sendRequest:(NSURLRequest *)request withDelegate:(id<RCTURLRequ
7782
expectedContentLength:[fileAttributes[NSFileSize] ?: @-1 integerValue]
7883
textEncodingName:nil];
7984

80-
[delegate URLRequest:weakOp didReceiveResponse:response];
85+
[delegate URLRequest:strongOp didReceiveResponse:response];
8186

8287
// Load data
8388
NSData *data = [NSData dataWithContentsOfURL:request.URL options:NSDataReadingMappedIfSafe error:&error];
8489
if (data) {
85-
[delegate URLRequest:weakOp didReceiveData:data];
90+
[delegate URLRequest:strongOp didReceiveData:data];
8691
}
87-
[delegate URLRequest:weakOp didCompleteWithError:error];
92+
[delegate URLRequest:strongOp didCompleteWithError:error];
8893
}];
8994

9095
weakOp = op;

0 commit comments

Comments
 (0)