Skip to content

Commit

Permalink
Fix Deadlock in RCTi18nUtil (iOS) (#31032)
Browse files Browse the repository at this point in the history
Summary:
Note: PR to react-native-macos here microsoft#733

Internally in Microsoft code, we ran into a deadlock where the main queue and the UIManager queue were both trying to access `[RCTI18nUtil sharedInstance]`, and were blocked on each other. This is similar to an earlier issue with RCTScreenScale decsribed [here](#18096).

To summarize:
1- RCTShadowView (on the UIManager queue) and RCTView (on the main queue) both try to access `[RCTI18nUtil sharedInstance]`
2- The UIManager thread gets there first, and lazily initializes the sharedInstance. Meanwhile, the main thread is waiting on a lock possessed by the UIManager thread
3- As part of the initialization, we set an NSUserDefault, which seems to require the (blocked) main thread.
4- Deadlock.

For whatever reason, this only happens on debug. I did not figure out why, but I do know based on [this comment](#18096 (comment)), that the UIManagerQueue should never block the main queue.

The fix is to not use NSUserDefaults, and simpy use atomic properties instead. We get the thread safety for free, and it also simplifies the code somewhat without changing the public API. The downside is values aren't persisted anymore, but I do not think that was necessary / intended.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[iOS] [Fixed] - Fix deadlock on RCTi18nUtil

Pull Request resolved: #31032

Test Plan:
Ran the RTLExample in RNTester, and ensured switching to RTL still worked, and that setting forceRTL would still work after reloading the bundle.

https://user-images.githubusercontent.com/6722175/108775429-aefdae80-7526-11eb-9a89-3114f7ddc2af.mov

Reviewed By: javache

Differential Revision: D29522152

Pulled By: RSNara

fbshipit-source-id: 160840f63a7b1d6721b0fd8294fb11990a4509fa
  • Loading branch information
Saadnajmi authored and facebook-github-bot committed Aug 6, 2021
1 parent 91437d6 commit fcead14
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 53 deletions.
20 changes: 14 additions & 6 deletions React/Modules/RCTI18nUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,19 @@
+ (instancetype)sharedInstance;

- (BOOL)isRTL;
- (BOOL)isRTLAllowed;
- (void)allowRTL:(BOOL)value;
- (BOOL)isRTLForced;
- (void)forceRTL:(BOOL)value;
- (BOOL)doLeftAndRightSwapInRTL;
- (void)swapLeftAndRightInRTL:(BOOL)value;

/**
* Should be used very early during app start up
* Before the bridge is initialized
*/
@property (atomic, setter=allowRTL:) BOOL isRTLAllowed;

/**
* Could be used to test RTL layout with English
* Used for development and testing purpose
*/
@property (atomic, setter=forceRTL:) BOOL isRTLForced;

@property (atomic, setter=swapLeftAndRightInRTL:) BOOL doLeftAndRightSwapInRTL;

@end
48 changes: 1 addition & 47 deletions React/Modules/RCTI18nUtil.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ + (instancetype)sharedInstance
dispatch_once(&onceToken, ^{
sharedInstance = [self new];
[sharedInstance swapLeftAndRightInRTL:true];
[sharedInstance allowRTL:true];
});

return sharedInstance;
Expand All @@ -40,53 +41,6 @@ - (BOOL)isRTL
return NO;
}

/**
* Should be used very early during app start up
* Before the bridge is initialized
* @return whether the app allows RTL layout, default is true
*/
- (BOOL)isRTLAllowed
{
NSNumber *value = [[NSUserDefaults standardUserDefaults] objectForKey:@"RCTI18nUtil_allowRTL"];
if (value == nil) {
return YES;
}
return [value boolValue];
}

- (void)allowRTL:(BOOL)rtlStatus
{
[[NSUserDefaults standardUserDefaults] setBool:rtlStatus forKey:@"RCTI18nUtil_allowRTL"];
[[NSUserDefaults standardUserDefaults] synchronize];
}

/**
* Could be used to test RTL layout with English
* Used for development and testing purpose
*/
- (BOOL)isRTLForced
{
BOOL rtlStatus = [[NSUserDefaults standardUserDefaults] boolForKey:@"RCTI18nUtil_forceRTL"];
return rtlStatus;
}

- (void)forceRTL:(BOOL)rtlStatus
{
[[NSUserDefaults standardUserDefaults] setBool:rtlStatus forKey:@"RCTI18nUtil_forceRTL"];
[[NSUserDefaults standardUserDefaults] synchronize];
}

- (BOOL)doLeftAndRightSwapInRTL
{
return [[NSUserDefaults standardUserDefaults] boolForKey:@"RCTI18nUtil_makeRTLFlipLeftAndRightStyles"];
}

- (void)swapLeftAndRightInRTL:(BOOL)value
{
[[NSUserDefaults standardUserDefaults] setBool:value forKey:@"RCTI18nUtil_makeRTLFlipLeftAndRightStyles"];
[[NSUserDefaults standardUserDefaults] synchronize];
}

// Check if the current device language is RTL
- (BOOL)isDevicePreferredLanguageRTL
{
Expand Down

0 comments on commit fcead14

Please sign in to comment.