FIntegralCurve MyCurve;
FKeyHandle HandleTo2000 = MyCurve.AddKey(2000, 2000);
bool AreKeysDifferent;
do
{
FKeyHandle Handle = MyCurve.FindKeyBeforeOrAt(1337);
AreKeysDifferent = Handle != HandleTo2000;
}
while (AreKeysDifferent);
UE_LOG(LogTemp, Error, TEXT("This should never happen, but eventually does"));
It will eventually hit a `check` in `FKeyHandle()`'s constructor because the counter loops around:
FKeyHandle::FKeyHandle()
{
static std::atomic<uint32> LastKeyHandleIndex = 1;
Index = ++LastKeyHandleIndex;
if (Index == 0)
{
// If we are cooking, allow wrap-around
if (IsRunningCookCommandlet())
{
// Skip indices until it's not 0 anymore as we can't
// assign without loss of thread-safety.
while (Index == 0)
{
Index = ++LastKeyHandleIndex;
}
}
else
{
check(Index != 0); // check in the unlikely event that this overflows
}
}
}
If we continue past the assert, or use a non development build, it’ll wrap around causing the test code above to eventually find that 2 entries are sharing the same KeyHandle even though they aren’t the same at all.
It could be that the code in FIntegralCurve didn’t anticipate that there’s a finite number of FKeyHandle that could ever be created:
FKeyHandle FIntegralCurve::FindKeyBeforeOrAt(float KeyTime) const
{
// If there are no keys or the time is before the first key return an invalid handle.
if (Keys.Num() == 0 || KeyTime < Keys[0].Time)
{
return FKeyHandle();
}
It seems however that the KeyHandle concept is flawed there and should instead use some kind of sentinel value / TOptional since a wrap around could have catastrophic consequences.
In our game, it happens in the Editor and we hit the assert, but there’s nothing preventing it from happening during gameplay if we wait long enough.
Sorry about the delay, and thank you for reporting this issue and including this very helpful repro function. I was able to reproduce the problem here, and I noticed that several engine functions return a default-constructed FKeyHandle to represent an invalid handle. Some examples below:
Interestingly, a special static function FKeyHandle::Invalid() exists, which returns a handle with invalid internal index 0 without incrementing the static atomic counter. A comment in file “KeyHandle.h” mentions that this is to “avoid allocating new handles unnecessarily”, and this special value is used appropriately on several other places in the codebase. The Find() and Get() functions I mentioned above (and maybe more) were probably overlooked.
I just filed an internal bug report about this issue so that the engine devs can take a closer look. Here’s the tracking number for it: UE-333138. It should become available once the devs mark it as public.