diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 9771c50ca..795ec8f90 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -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)); @@ -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); diff --git a/Tests/ASDisplayNodeTests.mm b/Tests/ASDisplayNodeTests.mm index 069da542b..29d98d36e 100644 --- a/Tests/ASDisplayNodeTests.mm +++ b/Tests/ASDisplayNodeTests.mm @@ -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); }