Segmentation fault while looping through actors and changing materials in C++

Hey folks,

I’m new to UE5 and I’m having some troubles here. Since I’ve been debugging this problem for several days (without any step further) and I still don’t understand what I’m doing wrong, the time has come to ask you.

I’m basically trying to make a cube blink in the standard 1st person template game in standalone mode. What I do is substantially the following (cutting hopefully useless stuff to keep things clearer):

//# Called by AReplaysCharacter::BeginPlay()
void UReplayGameInstance::BeginPlay()
{
    PreloadMaterials(RegexList, MaterialList);
    auto Actor = FindActor("SM_ChamferCube4", true);
    auto ActorLabel = UKismetSystemLibrary::GetDisplayName(Actor);
    auto SMC = GetStaticMeshComponent(Actor);
    //# oldchamfermat and newchamfermat are class members with type UMaterialInterface *
    oldchamfermat = SMC->GetMaterial(0);    
    newchamfermat = Cast<UMaterialInterface>(LoadedMaterials[0].material);
}

LoadedMaterials is just a TArray<LoadedMaterial> (a struct that matches Actors’ regexes and materials) where I load my materials in this way:

auto material = LoadMaterialInstanceDynamicFromPath(FName(*sPath));

Then, in Tick() I try to switch materials:

void UReplayGameInstance::Tick(float DeltaTime)
{
    tick_counter += 1;
    if (tick_counter % 50 == 0)
    {
        auto Actor = FindActor("SM_ChamferCube4", true);
        auto SMC = GetStaticMeshComponent(Actor);
        if ((tick_counter/10) % 2 == 0)
        {
            SMC->SetMaterial(0, newchamfermat);
        }
        else
        {
            SMC->SetMaterial(0, oldchamfermat);
        }
    }
}

FindActor() is just this:

AActor * UReplayGameInstance::FindActor(const FString & name, bool strict)
{
        TArray<AActor*> AllActors;
        UGameplayStatics::GetAllActorsOfClass(GetWorld(), AActor::StaticClass(), AllActors);

        for (AActor* Actor : AllActors)
        {
          auto ActorLabel = UKismetSystemLibrary::GetDisplayName(Actor);
          if (!strict and ActorLabel.Contains(name))
          {
            return Actor;
          }
          else if (ActorLabel == name)
          {
            return Actor;
          }
        }
        return nullptr;
}

And GetStaticMeshComponent() only does this:

UStaticMeshComponent * UReplayGameInstance::GetStaticMeshComponent(AActor *Actor)
{
    if (Actor)
    {
        auto ActorLabel = UKismetSystemLibrary::GetDisplayName(Actor);
        if (AStaticMeshActor* StaticMeshActor = Cast<AStaticMeshActor>(Actor))
        {
            UStaticMeshComponent* StaticMeshComponent = StaticMeshActor->GetStaticMeshComponent();
            if (StaticMeshComponent && StaticMeshComponent->GetStaticMesh())
            {
                return StaticMeshComponent;
            }
        }
    }
}

After a random amount of ticks, sometimes just a thousand, sometimes tens of thousands, i get one of these SIGSEGVs:

Unhandled Exception: SIGSEGV: unaligned memory access (SIMD vectors?)
Unhandled Exception: SIGSEGV: invalid attempt to read memory at address 0x0000000000000000

I’ve already tried printing most of the pointers, and none of them are ever null.

What puzzles me the most is that all the pointers, even those of my dynamic materials loaded at the beginning in BeginPlay() keep changing. The addresses keep moving forward, as if the objects were de-allocated and re-allocated every time.

I read that in UE5 one should always use the smart pointers that it makes available, but then I don’t understand why GetAllActorsOfClass() and many other functions return raw pointers instead of smart pointers.

I’m using UE5.2 under Ubuntu 22.04 x86_64, anyone have any idea what I’m doing wrong?

Thanks!

Nothing stands out as a smoking gun on a cursory glance. What does stand out though is the lack of checks on return values from functions, along with the heavy use of “auto”.

Right out the gate on BeginPlay with oldchamfermat and newchamfermat…there is never a check on the validity of either and you attempt to index an array without checking to make sure it actually has anything in it. I realize you might know it has something in it but from a general coding practice you shouldn’t rely on that. Subsequently your calls to FindActor and GetStaticMeshComponent never follow up with validity checks…you roll right into using them. I would start first by putting some explicit validity checks. For UObjects use IsValid and var != nullptr for non-UObjects. I would start by fixing those first and making sure you’re not attempting to access an invalid pointer or index…trust but verify…or maybe just don’t trust :slight_smile:

Also, while auto certainly makes writing things easier, it has the side-effect of making your code less readable, especially without it loaded in an IDE. Here’s a good read on the use of “auto” C++ 11 Auto: How to use and avoid abuse - A CODER’S JOURNEY (acodersjourney.com).

I did notice that your GetStaticMeshComponent function does not explicitly return anything unless everything checks out. You should return nullptr by default.

