Why does the engine crash when adding to an array ?

[KVogler]

Hi,

I have a little problem with my project.
Im getting the following creash reporter error message:

Assertion failed: (Index >= 0) & (Index < ArrayNum) [File:c:\program files\unreal engine\4.14\engine\source\runtime\core\public\Containers/Array.h] [Line: 634]
Array index out of bounds: 0 from an array of size -572662307

Since its part of the Epic code Im not really sure what is going on. Especially since the error does accur, but not always at the same condition.
I add items to an array and sooner or later the engine will crash. Sometimes when the array has 120 items, sometimes 128, sometimes 140.
The crash is absolute certain if one keeps adding items (my record is 150 before the crash).

I have put the project online. Here is the download link.
(just keep hitting the “M” key during PIE to see it crash…)

http://www.filehosting.org/file/details/633275/Project.zip

I guess that makes more sense than flooding this post with pages and pages of source code and BP screenshots…

Any help is appreciated

Cheers,
Klaus

[BrUnO_XaVIeR]

It’s because somewhere in your code you try to access an array item which pointer have been invalidated while you change that same array…
We don’t notice it on .NET languages, but C++ is apparently filled with scenarios like this…
Sometimes, when I know my code is accessing the array somewhere, I make a copy of the array (like .NET does) instead of modifying the array directly. Then after iteration is done I finally replace original array all at once.

[KVogler]

It’s because somewhere in your code you try to access an array item which pointer have been invalidated while you change that same array…

But adding items to an array should not invalidate existing items …

I make a copy of the array (like .NET does) instead of modifying the array directly.

I do that in c++ code. I know the BP system has its shortcommings in regards to arrays, but I was told in C++ they behave pretty much as expected…
Well apearaently not…

And why can I add 100+ items without any problems, but then desaster strikes at random points.

[BrUnO_XaVIeR]

The docs says that invalidation may happen, not that it never happens or always happens…
From the crashes I’ve got, it was always because I had a pointer to something in the array, but once I called Add() that pointer wasn’t valid anymore; there is when I will copy array values, and point to that copy instead, before adding anything to the real mvp.

[KVogler]

Im my case, Im adding something to an array and then never reference it again. Its just filling the array. I dont access any of the array items.

[hard_code]

How are you declaring your TArray and whats the code you are calling when adding an item? Are you using the default allocator?

The fact that size aka ArrayNum = -572662307 in that error message is odd as that variable keeps tract of the total items in the array.

[KVogler]

How are you declaring your TArray and whats the code you are calling when adding an item? Are you using the default allocator?

its declared as

TArray<UStaffMember*>   StaffList;

The code for Adding is essentially

UStaffMember* NewStaff;

NewStaff = NewObject<UStaffMember>();

StaffList.Add(NewStaff);

The full source code is in the project files (link in first post)

[hard_code]

@KVogler;650885

its declared as

TArray<UStaffMember*>   StaffList;

The code for Adding is essentially

UStaffMember* NewStaff;

NewStaff = NewObject<UStaffMember>();

StaffList.Add(NewStaff);

The full source code is in the project files (link in first post)

Try marking the tarray as a UPROPERTY?

UPROPERTY()
TArray<UStaffMember*>   StaffList;

Thats my guess. Whenever I see issues like this it has to do with garbage collection. Also make sure the actor or whatever that the tarray is declared in is not being garbage collected as well.

[KVogler]

Try marking the tarray as a UPROPERTY?

Ill try that first thing tomorrow
Is it important if the array is a private member? In terms of OOP I provide public getter and setter (which are UFUNCTION’ed).

Also make sure the actor or whatever that the tarray is declared in is not being garbage collected as well.

The owning actor is a GameState instance, referenced in the selected game mode. So GB should leave it alone during PIE… (?)

[Jambax]

I’m suspicious about having to put my e-mail address into a file hosting website btw - Google Drive is a much better alternative Can you post the code you’ve written here so we don’t need to DL?

Anyway, looking at the OP - it looks like you’re trying to access an Array element that doesn’t exist. E.g. calling MyArray[0] on an array with no elements.

[hard_code]

@KVogler;650906

Ill try that first thing tomorrow
Is it important if the array is a private member? In terms of OOP I provide public getter and setter (which are UFUNCTION’ed).

The owning actor is a GameState instance, referenced in the selected game mode. So GB should leave it alone during PIE… (?)

Ya the GameState is safe from being garbage collected.

Its fine to have private but on your getter you need to make sure you are not returing a new copy of the array otherwise the new copy could be garbage collected if its not set to another uproperty variable.

[KVogler]

on your getter you need to make sure you are not returing a new copy of the array otherwise the new copy could be garbage collected

The getter declaration looks like this:

TArray<UStaffMember*>& GetStaffList() { return StaffList; }

[hard_code]

@KVogler;651052

The getter declaration looks like this:

TArray<UStaffMember*>& GetStaffList() { return StaffList; }

Should be good.

[KVogler]

Should be good.

Think so too.

What strikes me most is that it seems to be a race condition of some sort as I get the crash at different points during PIE, or the GB interfering afterall…

[KVogler]

I’m suspicious about having to put my e-mail address into a file hosting website btw - Google Drive is a much better alternative

Yeah. Have uploaded it to google drive. Its a different version too. I added some comments and polished some others.
Please check the attached readme for crash repro steps.
PS: Just discovered a third scenario: Just add some staff members and do nothing. Crash will occur after a minute or so.
All three crashes produce different error messages.

