[BUG] Can't remove element from array of pointers (C++)

HI, I have following C++ code:

// file.h, Elements are inherited from UObject
TArray<UWeatherTargetsKDTree*> ChildHeap;

// file.cpp, inside function
auto f = ChildHeap.FindByPredicate([obj](UWeatherTargetsKDTree* tree) { return tree->ContainingObject == obj; });

if (f) {
		ChildHeap.Remove(*f);
}

I suppose this code should work, but it doesn’t. Lets look at implementation:

// inside Array.h, 
function int32 Remove(const ElementType& Item) {
	CheckAddress(&Item);
       //  rest
}

// source for CheckAddress:

FORCEINLINE void CheckAddress(const ElementType* Addr) const
	{
		checkf(Addr < GetData() || Addr >= (GetData() + ArrayMax), TEXT("Attempting to add a container element (%p) which already comes from the container (%p, ArrayMax: %d, ArrayNum: %d, SizeofElement: %d)!"), Addr, GetData(), ArrayMax, ArrayNum, sizeof(ElementType));
	}

So, I try to remove element, which is in container, but I can’t, because it is in container.

So, am I doing something wrong, or it is a bug?

Hello ,

It seems that the Remove is never being called due to you calling “return” prior to it. The return is exiting out of the function and nothing after it is being called.

Hi Matthew,
I can’t agree. I debugged it step by step and checked the content of variables. In ChildHeap variable, there was single element. Since I got exact pointer as a parameter to my function, variable f was valid and pointed to that single element in ChildHeap. Then execution proceeded to Array::Remove(…)
I checked content of parameter Item and it was again that element. So method CheckAddress was called. But if you read that error:

Attempting to add a container element (%p) which already comes from the container

you will see, that it is failing because it is in container (which is good that it is, but it throws error thus you are not able to remove item from container). And from that message you can see, that checking was intended to Adding, but not removing - in latter case the condition should be negated)

Ah, excuse me. I misread and didn’t notice that Return is for FindByPredicate, not the function itself. I’ll continue looking into this.

Is there any reason that you’re not using ChildHead.Contains or Find instead of FindByPredicate? I’m not seeing a reason for it in this case. If you do use these methods, does it work correctly?

Using these methods are not possible in my case. It would be too long article to precisely describe here relations between my gameobjects and reasons why it is implemented this way. But I’ll try to shortify it:

I have Actor, which has info about some blocks in game world (list of UWeatherTargetsKDTree*). This items are leafs in K-D tree structure. When this actor is about to be deleted from game world (by player action), it must not leave this globally used tree in invalid state. Since the tree will be pretty huge, I implemented algorithm to “clean it up” from bottom.

Each leaf of that global tree has this ChildHeap, which is of type defined in my question. I should mention that each UWeatherTargetsKDTree object has reference to UObject (which is in my case that about to be deleted block) (**ContainingObject **).

So I need to examine not a reference to element in heap, but a ContainingObject reference in element in heap. Thus my predicate.

Don’t worry, I solved this by using IndexOfByPredicate and then remove element from heap by using HeapRemoveAt with resulted index and provided sorting predicate.

Regardles of my reasons, this code is clearly a bug, because function CheckAddress must check address (to avoid memory exceptions), but when deleting item from array throw exception only when the element is not in source array.

** EDIT ** If you’d like to, you can see full source code here: .com/halbich/TauCetiF2/blob/ce18221f9e339781ad0656df05a2581c744df99f/Source/Blocks/Private/Tree/WeatherTargetsKDTree.cpp , method for deleting starts at line 249

Thank you for explaining. Even though you do say that this is clearly a bug, I don’t believe that it is. CheckAddress is asserting due to you using the exact same address that is already in the container. If you were to store the value at another address and use this same function, you wouldn’t have any issues. As you mentioned, the proper way to do this would be to get the index and then RemoveAt.

The reason you wouldn’t want to do this, and the reason for the check, is that the Remove function makes multiple removals and the address could become invalid after shuffling after the first removal. This could result in either extra removals that were not intended or not removing all of the intended elements.

The only bug I see here is the assert message itself saying that there is an issue inserting. Since CheckAddress is used in both Insert and Remove, this doesn’t make sense.

1 Like