Be Vewwwy Vewwwy Quiet, We’re Hunting UObjects
May 23, 2018
We were digging into user crash reports for a client project that were coming out of some cinematic movie sequence player code. There were quite a number of instances but the dumps weren’t telling us much beyond the fact there was a segfault related to the Sound->GetMaxAudibleDistance() call shown in the excerpt below.
void FAudioDevice::GetMaxDistanceAndFocusFactor(USoundBase* Sound, const UWorld* World, const FVector& Location, const FSoundAttenuationSettings* AttenuationSettingsToApply, float& OutMaxDistance, float& OutFocusFactor)
{
...
if (bHasAttenuationSettings)
{
...
}
else
{
// No need to scale the distance by focus factor since we're not using any attenuation settings
OutMaxDistance = Sound->GetMaxAudibleDistance();
OutFocusFactor = 1.0f;
}
} We started trawling through the crash dumps looking for anything that might lead to a repro case, initially without much luck. Fortunately a dump with a user comment of “this happened twice in a row” appeared, so perhaps it was location dependent? Teleporting to the position from the dump and moving around a little turned out to give us a 100% repro. Excellent! <insert Wyld Stallyns riff>
So the actual crash was in the -> operator, Sound was not null, but it wasn’t pointing to a valid UObject. The vtable was junk and trying to call the virtual GetMaxAudibleDistance() function was causing our crash.
OK, we’d got a bad UObject, it had most likely been garbage collected out from under us. So then began the next quest, what ultimately owned the Sound UObject*, and why wasn’t that owner still holding a reference to it.
And what a quest it was… I’m not going to walk through the movie player code because, politely put, it is somewhat convoluted. To cut that particular long story short, after working our way down, the problem Sound object was revealed as a UPROPERTY tagged USoundBase*. It turned out to be in an instance of FMovieSceneAudioSectionTemplate. This type is derived from FMovieSceneEvalTemplate and stored within a custom pointer type based on TInlineValue. *Deep breath* The pointer type implemented a custom Serialize() method, in order to allow it to handle read / write operations on UObjects, but not a corresponding AddReferencedObjects() method which would allow their detection by the garbage collector. Or, more correctly, would allow the GC to determine that this custom pointer type was holding a reference to its members and any UObjects they might contain. The result was that any member UObjects of FMovieSceneAudioSectionTemplate, or any of its derivatives, (of which there are about 30 in UE 4.15) were not going to be ref counted by the garbage collector. They were thus liable to be deleted at any given time, which was ultimately the cause of our crash.
In order to fix this we needed to implement that missing method and make sure all our UObjects were hunted down and accounted for.
The function declaration must be added to the class body, along with the override of the TStructOpsTypeTraits WithAddStructReferencedObjects enum value. Setting WithAddStructReferencedObjects == true indicates to the garbage collector that it should expect to call the AddStructReferencedObjects() method in this class.
USTRUCT()
struct FMovieSceneEvalTemplatePtr
#if CPP
: TInlineValue<FMovieSceneEvalTemplate, 48>
#endif
{
GENERATED_BODY()
...
MOVIESCENE_API void AddStructReferencedObjects(FReferenceCollector& ReferenceCollector) const;
};
template<> struct TStructOpsTypeTraits<FMovieSceneEvalTemplatePtr> : public TStructOpsTypeTraitsBase
{
enum { WithSerializer = true, WithCopy = true/*CL*/, WithAddStructReferencedObjects = true /*end CL*/};
}; The implementation then follows a common pattern. The class custom serialization method is called using the archive belonging to the GC’s reference collector.
void FMovieSceneEvalTemplatePtr::AddStructReferencedObjects(FReferenceCollector& ReferenceCollector) const
{
// Serialize UObj refs into the ref collector archive
SerializeEvaluationTemplate(*const_cast<FMovieSceneEvalTemplatePtr*>(this), ReferenceCollector.GetVerySlowReferenceCollectorArchive());
} Finally the serialization method (SerializeEvaluationTemplate()) needs to be modified to account for this usage (i.e. a call when the intention is neither to load nor save).
/** Serialize the template */
template<typename T, uint8 N>
bool SerializeEvaluationTemplate(TInlineValue<T, N>& Impl, FArchive& Ar)
{
Ar.UsingCustomVersion(FMovieSceneEvaluationCustomVersion::GUID);
if (Ar.IsLoading())
{
...
return true;
}
else if (Ar.IsSaving())
{
...
return true;
}
else // If we're neither loading nor saving then the call is from the reference collector
{
if (Impl.IsValid())
{
auto* ImplPtr = &Impl.GetValue();
UScriptStruct& Struct = ImplPtr->GetScriptStruct();
Struct.SerializeItem(Ar, ImplPtr, nullptr);
}
return true;
}
return false;
} With those changes, everything should be fixed right? Nope! Unfortunately we couldn’t get this code to be called at all. We double checked the implementation, and it was sound. So it looked like there was a further piece in the puzzle that was this bug.
Fortunately our resident garbage collection expert (I bet he’s always wanted to be known as that!) knew where to look almost immediately. The UStructProperty::ContainsObjectReference() method (in GarbageCollection.cpp) does what it says on the tin and checks if a USTRUCT contains any UObject references. The thing is, there was nothing in that method that would account for the USTRUCT types we were concerned with, those which implement custom GC and serialization behaviour. Rather worryingly this means a fair number of other systems were also not being correctly ref counted (but on the plus side we might just be about to fix a large number of GC related crashes!).
There is actually a relatively simple way of handling this though. It is possible to directly query the metadata associated with a USTRUCT and determine which (if any) TStructOpsTypeTraits are implemented for the type in question. In this case querying GetCppStructOps()->HasAddStructReferencedObjects() will tell us whether our USTRUCT implements the AddStructReferencedObjects() method. Logically if this method is implemented then we must have UObjects which need ref counting (and therefore ContainsObjectReference() should return true). Note that we don’t need to use the TArray parameter in this change as we are not inspecting properties individually. So we do not need to track which properties have already been queried (which is what the EncounteredStructProperties array is used for).
bool UStructProperty::ContainsObjectReference(TArray<const UStructProperty*>& EncounteredStructProps) const
{
if (EncounteredStructProps.Contains(this))
{
return false;
}
else
{
if (!Struct)
{
UE_LOG(LogGarbage, Warning, TEXT("Broken UStructProperty does not have a UStruct: %s"), *GetFullName() );
}
else
{
// This allows us to check for UObjects contained in USTRUCTs which have previously (and erroneously) not been ref counted.
if (Struct->GetCppStructOps() && Struct->GetCppStructOps()->HasAddStructReferencedObjects())
{
return true;
}
...
}
return false;
}
} While this solves the particular problem we were dealing with it is probable that the UStructProperty::ContainsWeakObjectReference() method (also in GarbageCollection.cpp) needs a similar addition to handle detecting objects with custom GC behaviour. However, we have not picked up any crashes or other issues here and so have not had the opportunity to investigate this function.
But with that, at least our current wascally wabbits UObjects are accounted for. So th-th-th-that’s all folks!
Credit(s): Josef Gluyas, Gareth Martin (Coconut Lizard)
Status: Currently unfixed in UE4.19
Github Pull: github.com/EpicGames/UnrealEngine/pull/4752