Skip to content

Commit

Permalink
add a separate check for this special case
Browse files Browse the repository at this point in the history
  • Loading branch information
rcancro committed Feb 6, 2025
1 parent 9c730fc commit 47e2acd
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
15 changes: 10 additions & 5 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
ASDisplayNodeFailAssert(@"Cannot add a view-backed node as a subnode of a layer-backed node. Supernode: %@, subnode: %@", self, subnode);
return;
}

BOOL isRasterized = subtreeIsRasterized(self);
if (isRasterized && subnode.nodeLoaded) {
ASDisplayNodeFailAssert(@"Cannot add loaded node %@ to rasterized subtree of node %@", ASObjectDescriptionMakeTiny(subnode), ASObjectDescriptionMakeTiny(self));
Expand All @@ -2114,13 +2114,18 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
__instanceLock__.lock();
NSUInteger subnodesCount = _subnodes.count;
__instanceLock__.unlock();

// If the subnodeIndex is out of bounds OR it will be once we call [subnode removeFromSupernode], exit early
if (subnodeIndex > subnodesCount || subnodeIndex < 0 || (subnode.supernode == self && subnodeIndex >= subnodesCount)) {

if (subnodeIndex > subnodesCount || subnodeIndex < 0) {
ASDisplayNodeFailAssert(@"Cannot insert a subnode at index %ld. Count is %ld", (long)subnodeIndex, (long)subnodesCount);
return;
}


// Check if subnode is already a in _subnodes. If so make sure the subnodeIndex will not be out of bounds once we call [subnode removeFromSupernode]
if (subnode.supernode == self && subnodeIndex >= subnodesCount) {
ASDisplayNodeFailAssert(@"node %@ is already a subnode of %@. index %ld will be out of bounds once we call [subnode removeFromSupernode]. This can be caused by using automaticallyManagesSubnodes while also calling addSubnode explicitly.", subnode, self, subnodeIndex);
return;
}

// Disable appearance methods during move between supernodes, but make sure we restore their state after we do our thing
ASDisplayNode *oldParent = subnode.supernode;
BOOL disableNotifications = shouldDisableNotificationsForMovingBetweenParents(oldParent, self);
Expand Down
2 changes: 1 addition & 1 deletion Tests/ASDisplayNodeTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2789,7 +2789,7 @@ - (void)testInsertExistingSubnode
DeclareNodeNamed(child);

[parent addSubnode:child];
// Previously this would cause a crash. We now protect inserting an already existing subnode
// Previously this would cause a crash. We now protect inserting an already existing subnode out of bounds
XCTAssertThrows([parent insertSubnode:child atIndex:1]);
XCTAssertTrue(parent.subnodes.count == 1);
}
Expand Down

0 comments on commit 47e2acd

Please sign in to comment.