Skip to content

Commit

Permalink
Fabric: Fixed threading issue in RCTNativeAnimatedModule
Browse files Browse the repository at this point in the history
Summary:
The previous version of the code accessed `_animIdIsManagedByFabric` on the main thread (which is should be accessed on the UIManager thread) and called `flushOperationQueues` on the main thread as well (also must be called on UIManager thread because it modifies instance variables (e.g. `_operations`) which supposed to be accessed on UIManager thread).

The diff fixes that introducing an additional queue jump. That's should be fine because the overall architecture of RCTNativeAnimatedModule is appeared to be asynchronous and should be resilient to possible races.

Reviewed By: sammy-SC

Differential Revision: D17523958

fbshipit-source-id: c4b4ce38b68b009726b2f6c28c38b32b9f9d6921
  • Loading branch information
shergin authored and facebook-github-bot committed Sep 23, 2019
1 parent 034a7d1 commit 30e9443
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions Libraries/NativeAnimation/RCTNativeAnimatedModule.m
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,16 @@ - (void)setBridge:(RCTBridge *)bridge
[self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
[nodesManager startAnimatingNode:animationId nodeTag:nodeTag config:config endCallback:callBack];
}];
__weak RCTNativeAnimatedModule *weakSelf = self;

RCTExecuteOnMainQueue(^{
__strong RCTNativeAnimatedModule *strongSelf = weakSelf;
if (strongSelf && [strongSelf->_nodesManager isNodeManagedByFabric:nodeTag]) {
strongSelf->_animIdIsManagedByFabric[animationId] = @YES;
[strongSelf flushOperationQueues];
}
if (![self->_nodesManager isNodeManagedByFabric:nodeTag]) {
return;
}

RCTExecuteOnUIManagerQueue(^{
self->_animIdIsManagedByFabric[animationId] = @YES;
[self flushOperationQueues];
});
});
}

Expand Down

0 comments on commit 30e9443

Please sign in to comment.