Permalink
Browse files

Fix potential retain cycles in Animated iOS

Summary:
Fixes potential retain cycles detected by an internal fb tool.

```
First:

__NSDictionaryM
-> RCTPropsAnimatedNode
-> _parentNodes -> __NSDictionaryM
-> RCTStyleAnimatedNode
-> _childNodes -> __NSDictionaryM

Second:

RCTScrollView
-> _eventDispatcher -> RCTEventDispatcher
-> _observers -> __NSArrayM
-> RCTNativeAnimatedModule
-> _nodesManager -> RCTNativeAnimatedNodesManager
-> _uiManager -> RCTUIManager
-> _viewRegistry -> __NSDictionaryM
-> RCTScrollView
```

First fix:
Use weak map for parent and child nodes, strong refs are managed by RCTNativeAnimatedNodesManager

Second fix:
Make RCTEventDispatcher observers a weak array and make sure we don't keep strong refs to UIManager in RCTNativeAnimatedNodesManager and RCTPropsAnimatedNode.

Tested that native animations still work in UIExplorer

[IOS] [BUGFIX] [NativeAnimated] - Fix potential retain cycles in Animated iOS
Closes #16506

Differential Revision: D6126400

Pulled By: shergin

fbshipit-source-id: 1ac5083f8ab79a806305edc23ae4796ed428f78b
  • Loading branch information...
