[GitHub] wqyfavor closed pull request #1280: [WEEX-467][iOS] Fix multithread issues related to transition animation.

[GitBox <gi...@apache.org>]

wqyfavor closed pull request #1280: [WEEX-467][iOS] Fix multithread issues related to transition animation.
URL: https://github.com/apache/incubator-weex/pull/1280
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/ios/sdk/WeexSDK/Sources/Component/WXComponent_internal.h b/ios/sdk/WeexSDK/Sources/Component/WXComponent_internal.h
index 5f63065bda..d8e2f8477e 100644
--- a/ios/sdk/WeexSDK/Sources/Component/WXComponent_internal.h
+++ b/ios/sdk/WeexSDK/Sources/Component/WXComponent_internal.h
@@ -152,6 +152,11 @@ typedef id (^WXDataBindingBlock)(NSDictionary *data, BOOL *needUpdate);
     NSMutableDictionary<NSString *, NSArray *> *_eventParameters;
 }
 
+/* _transform may be modified in mutiple threads. DO NOT use "_transform = XXX" directly.
+ Ivar access in ObjC is compiled to code with additional release or retain. So use Ivar in mutiple
+ thread may lead to crash. Use an ATOMIC property is well enough. */
+@property (atomic, strong) WXTransform *transform;
+
 ///--------------------------------------
 /// @name Package Internal Methods
 ///--------------------------------------
diff --git a/ios/sdk/WeexSDK/Sources/Model/WXComponent.mm b/ios/sdk/WeexSDK/Sources/Model/WXComponent.mm
index 58df862523..d877868eba 100644
--- a/ios/sdk/WeexSDK/Sources/Model/WXComponent.mm
+++ b/ios/sdk/WeexSDK/Sources/Model/WXComponent.mm
@@ -69,6 +69,8 @@ @implementation WXComponent
     __weak WXSDKInstance *_weexInstance;
 }
 
+@synthesize transform = _transform;
+
 #pragma mark Life Cycle
 
 - (instancetype)initWithRef:(NSString *)ref
@@ -758,7 +760,7 @@ - (void)setNativeTransform:(CGAffineTransform)transform
 {
     WXAssertMainThread();
     
-    _transform = [[WXTransform alloc] initWithNativeTransform:CATransform3DMakeAffineTransform(transform) instance:self.weexInstance];
+    self.transform = [[WXTransform alloc] initWithNativeTransform:CATransform3DMakeAffineTransform(transform) instance:self.weexInstance];
     if (!CGRectEqualToRect(self.calculatedFrame, CGRectZero)) {
         [_transform applyTransformForView:_view];
         [_layer setNeedsDisplay];
diff --git a/ios/sdk/WeexSDK/Sources/Module/WXAnimationModule.m b/ios/sdk/WeexSDK/Sources/Module/WXAnimationModule.m
index c8afd2bac3..6ed94414b3 100644
--- a/ios/sdk/WeexSDK/Sources/Module/WXAnimationModule.m
+++ b/ios/sdk/WeexSDK/Sources/Module/WXAnimationModule.m
@@ -264,7 +264,7 @@ - (void)transition:(NSString *)nodeRef args:(NSDictionary *)args callback:(WXMod
                 newInfo.toValue = @([wxTransform.translateY valueForMaximum:view.bounds.size.height]);
                 [infos addObject:newInfo];
             }
-            target->_transform = wxTransform;
+            target.transform = wxTransform;
         } else if ([property isEqualToString:@"backgroundColor"]) {
             info.propertyName = @"backgroundColor";
             info.fromValue = (__bridge id)(layer.backgroundColor);
diff --git a/ios/sdk/WeexSDK/Sources/Module/WXTransition.mm b/ios/sdk/WeexSDK/Sources/Module/WXTransition.mm
index 8e982ac00c..d185981dc7 100644
--- a/ios/sdk/WeexSDK/Sources/Module/WXTransition.mm
+++ b/ios/sdk/WeexSDK/Sources/Module/WXTransition.mm
@@ -256,7 +256,7 @@ - (void)_dealTransitionWithProperty:(NSString *)singleProperty
                 info.perValue = @([wxTransform.translateY floatValue] - [oldTransform.translateY floatValue]);
                 [_propertyArray addObject:info];
             }
-            _targetComponent->_transform = wxTransform;
+            _targetComponent.transform = wxTransform;
         }
         else
         {
@@ -355,8 +355,13 @@ - (void)_calculatetransitionProcessingStyle
             [_oldFilterStyles setObject:@(currentValue) forKey:info.propertyName];
         }
     }
+    
+    /* _oldFilterStyles could be modified in current thread while _updateViewStyles uses it in main thread.
+     This may lead to crash in _updateViewStyles because the dictionary items may be retained or
+     released multiple times by code like styles[@"transform"]. So we copy _oldFilterStyles and use a duplicate.*/
+    NSDictionary* dupStyles = [NSDictionary dictionaryWithDictionary:_oldFilterStyles];
     WXPerformBlockOnMainThread(^{
-        [_targetComponent _updateViewStyles:_oldFilterStyles];
+        [_targetComponent _updateViewStyles:dupStyles];
     });
     [_targetComponent _updateCSSNodeStyles:_oldFilterStyles];
     [_targetComponent.weexInstance.componentManager startComponentTasks];
diff --git a/ios/sdk/WeexSDK/Sources/View/WXComponent+ViewManagement.mm b/ios/sdk/WeexSDK/Sources/View/WXComponent+ViewManagement.mm
index 740e1681f8..1fccb5c209 100644
--- a/ios/sdk/WeexSDK/Sources/View/WXComponent+ViewManagement.mm
+++ b/ios/sdk/WeexSDK/Sources/View/WXComponent+ViewManagement.mm
@@ -252,11 +252,12 @@ - (void)_updateViewStyles:(NSDictionary *)styles
     }
     if (styles[@"transform"]) {
         id transformOrigin = styles[@"transformOrigin"] ?: self.styles[@"transformOrigin"];
-        _transform = [[WXTransform alloc] initWithCSSValue:[WXConvert NSString:styles[@"transform"]] origin:[WXConvert NSString:transformOrigin] instance:self.weexInstance];
+        WXTransform* transform = [[WXTransform alloc] initWithCSSValue:[WXConvert NSString:styles[@"transform"]] origin:[WXConvert NSString:transformOrigin] instance:self.weexInstance];
         if (!CGRectEqualToRect(self.calculatedFrame, CGRectZero)) {
-            [_transform applyTransformForView:_view];
+            [transform applyTransformForView:_view];
             [_layer setNeedsDisplay];
         }
+        self.transform = transform;
     }else if (styles[@"transformOrigin"]) {
         [_transform setTransformOrigin:[WXConvert NSString:styles[@"transformOrigin"]]];
         if (!CGRectEqualToRect(self.calculatedFrame, CGRectZero)) {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services