fix(ios): potential thread race in Vsync manager (#4185)

renderer/native/ios/renderer/HippyUIManager.mm

@@ -38,7 +38,7 @@
 #import "HippyRenderUtils.h"
 #import "HippyView.h"
 #import "HippyViewManager.h"
-#import "RenderVsyncManager.h"
+#import "HippyVsyncManager.h"
 #import "UIView+DomEvent.h"
 #import "UIView+Hippy.h"
 #import "HippyBridgeModule.h"
@@ -1203,7 +1203,7 @@ NSString *const HippyFontChangeTriggerNotification = @"HippyFontChangeTriggerNot
                 NSString *vsyncKey = [NSString stringWithFormat:@"%p-%d", strongSelf, node_id];
                 auto event = std::make_shared<hippy::DomEvent>(name_, node);
                 std::weak_ptr<DomNode> weakNode = node;
-                [[RenderVsyncManager sharedInstance] registerVsyncObserver:^{
+                [[HippyVsyncManager sharedInstance] registerVsyncObserver:^{
                     __strong __typeof(weakSelf)strongSelf = weakSelf;
                     HippyBridge *bridge = strongSelf.bridge;
                     [bridge.javaScriptExecutor executeAsyncBlockOnJavaScriptQueue:^{
@@ -1407,7 +1407,7 @@ NSString *const HippyFontChangeTriggerNotification = @"HippyFontChangeTriggerNot
            if (node) {
                //for kVSyncKey event, node is rootnode
                NSString *vsyncKey = [NSString stringWithFormat:@"%p-%d", self, node_id];
-               [[RenderVsyncManager sharedInstance] unregisterVsyncObserverForKey:vsyncKey];
+               [[HippyVsyncManager sharedInstance] unregisterVsyncObserverForKey:vsyncKey];
            }
        }];
    } else {
@@ -1421,7 +1421,7 @@ NSString *const HippyFontChangeTriggerNotification = @"HippyFontChangeTriggerNot
 
 - (void)removeVSyncEventOnRootNode:(std::weak_ptr<hippy::RootNode>)rootNode {
     NSString *vsyncKey = [NSString stringWithFormat:@"%p-%d", self, static_cast<int>(rootNode.lock()->GetId())];
-    [[RenderVsyncManager sharedInstance] unregisterVsyncObserverForKey:vsyncKey];
+    [[HippyVsyncManager sharedInstance] unregisterVsyncObserverForKey:vsyncKey];
 }
 
 - (void)addPropertyEvent:(const std::string &)name

renderer/native/ios/renderer/HippyVsyncManager.h

@@ -0,0 +1,88 @@
+/*!
+ * iOS SDK
+ *
+ * Tencent is pleased to support the open source community by making
+ * Hippy available.
+ *
+ * Copyright (C) 2019 THL A29 Limited, a Tencent company.
+ * All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#import <Foundation/Foundation.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+/**
+ A manager for coordinating V-sync synchronized callbacks.
+ Provides thread-safe registration of display link observers with configurable refresh rates.
+ */
+@interface HippyVsyncManager : NSObject
+
+///-------------------
+/// @name Singleton
+///-------------------
+
+/**
+ Returns the shared vsync manager instance.
+ 
+ @discussion Use this singleton instance to coordinate vsync observers across the application.
+ */
++ (instancetype)sharedInstance;
+
+///-------------------
+/// @name Registration
+///-------------------
+
+/**
+ Registers a vsync observer with default 60Hz refresh rate.
+ 
+ @param observer The block to execute on each vsync callback
+ @param key A unique identifier for the observer
+ 
+ @discussion Re-registering with the same key will replace the existing observer.
+ */
+- (void)registerVsyncObserver:(dispatch_block_t)observer forKey:(NSString *)key;
+
+/**
+ Registers a vsync observer with custom refresh rate.
+ 
+ @param observer The block to execute on each vsync callback
+ @param rate The desired refresh rate in Hz (1-120)
+ @param key A unique identifier for the observer
+ 
+ @discussion On iOS versions prior to 15, rates above 60Hz will be capped to 60Hz.
+ */
+- (void)registerVsyncObserver:(dispatch_block_t)observer rate:(float)rate forKey:(NSString *)key;
+
+/**
+ Unregisters and invalidates the vsync observer for the specified key.
+ 
+ @param key The identifier used during registration
+ 
+ @discussion Safe to call even if no observer exists for the key.
+ */
+- (void)unregisterVsyncObserverForKey:(NSString *)key;
+
+///--------------------
+/// @name Restrictions
+///--------------------
+
+- (instancetype)init NS_UNAVAILABLE;
++ (instancetype)new NS_UNAVAILABLE;
+
+@end
+
+NS_ASSUME_NONNULL_END
+

renderer/native/ios/renderer/HippyVsyncManager.m

@@ -0,0 +1,151 @@
+/*!
+ * iOS SDK
+ *
+ * Tencent is pleased to support the open source community by making
+ * Hippy available.
+ *
+ * Copyright (C) 2019 THL A29 Limited, a Tencent company.
+ * All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#import <QuartzCore/CADisplayLink.h>
+#import "HippyVsyncManager.h"
+#import "HippyAssert.h"
+#import <objc/runtime.h>
+
+// MARK: - CADisplayLink Category for Vsync Handling
+@interface CADisplayLink (Vsync)
+
+/// Associated block to be executed on vsync
+@property(nonatomic, copy) dispatch_block_t block;
+
+/// Applies refresh rate with validation
+/// @param rate Target refresh rate (1-120 Hz)
+- (void)applyRefreshRate:(float)rate;
+
+@end
+
+@implementation CADisplayLink (Vsync)
+
+- (void)setBlock:(dispatch_block_t)block {
+    // Use COPY association to maintain block ownership
+    objc_setAssociatedObject(self, @selector(block), block, OBJC_ASSOCIATION_COPY);
+}
+
+- (dispatch_block_t)block {
+    return objc_getAssociatedObject(self, _cmd);
+}
+
+- (void)applyRefreshRate:(float)rate {
+    // Validate refresh rate boundaries
+    HippyAssert(rate >= 1 && rate <= 120, @"VSync refresh rate must be between 1 and 120 Hz");
+    
+    if (@available(iOS 15.0, *)) {
+        CAFrameRateRange rateRange = CAFrameRateRangeMake(rate, rate, rate);
+        self.preferredFrameRateRange = rateRange;
+    } else {
+        // Cap to 60 FPS for devices below iOS 15
+        self.preferredFramesPerSecond = MIN(rate, 60);
+    }
+}
+
+@end
+
+// MARK: - Vsync Manager Implementation
+@interface HippyVsyncManager () {
+    NSMutableDictionary<NSString *, CADisplayLink *> *_observers;
+    dispatch_semaphore_t _semaphore;
+}
+
+@end
+
+@implementation HippyVsyncManager
+
+// MARK: - Singleton Pattern
++ (instancetype)sharedInstance {
+    static dispatch_once_t onceToken;
+    static HippyVsyncManager *instance;
+    dispatch_once(&onceToken, ^{
+        instance = [[HippyVsyncManager alloc] init];
+    });
+    return instance;
+}
+
+- (instancetype)init {
+    self = [super init];
+    if (self) {
+        _observers = [NSMutableDictionary dictionary];
+        _semaphore = dispatch_semaphore_create(1);
+    }
+    return self;
+}
+
+// MARK: - Vsync Callback Handling
+- (void)vsyncSignalInvoked:(CADisplayLink *)displayLink {
+    // Execute block if exists
+    dispatch_block_t block = displayLink.block;
+    if (block) {
+        block();
+    }
+}
+
+// MARK: - Public Interface
+- (void)registerVsyncObserver:(dispatch_block_t)observer forKey:(NSString *)key {
+    // Default to 60Hz refresh rate
+    [self registerVsyncObserver:observer rate:60.0f forKey:key];
+}
+
+- (void)registerVsyncObserver:(dispatch_block_t)observer rate:(float)rate forKey:(NSString *)key {
+    if (!observer || !key) {
+        HippyAssert(NO, @"Invalid parameters for observer registration");
+        return;
+    }
+    
+    // Remove existing observer for key
+    [self unregisterVsyncObserverForKey:key];
+    
+    // Create and configure display link
+    CADisplayLink *vsync = [CADisplayLink displayLinkWithTarget:self selector:@selector(vsyncSignalInvoked:)];
+    [vsync applyRefreshRate:rate];
+    [vsync addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
+    vsync.block = observer;
+    
+    // Thread-safe dictionary update
+    dispatch_semaphore_wait(_semaphore, DISPATCH_TIME_FOREVER);
+    _observers[key] = vsync;
+    dispatch_semaphore_signal(_semaphore);
+}
+
+- (void)unregisterVsyncObserverForKey:(NSString *)key {
+    if (!key) {
+        HippyAssert(NO, @"Attempted to unregister with nil key");
+        return;
+    }
+    
+    CADisplayLink *vsync = nil;
+    
+    // Thread-safe dictionary access
+    dispatch_semaphore_wait(_semaphore, DISPATCH_TIME_FOREVER);
+    vsync = _observers[key];
+    if (vsync) {
+        [_observers removeObjectForKey:key];
+    }
+    dispatch_semaphore_signal(_semaphore);
+    
+    // Invalidate outside lock to prevent deadlocks
+    [vsync invalidate];
+}
+
+@end

renderer/native/ios/renderer/NativeRenderManager.mm

@@ -24,7 +24,7 @@
 #import "HippyUIManager+Private.h"
 #import "NativeRenderManager.h"
 #import "HippyShadowText.h"
-#import "RenderVsyncManager.h"
+#import "HippyVsyncManager.h"
 #import "HippyAssert.h"
 #include "dom/dom_manager.h"
 #include "dom/layout_node.h"

renderer/native/ios/renderer/RenderVsyncManager.h

@@ -1,35 +0,0 @@
-/*!
- * iOS SDK
- *
- * Tencent is pleased to support the open source community by making
- * Hippy available.
- *
- * Copyright (C) 2019 THL A29 Limited, a Tencent company.
- * All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#import <Foundation/Foundation.h>
-
-@interface RenderVsyncManager : NSObject
-
-+ (instancetype)sharedInstance;
-
-- (void)registerVsyncObserver:(dispatch_block_t)observer forKey:(NSString *)key;
-
-- (void)registerVsyncObserver:(dispatch_block_t)observer rate:(float)rate forKey:(NSString *)key;
-
-- (void)unregisterVsyncObserverForKey:(NSString *)key;
-
-@end

renderer/native/ios/renderer/RenderVsyncManager.m

@@ -1,135 +0,0 @@
-/*!
- * iOS SDK
- *
- * Tencent is pleased to support the open source community by making
- * Hippy available.
- *
- * Copyright (C) 2019 THL A29 Limited, a Tencent company.
- * All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#import <QuartzCore/CADisplayLink.h>
-#import "RenderVsyncManager.h"
-#import "objc/runtime.h"
-
-@interface CADisplayLink (Vsync)
-
-@property(nonatomic, copy)dispatch_block_t block;
-
-- (void)applyRefreshRate:(float)rate;
-
-@end
-
-@implementation CADisplayLink (Vsync)
-
-- (void)setBlock:(dispatch_block_t)block {
-    objc_setAssociatedObject(self, @selector(block), block, OBJC_ASSOCIATION_COPY);
-}
-
-- (dispatch_block_t)block {
-    return objc_getAssociatedObject(self, _cmd);
-}
-
-- (void)applyRefreshRate:(float)rate {
-    NSAssert(1 <= rate && 120 >=rate, @"vsync refresh rate must between 1 and 120");
-    if (@available(iOS 15.0, *)) {
-        CAFrameRateRange rateRange = {rate, rate, rate};
-        self.preferredFrameRateRange = rateRange;
-    }
-    else if (@available(iOS 10.0, *)) {
-        self.preferredFramesPerSecond = rate;
-    }
-    else {
-#if __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0
-        self.frameInterval = 60.f / rate;
-#endif //__IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0
-    }
-}
-
-@end
-
-@interface RenderVsyncManager () {
-    NSMutableDictionary<NSString *, CADisplayLink *> *_observers;
-//    std::mutex _mutex;
-    dispatch_semaphore_t _semaphore;
-}
-
-@end
-
-@implementation RenderVsyncManager
-
-+ (instancetype)sharedInstance {
-    static dispatch_once_t onceToken;
-    static RenderVsyncManager *instance = nil;
-    dispatch_once(&onceToken, ^{
-        instance = [[RenderVsyncManager alloc] init];
-    });
-    return instance;
-}
-
-- (instancetype)init {
-    self = [super init];
-    if (self) {
-        _observers = [NSMutableDictionary dictionaryWithCapacity:8];
-        _semaphore = dispatch_semaphore_create(1);
-    }
-    return self;
-}
-
-- (void)vsyncSignalInvoked:(CADisplayLink *)displayLink {
-    dispatch_block_t block = displayLink.block;
-    if (block) {
-        block();
-    }
-}
-
-- (void)registerVsyncObserver:(dispatch_block_t)observer forKey:(NSString *)key {
-    if (observer && key) {
-        CADisplayLink *vsync = [CADisplayLink displayLinkWithTarget:self selector:@selector(vsyncSignalInvoked:)];
-        [vsync addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
-        vsync.block = observer;
-        dispatch_semaphore_wait(_semaphore, DISPATCH_TIME_FOREVER);
-        [_observers setObject:vsync forKey:key];
-        dispatch_semaphore_signal(_semaphore);
-    }
-}
-
-- (void)registerVsyncObserver:(dispatch_block_t)observer rate:(float)rate forKey:(NSString *)key {
-    if (observer && key) {
-        CADisplayLink *vsync = [CADisplayLink displayLinkWithTarget:self selector:@selector(vsyncSignalInvoked:)];
-        [vsync applyRefreshRate:rate];
-        [vsync addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
-        vsync.block = observer;
-        dispatch_semaphore_wait(_semaphore, DISPATCH_TIME_FOREVER);
-        [_observers setObject:vsync forKey:key];
-        dispatch_semaphore_signal(_semaphore);
-    }
-}
-
-- (void)unregisterVsyncObserverForKey:(NSString *)key {
-    if (key) {
-        CADisplayLink *vsync = [_observers objectForKey:key];
-        if (vsync) {
-            {
-                dispatch_semaphore_wait(_semaphore, DISPATCH_TIME_FOREVER);
-                [_observers removeObjectForKey:key];
-                dispatch_semaphore_signal(_semaphore);
-            }
-            [vsync invalidate];
-        }
-    }
-}
-
-@end