Skip to content

Commit

Permalink
Avoid sharing mutable attributed string across threads in RCTBaseText…
Browse files Browse the repository at this point in the history
…InputShadowView

Summary:
The `NSMutableAttributedString` was initialized with the same backing store as the cached attributed string on the shadow queue (background thread), but then passed to the main thread and ultimately the JS thread.

This explicitly copies the mutable attributed string into an immutable one on the shadow thread before passed off to other threads, which hopefully will address the `convertIdToFollyDynamic` crash that always includes `RCTBaseTextInputShadowView` touching an attributed string on the shadow queue in the trace.

Changelog:
[iOS][Fixed] - Possible fix for convertIdToFollyDynamic crash in RCTBaseTextInputView and RCTEventDispatcher

Reviewed By: sammy-SC

Differential Revision: D38133150

fbshipit-source-id: f371da5d17a32c3341287cd3e9730b31a98495f9
  • Loading branch information
christophpurrer authored and facebook-github-bot committed Jul 26, 2022
1 parent 546c4b4 commit 547220f
Showing 1 changed file with 7 additions and 8 deletions.
15 changes: 7 additions & 8 deletions Libraries/Text/TextInput/RCTBaseTextInputShadowView.m
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ - (void)uiManagerWillPerformMounting

RCTTextAttributes *textAttributes = [self.textAttributes copy];

NSMutableAttributedString *attributedText =
[[NSMutableAttributedString alloc] initWithAttributedString:[self attributedTextWithBaseTextAttributes:nil]];
NSMutableAttributedString *attributedText = [[self attributedTextWithBaseTextAttributes:nil] mutableCopy];

// Removing all references to Shadow Views and tags to avoid unnecessary retaining
// and problems with comparing the strings.
Expand All @@ -162,13 +161,13 @@ - (void)uiManagerWillPerformMounting
[attributedText insertAttributedString:propertyAttributedText atIndex:0];
}

BOOL isAttributedTextChanged = NO;
NSAttributedString *newAttributedText;
if (![_previousAttributedText isEqualToAttributedString:attributedText]) {
// We have to follow `set prop` pattern:
// If the value has not changed, we must not notify the view about the change,
// otherwise we may break local (temporary) state of the text input.
isAttributedTextChanged = YES;
_previousAttributedText = [attributedText copy];
newAttributedText = [attributedText copy];
_previousAttributedText = newAttributedText;
}

NSNumber *tag = self.reactTag;
Expand All @@ -183,10 +182,10 @@ - (void)uiManagerWillPerformMounting
baseTextInputView.reactBorderInsets = borderInsets;
baseTextInputView.reactPaddingInsets = paddingInsets;

if (isAttributedTextChanged) {
if (newAttributedText) {
// Don't set `attributedText` if length equal to zero, otherwise it would shrink when attributes contain like `lineHeight`.
if (attributedText.length != 0) {
baseTextInputView.attributedText = attributedText;
if (newAttributedText.length != 0) {
baseTextInputView.attributedText = newAttributedText;
} else {
baseTextInputView.attributedText = nil;
}
Expand Down

0 comments on commit 547220f

Please sign in to comment.