From b6f3f745e4f3064acc08599aca727feb39563986 Mon Sep 17 00:00:00 2001 From: wwwcg Date: Fri, 28 Feb 2025 17:41:28 +0800 Subject: [PATCH] fix(ios): resolve animation thread race & add protection for running script --- ios/sdk/base/executors/HippyJSCExecutor.mm | 26 ++++++++-- .../animation2/HippyNextAnimationModule.m | 50 ++++++++++++++++--- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/ios/sdk/base/executors/HippyJSCExecutor.mm b/ios/sdk/base/executors/HippyJSCExecutor.mm index 6cd9b7899b1..58cfc0abc2d 100644 --- a/ios/sdk/base/executors/HippyJSCExecutor.mm +++ b/ios/sdk/base/executors/HippyJSCExecutor.mm @@ -744,7 +744,8 @@ static void handleJsExcepiton(std::shared_ptr scope) { HippyPerformanceLogger *performanceLogger, JSGlobalContextRef ctx) { @autoreleasepool { - HippyLogInfo(@"load script begin, length %zd for url %@", [script length], [sourceURL absoluteString]); + NSString *absoluteSourceUrl = [sourceURL absoluteString]; + HippyLogInfo(@"load script begin, length %zd for url %@", [script length], absoluteSourceUrl); if (isCommonBundle) { [performanceLogger markStartForTag:HippyPLCommonScriptExecution]; } else { @@ -753,9 +754,26 @@ static void handleJsExcepiton(std::shared_ptr scope) { JSValueRef jsError = NULL; NSString *scriptText = [[NSString alloc] initWithData:script encoding:NSUTF8StringEncoding]; + if (!absoluteSourceUrl) { + HippyLogError(@"Missing bundle URL for script execution"); + return [NSError errorWithDomain:HippyErrorDomain + code:NSFileNoSuchFileError + userInfo:@{ + NSLocalizedDescriptionKey: @"JavaScript bundle URL is required", + }]; + } + + if (!scriptText) { + HippyLogError(@"Script decoding failed, data length: %lu bytes", (unsigned long)script.length); + return [NSError errorWithDomain:HippyErrorDomain + code:NSFileReadInapplicableStringEncodingError + userInfo:@{ + NSLocalizedDescriptionKey: @"Failed to decode script content", + }]; + } JSStringRef execJSString = JSStringCreateWithCFString((__bridge CFStringRef)scriptText); - //JSStringCreateWithUTF8CString((const char *)script.bytes); - JSStringRef bundleURL = JSStringCreateWithCFString((__bridge CFStringRef)sourceURL.absoluteString); + JSStringRef bundleURL = JSStringCreateWithCFString((__bridge CFStringRef)absoluteSourceUrl); + NSLock *lock = jslock(); BOOL lockSuccess = [lock lockBeforeDate:[NSDate dateWithTimeIntervalSinceNow:1]]; @@ -773,7 +791,7 @@ static void handleJsExcepiton(std::shared_ptr scope) { NSError *error = jsError ? HippyNSErrorFromJSErrorRef(jsError, ctx) : nil; // HIPPY_PROFILE_END_EVENT(0, @"js_call"); - HippyLogInfo(@"load script end,length %zd for url %@, error %@", [script length], [sourceURL absoluteString], [error description]); + HippyLogInfo(@"load script end,length %zd for url %@, error %@", [script length], absoluteSourceUrl, [error description]); return error; } diff --git a/ios/sdk/module/animation2/HippyNextAnimationModule.m b/ios/sdk/module/animation2/HippyNextAnimationModule.m index f3239b207d1..28c0f7755c7 100644 --- a/ios/sdk/module/animation2/HippyNextAnimationModule.m +++ b/ios/sdk/module/animation2/HippyNextAnimationModule.m @@ -31,12 +31,14 @@ @interface HippyNextAnimationModule () -/// Map of id-animation -@property (atomic, strong) NSMutableDictionary *animationById; -/// Map of hippyTag-Params -@property (atomic, strong) NSMutableDictionary *paramsByHippyTag; -/// Map of AnimationId-Params -@property (atomic, strong) NSMutableDictionary *> *paramsByAnimationId; +/// Lock for animationById & paramsByHippyTag +@property (nonatomic, strong) NSLock *dataAccessLock; +/// Map of id-animation, may access only in shadow and main thread +@property (nonatomic, strong) NSMutableDictionary *animationById; +/// Map of hippyTag-Params, may access only in shadow and main thread +@property (nonatomic, strong) NSMutableDictionary *paramsByHippyTag; +/// Map of AnimationId-Params, access only in shadow thread +@property (nonatomic, strong) NSMutableDictionary *> *paramsByAnimationId; /// whether should relayout on next frame @property (atomic, assign) BOOL shouldCallUIManagerToUpdateLayout; @@ -67,8 +69,10 @@ - (dispatch_queue_t)methodQueue { } - (void)invalidate { + [self.dataAccessLock lock]; [self.animationById removeAllObjects]; [self.paramsByHippyTag removeAllObjects]; + [self.dataAccessLock unlock]; [self.paramsByAnimationId removeAllObjects]; [HPOPAnimator.sharedAnimator removeAnimatorDelegate:self]; } @@ -76,6 +80,7 @@ - (void)invalidate { - (instancetype)init { self = [super init]; if (self) { + _dataAccessLock = [[NSLock alloc] init]; _animationById = [NSMutableDictionary dictionary]; _paramsByHippyTag = [NSMutableDictionary dictionary]; _paramsByAnimationId = [NSMutableDictionary dictionary]; @@ -101,7 +106,9 @@ - (instancetype)init { anim.controlDelegate = self; // save animation + [self.dataAccessLock lock]; self.animationById[animationId] = anim; + [self.dataAccessLock unlock]; } HIPPY_EXPORT_METHOD(createAnimationSet:(NSNumber *__nonnull)animationId @@ -112,7 +119,9 @@ - (instancetype)init { for (NSDictionary * info in children) { NSNumber *subAnimationId = info[@"animationId"]; BOOL follow = [info[@"follow"] boolValue]; + [self.dataAccessLock lock]; HippyNextAnimation *ani = self.animationById[subAnimationId]; + [self.dataAccessLock unlock]; if (ani == nil) { HippyAssert(ani != nil, @"create group animation but use illege sub animaiton"); return; @@ -125,11 +134,15 @@ - (instancetype)init { group.animations = anis; group.animationId = animationId; group.delegate = self; + [self.dataAccessLock lock]; self.animationById[animationId] = group; + [self.dataAccessLock unlock]; } HIPPY_EXPORT_METHOD(startAnimation:(NSNumber *__nonnull)animationId) { + [self.dataAccessLock lock]; HippyNextAnimation *anim = self.animationById[animationId]; + [self.dataAccessLock unlock]; if (HippyNextAnimationStartedState == anim.state) { return; } @@ -172,19 +185,25 @@ - (instancetype)init { } HIPPY_EXPORT_METHOD(pauseAnimation:(NSNumber *__nonnull)animationId) { + [self.dataAccessLock lock]; HippyNextAnimation *anim = self.animationById[animationId]; + [self.dataAccessLock unlock]; [anim setPausedWithoutReset:YES]; } HIPPY_EXPORT_METHOD(resumeAnimation:(NSNumber *__nonnull)animationId) { + [self.dataAccessLock lock]; HippyNextAnimation *anim = self.animationById[animationId]; + [self.dataAccessLock unlock]; [anim setPausedWithoutReset:NO]; } HIPPY_EXPORT_METHOD(updateAnimation:(NSNumber *__nonnull)animationId params:(NSDictionary *)params) { if (!params) return; + [self.dataAccessLock lock]; HippyNextAnimation *anim = self.animationById[animationId]; + [self.dataAccessLock unlock]; anim.state = HippyNextAnimationInitState; [anim updateAnimation:params]; @@ -193,7 +212,9 @@ - (instancetype)init { [p.animationIdWithPropDictionary enumerateKeysAndObjectsUsingBlock:^(NSString * _Nonnull key, NSNumber * _Nonnull obj, BOOL * _Nonnull stop) { + [self.dataAccessLock lock]; HippyNextAnimation *rcani = self.animationById[obj]; + [self.dataAccessLock unlock]; if ([obj isEqual:animationId]) { [p setValue:[rcani getPretreatedFromValueForAnimType:key] forProp:key]; HippyLogInfo(@"[Hippy_OC_Log][Animation], Update_Animation:[%@] key:[%@]", animationId, key); @@ -206,8 +227,9 @@ - (instancetype)init { } HIPPY_EXPORT_METHOD(destroyAnimation:(NSNumber * __nonnull)animationId) { - + [self.dataAccessLock lock]; [self.animationById removeObjectForKey:animationId]; + [self.dataAccessLock unlock]; [self.paramsByAnimationId removeObjectForKey:animationId]; } @@ -227,9 +249,13 @@ - (NSDictionary *)bindAnimaiton:(NSDictionary *)params rootTag:rootTag]; [p parse]; + [self.dataAccessLock lock]; BOOL contain = [self.paramsByHippyTag.allValues containsObject:p]; + [self.dataAccessLock unlock]; [p.animationIdWithPropDictionary enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSNumber *animationId, __unused BOOL *stop) { + [self.dataAccessLock lock]; HippyNextAnimation *ani = self.animationById[animationId]; + [self.dataAccessLock unlock]; if (ani.state == HippyNextAnimationFinishState) { id tmpToValue = [ani getPretreatedToValueForAnimType:key]; @@ -260,7 +286,9 @@ - (NSDictionary *)bindAnimaiton:(NSDictionary *)params }]; // record viewTag and view params + [self.dataAccessLock lock]; [self.paramsByHippyTag setObject:p forKey:viewTag]; + [self.dataAccessLock unlock]; return p.updateParams; } @@ -269,11 +297,15 @@ - (NSDictionary *)bindAnimaiton:(NSDictionary *)params /// - Parameter view: view with animation - (void)connectAnimationToView:(UIView *)view { NSNumber *hippyTag = view.hippyTag; + [self.dataAccessLock lock]; HippyNextAnimationViewParams *p = self.paramsByHippyTag[hippyTag]; + [self.dataAccessLock unlock]; for (NSString *prop in p.animationIdWithPropDictionary.allKeys) { NSNumber *animationId = p.animationIdWithPropDictionary[prop]; + [self.dataAccessLock lock]; HippyNextAnimation *anim = self.animationById[animationId]; + [self.dataAccessLock unlock]; if (HippyNextAnimationReadyState != anim.state) { continue; } @@ -319,7 +351,9 @@ - (void)stopAnimationAndUpdateViewToEndState:(NSNumber * _Nonnull)animationId __unused BOOL *stop) { __strong __typeof(weakSelf)strongSelf = weakSelf; [p.animationIdWithPropDictionary enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSNumber *obj, __unused BOOL *stop1) { + [strongSelf.dataAccessLock lock]; HippyNextAnimation *ani = strongSelf.animationById[obj]; + [strongSelf.dataAccessLock unlock]; if (![obj isEqual:animationId]) { return; } @@ -525,7 +559,9 @@ - (void)hpop_animationDidStop:(HPOPAnimation *)anim finished:(BOOL)finished { __strong __typeof(weakSelf)strongSelf = weakSelf; for (HippyNextAnimationViewParams *p in strongSelf.paramsByAnimationId[animationId]) { [p.animationIdWithPropDictionary enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSNumber *obj, __unused BOOL *stop1) { + [strongSelf.dataAccessLock lock]; HippyNextAnimation *ani = strongSelf.animationById[obj]; + [strongSelf.dataAccessLock unlock]; if (![obj isEqual:animationId]) { return; }