Skip to content

Commit

Permalink
fix(ios): potential thread race in Vsync manager
Browse files Browse the repository at this point in the history
  • Loading branch information
wwwcg committed Feb 13, 2025
1 parent 78ba9a8 commit 6b7e338
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 175 deletions.
8 changes: 4 additions & 4 deletions renderer/native/ios/renderer/HippyUIManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1203,7 +1203,7 @@ - (void)addEventName:(const std::string &)name
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:^{
Expand Down Expand Up @@ -1407,7 +1407,7 @@ - (void)removeEventName:(const std::string &)eventName
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 {
Expand All @@ -1421,7 +1421,7 @@ - (void)removeEventName:(const std::string &)eventName

- (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
Expand Down
88 changes: 88 additions & 0 deletions renderer/native/ios/renderer/HippyVsyncManager.h
Original file line number Diff line number Diff line change
@@ -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

151 changes: 151 additions & 0 deletions renderer/native/ios/renderer/HippyVsyncManager.m
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion renderer/native/ios/renderer/NativeRenderManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
35 changes: 0 additions & 35 deletions renderer/native/ios/renderer/RenderVsyncManager.h

This file was deleted.

Loading

0 comments on commit 6b7e338

Please sign in to comment.