Not entirely the case. You should use it where it “makes sense”. Be careful with them as they effect the lifespan of the object…among other things Smart Pointers in Unreal Engine | Unreal Engine 5.2 Documentation. We’ll give Epic a break on not being completely inline with there suggested coding practices. They have been working this engine likely for longer than the suggested coding practices existed. There are going to be a lot of spots that don’t adhere to what they preach but can easily be explained by “if it isn’t broke, don’t fix it”. I would like to believe that anything new being written going forward adheres closely to their internal practices…then again, humans will be humans.

I would, however, argue for using TObjectPtr for UObjects TObjectPtr | Unreal Engine 5.2 Documentation

By chance are you using live coding? If so, don’t. It can cause weird issues…potentially like the one you’re seeing :slight_smile:

Oh nonono sorry! My code actually looks like this, I’ve pruned out all the checks and messages to try to make the code “more readable” (bad idea in hindsight).

I really placed double and triple checks and prints everywhere in my code, I really don’t know where to watch next.

I’ve also tried to break with gdb in the middle of SetMaterial() (at Runtime/Engine/Private/Components/MeshComponent.cpp:52), the code below always runs without any error or null pointer, it simply throws SIGSEGV on UpdatePhysicalMaterials()

I don’t know, it’s so weird, I’ll try to create another project with a very minimal working example and I’ll post it back here.

Thanks!

Ok, so it shouldn’t be a problem of raw pointers vs. smart pointers… or maybe yes, I’ve read that GetAllActorsOfClass() is slow (and I call it every 50 ticks), maybe someone/something marks my actors for deletion in the middle of it?

But I still don’t understand why this should happen, I call SetMaterial() only on one single ChamferCube (the number 4 to be precise), why should it go out of scope/lifespan/whatever and change pointer value? And why does this pointer change work a hundred times and then all of a sudden it breaks with a SIGSEGV?

EDIT: nope, no live coding, plain old vim :slight_smile:

Are your LoadedMaterials, TArray<LoadedMaterial>, the underlying struct(s), your UMaterialInterface properties, all prefixed with UPROPERTY / USTRUCT ?
If not, your loaded mats might get garbage collected at some point, resulting in crash when trying to use them after some time

1 Like

Ok, this is much better :slight_smile:

I’m still a little concerned that you’re not using IsValid on any of the UObjects. You’re checking for nullptr but that’s not all that IsValid checks…it also makes sure that it’s not marked for garbage collection. I know it doesn’t make sense that any of this would be, but I would just make certain that any UObjects I’m attempting to use pass that check. This includes your actors, components, materials, etc.

A man after my own heart. I’m a vim user in my daily life. For this though, I find the IDE especially helpful when needing to search through the UE codebase quickly…albeit find and grep are great alternatives :wink:

Also, @Chatouille had a good suggestion. I wasn’t even aware that was a possibility!

Hey @Chatouille I think you nailed it!

I chopped down my code to a minimal working example, but the segmentation fault still popped up, albeit after a few minutes instead of tens of seconds.

Instead, marking materials and functions that deal with the change of materials as UPROPERTY and UFUNCTION seems to have had the desired effect! I went to get a coffee and the executable ran for a good 15 minutes!

I’ll leave the code here for posterity, I’ve been sweating over it for days, I hope it saves a few curses for those who come later :laughing:

Thank you so much for the help, I didn’t know where to look anymore :sweat_smile:

blinking-cube.zip (38.7 KB)

Thanks @gh0stfase1, I also followed your advice and plastered my code with IsValid(), but I actually didn’t notice any red line in the log either for nullptr or for IsValid() returning false.

In any case, I made treasure of it for the future and I’ll continue to use them in all “suspicious cases”. Hoping that Epic finally starts using smart pointers everywhere to minimize cases like this…

Yes, I love vim, I love grep, and I love abusing of all kinds of regexes to search and replace complex strings. No IDE can compete with their versatility. And soon, when there will be language models that are a little smaller than now and capable of ingesting entire code bases, vim will also have really interesting auto-completion possibilities (at the moment I’m using Codeium, its hit/miss ratio isn’t bad but there’s ample room for improvement).

1 Like

We both learned something on this one! I’ve always used UPROPERTY and UFUNCTION as a matter of course. I was not aware that it could affect garbage collection :face_with_spiral_eyes:. I guess I can rationalize it with, if you’re not using UFUNCTIONS or members decorated with UPROPERTY, the engine doesn’t actually know if the UObject is being used (reflection system?). In which case, after some time, it will pull the rug on you.

Anyway, best of luck!

Using UPROPERTY is essentially smart pointers for UObject types. It exposes them to the reflection system, which is how the engine can count references, and decide to garbage collect an object which is not referenced. Also when an UObject is destroyed, all UPROPERTIES pointing to it are automatically turned into nullptr. This is only really applicable to Actors though because you can’t “destroy” an UObject yourself, and an UObject won’t be destroyed if referenced…

If you want to point to an UObject without reflection system (no UPROPERTY), you can also use F/TWeakObjectPtr. Those weak references will not forcibly keep the object alive, and will not be automatically nulled on its destruction, but it has capability to check when the pointed-to address no longer contains the original object.

Also note that when using a raw pointer without UPROPERTY, even IsValid can go false positive. There’s no truly safe way of checking raw pointer validity, so always use UPROPERTY or WeakObjectPtr.

1 Like

Understood, thanks again!