janicduplessis authored and facebook-github-bot committed Oct 23, 2017
1 parent 18364e9 commit c47759a9ae757ddaabde560a6b2af92e70a92453
@@ -16,8 +16,8 @@ - (void)performUpdate
[super performUpdate];
NSArray<NSNumber *> *inputNodes = self.config[@"input"];
if (inputNodes.count > 1) {
RCTValueAnimatedNode *parent1 = (RCTValueAnimatedNode *)self.parentNodes[inputNodes[0]];
RCTValueAnimatedNode *parent2 = (RCTValueAnimatedNode *)self.parentNodes[inputNodes[1]];
RCTValueAnimatedNode *parent1 = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNodes[0]];
RCTValueAnimatedNode *parent2 = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNodes[1]];
if ([parent1 isKindOfClass:[RCTValueAnimatedNode class]] &&
[parent2 isKindOfClass:[RCTValueAnimatedNode class]]) {
self.value = parent1.value + parent2.value;
@@ -17,8 +17,8 @@
@property (nonatomic, readonly) NSNumber *nodeTag;
@property (nonatomic, copy, readonly) NSDictionary<NSString *, id> *config;
@property (nonatomic, copy, readonly) NSDictionary<NSNumber *, RCTAnimatedNode *> *childNodes;
@property (nonatomic, copy, readonly) NSDictionary<NSNumber *, RCTAnimatedNode *> *parentNodes;
@property (nonatomic, copy, readonly) NSMapTable<NSNumber *, RCTAnimatedNode *> *childNodes;
@property (nonatomic, copy, readonly) NSMapTable<NSNumber *, RCTAnimatedNode *> *parentNodes;
@property (nonatomic, readonly) BOOL needsUpdate;
@@ -13,8 +13,8 @@
@implementation RCTAnimatedNode
{
NSMutableDictionary<NSNumber *, RCTAnimatedNode *> *_childNodes;
NSMutableDictionary<NSNumber *, RCTAnimatedNode *> *_parentNodes;
NSMapTable<NSNumber *, RCTAnimatedNode *> *_childNodes;
NSMapTable<NSNumber *, RCTAnimatedNode *> *_parentNodes;
}
- (instancetype)initWithTag:(NSNumber *)tag
@@ -29,23 +29,23 @@ - (instancetype)initWithTag:(NSNumber *)tag
RCT_NOT_IMPLEMENTED(- (instancetype)init)
- (NSDictionary<NSNumber *, RCTAnimatedNode *> *)childNodes
- (NSMapTable<NSNumber *, RCTAnimatedNode *> *)childNodes
{
return _childNodes;
}
- (NSDictionary<NSNumber *, RCTAnimatedNode *> *)parentNodes
- (NSMapTable<NSNumber *, RCTAnimatedNode *> *)parentNodes
{
return _parentNodes;
}
- (void)addChild:(RCTAnimatedNode *)child
{
if (!_childNodes) {
_childNodes = [NSMutableDictionary new];
_childNodes = [NSMapTable strongToWeakObjectsMapTable];
}
if (child) {
_childNodes[child.nodeTag] = child;
[_childNodes setObject:child forKey:child.nodeTag];
[child onAttachedToNode:self];
}
}
@@ -64,10 +64,10 @@ - (void)removeChild:(RCTAnimatedNode *)child
- (void)onAttachedToNode:(RCTAnimatedNode *)parent
{
if (!_parentNodes) {
_parentNodes = [NSMutableDictionary new];
_parentNodes = [NSMapTable strongToWeakObjectsMapTable];
}
if (parent) {
_parentNodes[parent.nodeTag] = parent;
[_parentNodes setObject:parent forKey:parent.nodeTag];
}
}
@@ -83,26 +83,26 @@ - (void)onDetachedFromNode:(RCTAnimatedNode *)parent
- (void)detachNode
{
for (RCTAnimatedNode *parent in _parentNodes.allValues) {
for (RCTAnimatedNode *parent in _parentNodes.objectEnumerator) {
[parent removeChild:self];
}
for (RCTAnimatedNode *child in _childNodes.allValues) {
for (RCTAnimatedNode *child in _childNodes.objectEnumerator) {
[self removeChild:child];
}
}
- (void)setNeedsUpdate
{
_needsUpdate = YES;
for (RCTAnimatedNode *child in _childNodes.allValues) {
for (RCTAnimatedNode *child in _childNodes.objectEnumerator) {
[child setNeedsUpdate];
}
}
- (void)updateNodeIfNecessary
{
if (_needsUpdate) {
for (RCTAnimatedNode *parent in _parentNodes.allValues) {
for (RCTAnimatedNode *parent in _parentNodes.objectEnumerator) {
[parent updateNodeIfNecessary];
}
[self performUpdate];
@@ -51,7 +51,7 @@ - (void)performUpdate
- (CGFloat)inputNodeValue
{
RCTValueAnimatedNode *inputNode = (RCTValueAnimatedNode *)self.parentNodes[_inputNodeTag];
RCTValueAnimatedNode *inputNode = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:_inputNodeTag];
if (![inputNode isKindOfClass:[RCTValueAnimatedNode class]]) {
RCTLogError(@"Illegal node ID set as an input for Animated.DiffClamp node");
return 0;
@@ -19,8 +19,8 @@ - (void)performUpdate
NSArray<NSNumber *> *inputNodes = self.config[@"input"];
if (inputNodes.count > 1) {
RCTValueAnimatedNode *parent1 = (RCTValueAnimatedNode *)self.parentNodes[inputNodes[0]];
RCTValueAnimatedNode *parent2 = (RCTValueAnimatedNode *)self.parentNodes[inputNodes[1]];
RCTValueAnimatedNode *parent1 = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNodes[0]];
RCTValueAnimatedNode *parent2 = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNodes[1]];
if ([parent1 isKindOfClass:[RCTValueAnimatedNode class]] &&
[parent2 isKindOfClass:[RCTValueAnimatedNode class]]) {
if (parent2.value == 0) {
@@ -16,7 +16,7 @@ - (void)performUpdate
[super performUpdate];
NSNumber *inputNode = self.config[@"input"];
NSNumber *modulus = self.config[@"modulus"];
RCTValueAnimatedNode *parent = (RCTValueAnimatedNode *)self.parentNodes[inputNode];
RCTValueAnimatedNode *parent = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNode];
self.value = fmodf(parent.value, modulus.floatValue);
}
@@ -17,8 +17,8 @@ - (void)performUpdate
NSArray<NSNumber *> *inputNodes = self.config[@"input"];
if (inputNodes.count > 1) {
RCTValueAnimatedNode *parent1 = (RCTValueAnimatedNode *)self.parentNodes[inputNodes[0]];
RCTValueAnimatedNode *parent2 = (RCTValueAnimatedNode *)self.parentNodes[inputNodes[1]];
RCTValueAnimatedNode *parent1 = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNodes[0]];
RCTValueAnimatedNode *parent2 = (RCTValueAnimatedNode *)[self.parentNodes objectForKey:inputNodes[1]];
if ([parent1 isKindOfClass:[RCTValueAnimatedNode class]] &&
[parent2 isKindOfClass:[RCTValueAnimatedNode class]]) {
self.value = parent1.value * parent2.value;
@@ -20,7 +20,7 @@ @implementation RCTPropsAnimatedNode
{
NSNumber *_connectedViewTag;
NSString *_connectedViewName;
RCTUIManager *_uiManager;
__weak RCTUIManager *_uiManager;
NSMutableDictionary<NSString *, NSObject *> *_propsDictionary;
}
@@ -85,18 +85,18 @@ - (void)performUpdate
if (!_connectedViewTag) {
return;
}
[self.parentNodes enumerateKeysAndObjectsUsingBlock:^(NSNumber *_Nonnull parentTag, RCTAnimatedNode *_Nonnull parentNode, BOOL *_Nonnull stop) {
for (NSNumber *parentTag in self.parentNodes.keyEnumerator) {
RCTAnimatedNode *parentNode = [self.parentNodes objectForKey:parentTag];
if ([parentNode isKindOfClass:[RCTStyleAnimatedNode class]]) {
[self->_propsDictionary addEntriesFromDictionary:[(RCTStyleAnimatedNode *)parentNode propsDictionary]];
} else if ([parentNode isKindOfClass:[RCTValueAnimatedNode class]]) {
NSString *property = [self propertyNameForParentTag:parentTag];
CGFloat value = [(RCTValueAnimatedNode *)parentNode value];
self->_propsDictionary[property] = @(value);
}
}];
}
if (_propsDictionary.count) {
[_uiManager synchronouslyUpdateViewOnUIThread:_connectedViewTag
@@ -37,7 +37,7 @@ - (void)performUpdate
NSDictionary<NSString *, NSNumber *> *style = self.config[@"style"];
[style enumerateKeysAndObjectsUsingBlock:^(NSString *property, NSNumber *nodeTag, __unused BOOL *stop) {
RCTAnimatedNode *node = self.parentNodes[nodeTag];
RCTAnimatedNode *node = [self.parentNodes objectForKey:nodeTag];
if (node) {
if ([node isKindOfClass:[RCTValueAnimatedNode class]]) {
RCTValueAnimatedNode *parentNode = (RCTValueAnimatedNode *)node;
@@ -41,7 +41,7 @@ - (void)performUpdate
NSNumber *value;
if ([type isEqualToString: @"animated"]) {
NSNumber *nodeTag = transformConfig[@"nodeTag"];
RCTAnimatedNode *node = self.parentNodes[nodeTag];
RCTAnimatedNode *node = [self.parentNodes objectForKey:nodeTag];
if (![node isKindOfClass:[RCTValueAnimatedNode class]]) {
continue;
}
@@ -30,7 +30,7 @@
@implementation RCTNativeAnimatedNodesManager
{
RCTUIManager *_uiManager;
__weak RCTUIManager *_uiManager;
NSMutableDictionary<NSNumber *, RCTAnimatedNode *> *_animationNodes;
// Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time
// there will be only one driver per mapping so all code code should be optimized around that.
@@ -46,7 +46,7 @@ @implementation RCTEventDispatcher
// This array contains ids of events in order they come in, so we can emit them to JS in the exact same order.
NSMutableArray<NSNumber *> *_eventQueue;
BOOL _eventsDispatchScheduled;
NSMutableArray<id<RCTEventDispatcherObserver>> *_observers;
NSHashTable<id<RCTEventDispatcherObserver>> *_observers;
NSLock *_observersLock;
}
@@ -61,7 +61,7 @@ - (void)setBridge:(RCTBridge *)bridge
_eventQueue = [NSMutableArray new];
_eventQueueLock = [NSLock new];
_eventsDispatchScheduled = NO;
_observers = [NSMutableArray new];
_observers = [NSHashTable weakObjectsHashTable];
_observersLock = [NSLock new];
}

0 comments on commit c47759a

Please sign in to comment.