Run RCTAccessibilityManager methods on the main queue - #58476
Closed
christophpurrer wants to merge 1 commit into
Closed
Run RCTAccessibilityManager methods on the main queue#58476christophpurrer wants to merge 1 commit into
christophpurrer wants to merge 1 commit into
Conversation
Summary: RCTAccessibilityManager mirrors UIKit accessibility state into ivars from nine NSNotificationCenter handlers, which are delivered on the main thread. The module declared no methodQueue, so its exported methods ran on the JS thread instead, and none of its properties are atomic. That leaves the content size multipliers open to a data race. The `multipliers` getter lazily assigns `_multipliers` on read and is reached from the main thread via the content size category notification, while `setAccessibilityContentSizeMultipliers:` reaches `setMultipliers:` from the JS thread and releases the previous dictionary. Concurrent access can over-release that dictionary and leave a dangling pointer behind, which then faults on the next message to it. Declare methodQueue as the main queue so the exported methods and the notification handlers serialize on one thread. The module already requires main queue setup and every method reads UIKit-derived state, so this matches how it is used. Changelog: [iOS][Fixed] - Fix data race on accessibility content size multipliers in `RCTAccessibilityManager` Differential Revision: D119565016
meta-cla
Bot
added
the
CLA Signed
|
@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D119565016. |
|
This pull request has been merged in ea2ca1b. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
RCTAccessibilityManager mirrors UIKit accessibility state into ivars from nine
NSNotificationCenter handlers, which are delivered on the main thread. The module
declared no methodQueue, so its exported methods ran on the JS thread instead, and
none of its properties are atomic.
That leaves the content size multipliers open to a data race. The
multipliersgetter lazily assigns
_multiplierson read and is reached from the main thread viathe content size category notification, while
setAccessibilityContentSizeMultipliers:reaches
setMultipliers:from the JS thread and releases the previous dictionary.Concurrent access can over-release that dictionary and leave a dangling pointer
behind, which then faults on the next message to it.
Declare methodQueue as the main queue so the exported methods and the notification
handlers serialize on one thread. The module already requires main queue setup and
every method reads UIKit-derived state, so this matches how it is used.
Changelog:
[iOS][Fixed] - Fix data race on accessibility content size multipliers in
RCTAccessibilityManagerDifferential Revision: D119565016