Skip to content

Commit

Permalink
fix(ios): potential thread race while accessing bridge
Browse files Browse the repository at this point in the history
Refactoring: remove some debug code
  • Loading branch information
wwwcg committed Sep 13, 2024
1 parent 10d6f1b commit 21a106d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 52 deletions.
3 changes: 1 addition & 2 deletions framework/ios/base/bridge/HippyBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1295,9 +1295,8 @@ - (void)setRootView:(UIView *)rootView {
// But one HippyBridge can only have one UIManager.
HippyUIManager *uiManager = self.uiManager;
if (!uiManager) {
uiManager = [[HippyUIManager alloc] init];
uiManager = [[HippyUIManager alloc] initWithBridge:self];
[uiManager setDomManager:domManager];
[uiManager setBridge:self];
self.uiManager = uiManager;
}

Expand Down
7 changes: 6 additions & 1 deletion renderer/native/ios/renderer/HippyUIManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ HIPPY_EXTERN NSString *const HippyUIManagerDidEndBatchNotification;
@interface HippyUIManager : NSObject <HippyInvalidating>

/// HippyBridge instance
@property (nonatomic, weak) HippyBridge *bridge;
@property (nonatomic, weak, readonly) HippyBridge *bridge;

/// View Registry of all nodes
@property (nonatomic, readonly) HippyComponentMap *viewRegistry;
Expand All @@ -78,6 +78,11 @@ HIPPY_EXTERN NSString *const HippyUIManagerDidEndBatchNotification;
/// default is NO.
@property (nonatomic, assign) BOOL uiCreationLazilyEnabled;

/// Init method
/// - Parameter bridge: HippyBridge
- (instancetype)initWithBridge:(HippyBridge *)bridge NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

/// Gets the view associated with a hippyTag.
/// - Parameters:
Expand Down
54 changes: 5 additions & 49 deletions renderer/native/ios/renderer/HippyUIManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,6 @@ @interface HippyUIManager() {



#if HIPPY_DEBUG
@property(nonatomic, assign) std::unordered_map<int32_t, std::unordered_map<int32_t, std::shared_ptr<hippy::DomNode>>> domNodesMap;
- (std::shared_ptr<hippy::DomNode>)domNodeForTag:(int32_t)dom_tag onRootNode:(int32_t)root_tag;
- (std::vector<std::shared_ptr<hippy::DomNode>>)childrenForNodeTag:(int32_t)tag onRootNode:(int32_t)root_tag;
#endif

@end

@implementation HippyUIManager
Expand All @@ -220,9 +214,10 @@ @implementation HippyUIManager

#pragma mark Life cycle

- (instancetype)init {
- (instancetype)initWithBridge:(HippyBridge *)bridge {
self = [super init];
if (self) {
_bridge = bridge;
[self initContext];
}
return self;
Expand Down Expand Up @@ -671,9 +666,8 @@ - (void)updateView:(nonnull NSNumber *)componentTag
}];
}

#pragma mark - Render Context Implementation

- (__kindof HippyViewManager *)viewManagerForViewName:(NSString *)viewName {
HippyBridge *strongBridge = self.bridge;
if (!_viewManagers) {
_viewManagers = [NSMutableDictionary dictionary];
if (_extraComponents) {
Expand All @@ -686,7 +680,7 @@ - (__kindof HippyViewManager *)viewManagerForViewName:(NSString *)viewName {
[_viewManagers setObject:cls forKey:viewName];
}
}
NSArray<Class> *classes = HippyGetViewManagerClasses(self.bridge);
NSArray<Class> *classes = HippyGetViewManagerClasses(strongBridge);
NSMutableDictionary *defaultViewManagerClasses = [NSMutableDictionary dictionaryWithCapacity:[classes count]];
for (Class cls in classes) {
NSString *viewName = viewNameFromViewManagerClass(cls);
Expand All @@ -701,7 +695,7 @@ - (__kindof HippyViewManager *)viewManagerForViewName:(NSString *)viewName {
id object = [_viewManagers objectForKey:viewName];
if (object_isClass(object)) {
HippyViewManager *viewManager = [object new];
viewManager.bridge = self.bridge;
viewManager.bridge = strongBridge;
NSAssert([viewManager isKindOfClass:[HippyViewManager class]], @"Must be a HippyViewManager instance");
[_viewManagers setObject:viewManager forKey:viewName];
object = viewManager;
Expand Down Expand Up @@ -781,12 +775,6 @@ - (void)createRenderNodes:(std::vector<std::shared_ptr<DomNode>> &&)nodes
if (!strongRootNode) {
return;
}
#if HIPPY_DEBUG
auto &nodeMap = _domNodesMap[strongRootNode->GetId()];
for (auto node : nodes) {
nodeMap[node->GetId()] = node;
}
#endif
NSNumber *rootNodeTag = @(strongRootNode->GetId());
std::lock_guard<std::mutex> lock([self renderQueueLock]);
NativeRenderViewsRelation *manager = [[NativeRenderViewsRelation alloc] init];
Expand Down Expand Up @@ -877,12 +865,6 @@ - (void)updateRenderNodes:(std::vector<std::shared_ptr<DomNode>>&&)nodes
if (!strongRootNode) {
return;
}
#if HIPPY_DEBUG
auto &nodeMap = _domNodesMap[strongRootNode->GetId()];
for (auto node : nodes) {
nodeMap[node->GetId()] = node;
}
#endif
std::lock_guard<std::mutex> lock([self renderQueueLock]);
NSNumber *rootTag = @(strongRootNode->GetId());
for (const auto &node : nodes) {
Expand All @@ -909,12 +891,6 @@ - (void)deleteRenderNodesIds:(std::vector<std::shared_ptr<hippy::DomNode>> &&)no
if (!strongRootNode) {
return;
}
#if HIPPY_DEBUG
auto &nodeMap = _domNodesMap[strongRootNode->GetId()];
for (auto node : nodes) {
nodeMap[node->GetId()] = nullptr;
}
#endif
std::lock_guard<std::mutex> lock([self renderQueueLock]);
NSNumber *rootTag = @(strongRootNode->GetId());
NSDictionary *currentRegistry = [_shadowViewRegistry componentsForRootTag:rootTag];
Expand Down Expand Up @@ -1508,26 +1484,6 @@ - (void)domEventDidHandle:(const std::string &)eventName forNode:(int32_t)tag on
// no op
}

#pragma mark Debug Methods
#if HIPPY_DEBUG
- (std::shared_ptr<hippy::DomNode>)domNodeForTag:(int32_t)dom_tag onRootNode:(int32_t)root_tag {
auto find = _domNodesMap.find(root_tag);
if (_domNodesMap.end() == find) {
return nullptr;
}
auto map = find->second;
auto domFind = map.find(dom_tag);
if (map.end() == domFind) {
return nullptr;
}
return domFind->second;
}
- (std::vector<std::shared_ptr<hippy::DomNode>>)childrenForNodeTag:(int32_t)tag onRootNode:(int32_t)root_tag {
auto node = [self domNodeForTag:tag onRootNode:root_tag];
return node ? node->GetChildren() : std::vector<std::shared_ptr<hippy::DomNode>>{};
}
#endif

@end


Expand Down

0 comments on commit 21a106d

Please sign in to comment.