From 44810f749841b8340cc06fecbf861c6390751c29 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Fri, 28 Mar 2025 09:05:01 -0700 Subject: [PATCH] Reland: avoid race condition crash in [RCTDataRequestHandler invalidate (#50342) Summary: Reland https://github.com/facebook/react-native/commit/6bc5ddea3ea3ca20060ea0181630539931948085, the reason the previous commit has an issue is `weakOp` would always `nil` when captured in block :https://github.com/facebook/react-native/blob/6bc5ddea3ea3ca20060ea0181630539931948085/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm#L52-L54 Now, we can create an `NSBlockOperation` and use `addExecutionBlock` instead. `weakOp` can be captured correctly. ## Changelog: [IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate Pull Request resolved: https://github.com/facebook/react-native/pull/50342 Test Plan: N/A Reviewed By: cipolleschi Differential Revision: D72047232 Pulled By: javache fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4 --- .../Libraries/Network/RCTDataRequestHandler.mm | 16 ++++++++++------ .../Libraries/Network/RCTFileRequestHandler.mm | 18 +++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm b/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm index 2aff977cf65..c91f89e22bd 100644 --- a/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm +++ b/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm @@ -49,8 +49,13 @@ RCT_EXPORT_MODULE() _queue.maxConcurrentOperationCount = 2; } - __weak __block NSBlockOperation *weakOp; - __block NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{ + NSBlockOperation *op = [NSBlockOperation new]; + __weak NSBlockOperation *weakOp = op; + [op addExecutionBlock:^{ + NSBlockOperation *strongOp = weakOp; // Strong reference to avoid deallocation during execution + if (strongOp == nil || [strongOp isCancelled]) { + return; + } // Get mime type NSRange firstSemicolon = [request.URL.resourceSpecifier rangeOfString:@";"]; NSString *mimeType = @@ -62,18 +67,17 @@ RCT_EXPORT_MODULE() expectedContentLength:-1 textEncodingName:nil]; - [delegate URLRequest:weakOp didReceiveResponse:response]; + [delegate URLRequest:strongOp didReceiveResponse:response]; // Load data NSError *error; NSData *data = [NSData dataWithContentsOfURL:request.URL options:NSDataReadingMappedIfSafe error:&error]; if (data) { - [delegate URLRequest:weakOp didReceiveData:data]; + [delegate URLRequest:strongOp didReceiveData:data]; } - [delegate URLRequest:weakOp didCompleteWithError:error]; + [delegate URLRequest:strongOp didCompleteWithError:error]; }]; - weakOp = op; [_queue addOperation:op]; return op; } diff --git a/packages/react-native/Libraries/Network/RCTFileRequestHandler.mm b/packages/react-native/Libraries/Network/RCTFileRequestHandler.mm index 4ca36256c68..a1de92f33fe 100644 --- a/packages/react-native/Libraries/Network/RCTFileRequestHandler.mm +++ b/packages/react-native/Libraries/Network/RCTFileRequestHandler.mm @@ -53,14 +53,19 @@ RCT_EXPORT_MODULE() _fileQueue.maxConcurrentOperationCount = 4; } - __weak __block NSBlockOperation *weakOp; - __block NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{ + NSBlockOperation *op = [NSBlockOperation new]; + __weak NSBlockOperation *weakOp = op; + [op addExecutionBlock:^{ + NSBlockOperation *strongOp = weakOp; // Strong reference to avoid deallocation during execution + if (strongOp == nil || [strongOp isCancelled]) { + return; + } // Get content length NSError *error = nil; NSFileManager *fileManager = [NSFileManager new]; NSDictionary *fileAttributes = [fileManager attributesOfItemAtPath:request.URL.path error:&error]; if (!fileAttributes) { - [delegate URLRequest:weakOp didCompleteWithError:error]; + [delegate URLRequest:strongOp didCompleteWithError:error]; return; } @@ -77,17 +82,16 @@ RCT_EXPORT_MODULE() expectedContentLength:[fileAttributes[NSFileSize] ?: @-1 integerValue] textEncodingName:nil]; - [delegate URLRequest:weakOp didReceiveResponse:response]; + [delegate URLRequest:strongOp didReceiveResponse:response]; // Load data NSData *data = [NSData dataWithContentsOfURL:request.URL options:NSDataReadingMappedIfSafe error:&error]; if (data) { - [delegate URLRequest:weakOp didReceiveData:data]; + [delegate URLRequest:strongOp didReceiveData:data]; } - [delegate URLRequest:weakOp didCompleteWithError:error]; + [delegate URLRequest:strongOp didCompleteWithError:error]; }]; - weakOp = op; [_fileQueue addOperation:op]; return op; }