Note that the code design is still very crude and highly WIP, but before I do some refactoring and reorganization, I’d like to get this issue understood/fixed…

https://drive.google.com/file/d/0B8K5b8-u_ENTMERScU1IbHowUGs/view?usp=sharing

Can you post the code you’ve written here so we don’t need to DL?

That would mean the full source code of about a dozen classes and approx 30 to 40 blueprint screenshots of all functions and bindings.
Im not sure that would make much sense.

[gmpreussner]

@KVogler;650699

I have a little problem with my project.

Hi Klaus,

TArray works fine. The problem is with the life time of your UObjects, such as UStaffUnit, UStaffMember, etc.

What you need to know is that UObjects in UE4 are special in that they are garbage collected objects. The engine’s garbage collector will regularly sweep all UObjects in memory and determine whether or not anything still references those objects. If nothing references an object, the garbage collector will destroy it. This system frees you from the burden of managing the life time of your game objects yourself, which could get really quite complicated for any non-trivial game logic.

Now, with great power also comes great responsibility. In particular, it is your responsibility to tell the garbage collector when a UObject is still being referenced. In UE4 we handle this via so called UProperties. Those are class member fields in your class declaration that are marked up with the UPROPERTY() macro. This macro will be detected by Unreal Header Tool (UHT) when you compile your code, and cause it to generate the hidden magic glue that is needed by the garbage collector to become aware of the fact that your object is referencing some other UObject. Note that in C# and Java this all happens automatically, because it is a feature built into the language. C++ does not have these capabilities out of the box, which is why we need to use these kind of tricks instead.

So, basically, what you are missing is the UPROPERTY() markup in all the places where you keep pointers to UObjects. For example, in UStaffManager, the following would be the correct code:

[FONT=Courier New]UPROPERTY()
UStaffGenerator* Generator; // Factory class for creating UStaffMNember instances

UPROPERTY()
UStaffUnitList* StaffUnits; // Array with all UStaffUnit instances

UPROPERTY()
UExitUnit* ExitUnit; // Special Exit unit ith extended behaviour

UPROPERTY()
UStaffUnit* AllStaff; // Array with a duplicate pointer to each UStaffMember instance referenced by …

With these changes, the garbage collector will be able to discover that UStaffManager may hold references to UStaffGenerator, UStaffUnit, and other UObject derived instances. You have to do this whenever you wish to hold on to UObject instances beyond the scope of the current function call. In local function calls it is OK to simply reference a pointer to a UObject derived class.

If you don’t mark up your pointers with UPROPERTY, what happens is that the garbage collector thinks that nothing in your game is still referencing your newly created instances of UStaffUnit etc. It then correctly frees the memory of those objects. This can happen a short time after you created them. Once the object is freed, the memory associated with it becomes invalid and may be reused by other objects. This is why your TArray ends up having ‘random’ data in it, and why you’re seeing an odd array size of -572662307. The memory of the TArray is basically all garbage.

---

Bonus knowledge: If you need to hold on to a UObject within a non-UObject class (i.e. a regular C++ class), then you cannot use the UPROPERTY() markup. Instead you use the TWeakObjectPtr template to store your pointers. Note that this is a weak pointer, meaning that it does not take ownership of the life time of your object. However, this pointer will be automatically reset to nullptr once the garbage collector destroys the referenced UObject. Before dereferencing a weak object pointer you must therefore check whether it is actually still valid.

---

I hope this clears everything up.

In general, the rule of thumb is this: If you seemingly run into an issue with basic core types in UE4 then the problem is your code, not UE4. The core types have evolved over 25 years and stood the test of time. While it is not impossible, it is extremely unlikely that they are broken. Also, if they were broken, we would detect it pretty much immediately, because they are used in innumerable ways in the Engine.

---

Here are some useful documentation links:

Garbage Collection Overview
Unreal Object Handling
UNREAL PROPERTY SYSTEM (REFLECTION)
Properties
UPROPERTY

[rubenst]

Cool, thanks for the very detailed answer Max!

Initially we had the exact opposite in mind, that if we left off the UPORPERTY() makros it would not-opt-in to garbage collection.
Seems we have been mistaken on this one. hehe

We will try your solution asap and let you know how it goes.

[KVogler]

@gmpreussner

Thanks for the quick reply

In particular, it is your responsibility to tell the garbage collector when a UObject is still being referenced. In UE4 we handle this via so called UProperties. Those are class member fields in your class declaration that are marked up with the UPROPERTY() macro.

Ah ok My line of thought was: UPROPERTY() introduces the member to the garbage collection system. Ergo, if I dont use UPROPERTY(), the garbage collection would ignore those members and never collect them.
Like they would be completely under the radar.
Appearently I was wrong

In general, the rule of thumb is this: If you seemingly run into an issue with basic core types in UE4 then the problem is your code, not UE4.

That was my assumption all along
It somehow seems unlikely that you guys could create something like Paragon/Fortnite, etc. without properly working array containers

I’ll update my code and see how it goes.
Thanks again

Cheers,
Klaus

[theonecalledtom]

For what it’s worth “-572662307” is not a “random” number at all (this might have been mentioned above).

	-572662307	0xdddddddd	int

Or the typical debug memory filled for deleted memory.