Skip to content

Fix potential race conditions when executing commitHooks - #53862

Closed
christophpurrer wants to merge 1 commit into
react:mainfrom
christophpurrer:export-D82846245
Closed

Fix potential race conditions when executing commitHooks#53862
christophpurrer wants to merge 1 commit into
react:mainfrom
christophpurrer:export-D82846245

Conversation

@christophpurrer

Copy link
Copy Markdown
Contributor

Summary:
Recently we saw use-after-free race condition where the ImageFetcher object was being destroyed while still registered as a UIManagerCommitHook.

The crash occurred in std::vector::size() at line 635 when accessing corrupted memory.
The root cause could be improper lifecycle management between ImageFetcher destruction and commit hook execution.

The fix here modifies UIManager::shadowTreeWillCommit() to create a stable snapshot by copying the commitHooks_ vector while holding the lock.

Changelog: [Internal]

Differential Revision: D82846245

@meta-clameta-claBot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 19, 2025
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Sep 20, 2025
Summary:
Recently we saw `use-after-free race condition` where the ImageFetcher object was being destroyed while still registered as a UIManagerCommitHook.
The crash occurred in `std::vector::size()` at line 635 when accessing corrupted memory.
The root cause could be improper lifecycle management between ImageFetcher destruction and commit hook execution.
The fix here modifies `UIManager::shadowTreeWillCommit()` to create a stable snapshot by copying the commitHooks_ vector while holding the lock.
## Reason
- If a thread is iterating over `commitHooks_` with a `shared_lock`, and another thread acquires a `unique_lock` to modify the vector (add/remove), the iterator in the first thread can become invalid, leading to a crash (use-after-free or out-of-bounds).
- This can happen if the lock is not held for the entire duration of the read or write, or if the lock is not correctly used everywhere commitHooks_ is accessed.
Changelog: [Internal]
Differential Revision: D82846245
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating diff in D82846245.

@meta-codesync

Copy link
Copy Markdown

@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D82846245.

…mitHooks (react#53862)
Summary:
Recently, we observed a `use-after-free race condition` where the ImageFetcher object was destroyed while it was still registered as a UIManagerCommitHook.
The crash occurred in `std::vector::size()` at line 635 when accessing corrupted memory.
Changelog: [Internal]
Differential Revision: D82846245
@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label Oct 7, 2025
@meta-codesync

Copy link
Copy Markdown

This pull request has been merged in 44725b8.

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @christophpurrer in 44725b8

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.fb-exportedMergedThis PR has been merged.meta-exportedp: FacebookPartner: FacebookPartner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@christophpurrer@facebook-github-bot@react-native-bot