Overly aggressive Garbage Collection bug

Development Programming & Scripting C++
, , , , , ,
1

Hey all,

Wanted to report this odd exception that showed up.

The exception was thrown by the garbage collection code in ConditionalBeginDestroy() when I attempt to delete an actor (in the level editor, not through code) that I allocated a dynamic object in and manually freed.

The odd thing is when this class wasn’t inheriting from a base class I had defined and instead directly from AActor, this bug did not happen.

The exception was thrown by an IsValidLowLevel() check so I’m assuming it is because the Garbage Collector was trying to free already freed memory (i.e. the object I dynamically allocated and then freed).

If I do not attempt to manually free the object, no exception is thrown.

Code is provided below.

void AProceduralTerrainActor::OnConstruction(const FTransform& Transform)
{
	UE_LOG(LogTemp, Warning, TEXT("In OnConstruction!"));
	Super::OnConstruction(Transform);

	if (HeightmapTexture != NULL)
	{
		// Needed for first time construction
		if (!bIsHeightMapBuilt)
		{
			UE_LOG(LogTemp, Warning, TEXT("Constructing Heightmap!"));
			// Create heightmap builder and set reference
			UHeightmapBuilder* builder = NewObject<UHeightmapBuilder>();
			builder->SetTerrainReference(*this);
			builder->CreateHeightmapFromActorReference();
			bIsHeightMapBuilt = true;

			// Deallocate builder object

            //FMemory::Free(builder);   <----- If this is uncommented, throws an exception in ConditionalBeginDestroy()
		}
		GenerateMeshFromHeightmap();
	}
	else
	{
		GenerateDefaultMesh();
	}
}
2

Hi Nightmask3,

After looking at the code you provided, it does appear that your use of FMemory::Free(builder) is what is causing the problem. This interferes with the Engine’s garbage collection which attempts to free the memory reserved for the UObject only to find that the memory has already been released.

Manually freeing a UObject is generally a bad idea. When the Engine’s garbage collection kicks in, it will take care of cleaning up the UObject once there are no longer any references to the UObject. If there is a pressing need to clean up the UObject immediately, you can call GetWorld()->ForceGarbageCollection(true);, which will result in garbage collection running at the next available opportunity.

Another option to examine would be whether builder could be an AActor class instead of a UObject class. As an AActor, it could be destroyed once you are done with it (and garbage collection would take care of cleaning it up). I am not sure if this would be possible in your particular use case.

3

Hey ,

Thanks for the reply!

‘builder’ is indeed an instance of a UObject based class, and I chose it to be so as there is never a need for it to exist in-world.

However this problem was easily fixed by simply letting the garbage collector do it’s thing.

I am just accustomed to the “good C++ practice” of freeing up memory as soon as it is no longer needed and the lack of the need to do so is what threw me off.