Skip to content

[iOS] Reland: avoid race condition crash in [RCTDataRequestHandler invalidate#50342

Closed
zhongwuzw wants to merge 1 commit intofacebook:mainfrom
zhongwuzw:features/fix_handler_crash
Closed

[iOS] Reland: avoid race condition crash in [RCTDataRequestHandler invalidate#50342
zhongwuzw wants to merge 1 commit intofacebook:mainfrom
zhongwuzw:features/fix_handler_crash

Conversation

@zhongwuzw
Copy link
Copy Markdown
Contributor

Summary:

Reland 6bc5dde, the reason the previous commit has an issue is weakOp would always nil when captured in block :

__weak NSBlockOperation *weakOp;
NSBlockOperation *op = [NSBlockOperation blockOperationWithBlock:^{
NSBlockOperation *strongOp = weakOp; // Strong reference to avoid deallocation during execution

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

Test Plan:

N/A

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Mar 28, 2025
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 29, 2025
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@javache merged this pull request in 44810f7.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @zhongwuzw in 44810f7

When will my fix make it into a release? | How to file a pick request?

Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Mar 31, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Mar 31, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Mar 31, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Mar 31, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Mar 31, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Mar 31, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Saadnajmi added a commit to microsoft/react-native-macos that referenced this pull request Apr 8, 2025
…aRequestHandler invalidate]" (#2450)

Cherry pick facebook#50342 , which
fixes a bug with our fix from #2383

I chose to omit the macOS tags as this is a cherry-pick.

Co-authored-by: zhongwuzw <zhongwuzw@qq.com>
Saadnajmi pushed a commit to microsoft/react-native-macos that referenced this pull request Apr 8, 2025
…te (facebook#50342)

Summary:
Reland facebook@6bc5dde, 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.

[IOS] [FIXED] - Reland: avoid race condition crash in [RCTDataRequestHandler invalidate

Pull Request resolved: facebook#50342

Test Plan: N/A

Reviewed By: cipolleschi

Differential Revision: D72047232

Pulled By: javache

fbshipit-source-id: c3a5a9fca909f7eac16d78b650c9ea9f5b8e64e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants