From 6bc5ddea3ea3ca20060ea0181630539931948085 Mon Sep 17 00:00:00 2001 From: Saad Najmi Date: Mon, 3 Mar 2025 05:58:07 -0800 Subject: [PATCH] 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: https://github.com/facebook/react-native/pull/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 --- .../Libraries/Network/RCTDataRequestHandler.mm | 15 ++++++++++----- .../Libraries/Network/RCTFileRequestHandler.mm | 17 +++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm b/packages/react-native/Libraries/Network/RCTDataRequestHandler.mm index 2aff977cf65..d5793870f55 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:^{ + __weak NSBlockOperation *weakOp; + NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{ + 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,15 +67,15 @@ 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; diff --git a/packages/react-native/Libraries/Network/RCTFileRequestHandler.mm b/packages/react-native/Libraries/Network/RCTFileRequestHandler.mm index 4ca36256c68..cfcd8243862 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:^{ + __weak NSBlockOperation *weakOp; + NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{ + 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,14 +82,14 @@ 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;