Skip to content

Commit

Permalink
Expose UIManager queue via a static function to prevent race conditions
Browse files Browse the repository at this point in the history
Summary: Having UI modules access the shadowQueue via UIManager.methodQueue is fragile and leads to race conditions in startup, sometimes resulting in an error where the methodQueue is set twice, or not at all.

Reviewed By: javache

Differential Revision: D3304890

fbshipit-source-id: 7198d28314dbec798877fcaaf17ae017d50157e9
  • Loading branch information
nicklockwood authored and Facebook Github Bot 9 committed May 16, 2016
1 parent 1673e00 commit d2934e5
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
5 changes: 5 additions & 0 deletions React/Modules/RCTUIManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
#import "RCTViewManager.h"
#import "RCTRootView.h"

/**
* UIManager queue
*/
RCT_EXTERN dispatch_queue_t RCTGetUIManagerQueue(void);

/**
* Default name for the UIManager queue
*/
Expand Down
44 changes: 25 additions & 19 deletions React/Modules/RCTUIManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ static UIViewAnimationOptions UIViewAnimationOptionsFromRCTAnimationType(RCTAnim

// Use a custom initialization function rather than implementing `+initialize` so that we can control
// when the initialization code runs. `+initialize` runs immediately before the first message is sent
// to the class which may be too late for us. By this time, we may have missed some
// to the class which may be too late for us. By this time, we may have missed some
// `UIKeyboardWillChangeFrameNotification`s.
+ (void)initializeStatics
{
Expand Down Expand Up @@ -208,8 +208,6 @@ - (instancetype)initWithDictionary:(NSDictionary *)config callback:(RCTResponseS

@implementation RCTUIManager
{
dispatch_queue_t _shadowQueue;

// Root views are only mutated on the shadow queue
NSMutableSet<NSNumber *> *_rootViewTags;
NSMutableArray<dispatch_block_t> *_pendingUIBlocks;
Expand All @@ -235,7 +233,7 @@ @implementation RCTUIManager
- (void)didReceiveNewContentSizeMultiplier
{
__weak RCTUIManager *weakSelf = self;
dispatch_async(self.methodQueue, ^{
dispatch_async(RCTGetUIManagerQueue(), ^{
RCTUIManager *strongSelf = weakSelf;
if (strongSelf) {
[[NSNotificationCenter defaultCenter] postNotificationName:RCTUIManagerWillUpdateViewsDueToContentSizeMultiplierChangeNotification
Expand Down Expand Up @@ -342,22 +340,29 @@ - (void)setBridge:(RCTBridge *)bridge
selector:@selector(interfaceOrientationWillChange:)
name:UIApplicationWillChangeStatusBarOrientationNotification
object:nil];

[RCTAnimation initializeStatics];
}

- (dispatch_queue_t)methodQueue
dispatch_queue_t RCTGetUIManagerQueue(void)
{
if (!_shadowQueue) {
static dispatch_queue_t shadowQueue;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
if ([NSOperation instancesRespondToSelector:@selector(qualityOfService)]) {
dispatch_queue_attr_t attr = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_USER_INTERACTIVE, 0);
_shadowQueue = dispatch_queue_create(RCTUIManagerQueueName, attr);
shadowQueue = dispatch_queue_create(RCTUIManagerQueueName, attr);
} else {
_shadowQueue = dispatch_queue_create(RCTUIManagerQueueName, DISPATCH_QUEUE_SERIAL);
dispatch_set_target_queue(_shadowQueue, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0));
shadowQueue = dispatch_queue_create(RCTUIManagerQueueName, DISPATCH_QUEUE_SERIAL);
dispatch_set_target_queue(shadowQueue, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0));
}
}
return _shadowQueue;
});
return shadowQueue;
}

- (dispatch_queue_t)methodQueue
{
return RCTGetUIManagerQueue();
}

- (void)registerRootView:(UIView *)rootView withSizeFlexibility:(RCTRootViewSizeFlexibility)sizeFlexibility
Expand All @@ -378,7 +383,7 @@ - (void)registerRootView:(UIView *)rootView withSizeFlexibility:(RCTRootViewSize

// Register shadow view
__weak RCTUIManager *weakSelf = self;
dispatch_async(_shadowQueue, ^{
dispatch_async(RCTGetUIManagerQueue(), ^{
RCTUIManager *strongSelf = weakSelf;
if (!_viewRegistry) {
return;
Expand Down Expand Up @@ -419,7 +424,7 @@ - (void)setFrame:(CGRect)frame forView:(UIView *)view
}

NSNumber *reactTag = view.reactTag;
dispatch_async(_shadowQueue, ^{
dispatch_async(RCTGetUIManagerQueue(), ^{
RCTShadowView *shadowView = _shadowViewRegistry[reactTag];
RCTAssert(shadowView != nil, @"Could not locate shadow view with tag #%@", reactTag);

Expand Down Expand Up @@ -452,7 +457,7 @@ - (void)setIntrinsicContentSize:(CGSize)size forView:(UIView *)view
RCTAssertMainThread();

NSNumber *reactTag = view.reactTag;
dispatch_async(_shadowQueue, ^{
dispatch_async(RCTGetUIManagerQueue(), ^{
RCTShadowView *shadowView = _shadowViewRegistry[reactTag];
RCTAssert(shadowView != nil, @"Could not locate root view with tag #%@", reactTag);

Expand All @@ -469,7 +474,7 @@ - (void)setBackgroundColor:(UIColor *)color forView:(UIView *)view
NSNumber *reactTag = view.reactTag;

__weak RCTUIManager *weakSelf = self;
dispatch_async(_shadowQueue, ^{
dispatch_async(RCTGetUIManagerQueue(), ^{
RCTUIManager *strongSelf = weakSelf;
if (!_viewRegistry) {
return;
Expand Down Expand Up @@ -505,9 +510,9 @@ - (void)_purgeChildren:(NSArray<id<RCTComponent>> *)children

- (void)addUIBlock:(RCTViewManagerUIBlock)block
{
RCTAssertThread(_shadowQueue,
RCTAssertThread(RCTGetUIManagerQueue(),
@"-[RCTUIManager addUIBlock:] should only be called from the "
"UIManager's _shadowQueue (it may be accessed via `bridge.uiManager.methodQueue`)");
"UIManager's queue (get this using `RCTGetUIManagerQueue()`)");

if (!block) {
return;
Expand Down Expand Up @@ -1103,7 +1108,8 @@ - (void)_layoutAndMount

- (void)flushUIBlocks
{
RCTAssertThread(_shadowQueue, @"flushUIBlocks can only be called from the shadow queue");
RCTAssertThread(RCTGetUIManagerQueue(),
@"flushUIBlocks can only be called from the shadow queue");

// First copy the previous blocks into a temporary variable, then reset the
// pending blocks to a new array. This guards against mutation while
Expand Down
5 changes: 1 addition & 4 deletions React/Views/RCTViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ @implementation RCTViewManager

- (dispatch_queue_t)methodQueue
{
RCTAssert(_bridge, @"Bridge not set");
RCTAssert(_bridge.uiManager || !_bridge.valid, @"UIManager not initialized");
RCTAssert(_bridge.uiManager.methodQueue || !_bridge.valid, @"UIManager.methodQueue not initialized");
return _bridge.uiManager.methodQueue;
return RCTGetUIManagerQueue();
}

- (UIView *)view
Expand Down

0 comments on commit d2934e5

Please sign in to comment.