Hi,
We encountered this on editor crash on 5.6 and can reproduce in a simple UE5.7 project, when using the GetUserData node in a control rig, and the data retrieved is a struct asset.
- Create the struct asset
- Reference it in a data asset that can be retrieved by the control rig (see screenshot)
- Have two or more skeletal mesh components using the same skeletal mesh / control rig
- Launch the editor with `-game`, or run an automation test on the level, the control rig registry needs to register the user-defined struct to crash
The core issue is that GetUserData calls Registry.GetTypeIndexFromCPPType which internally can call RefreshEngineTypes_NoLock, 100% for user-defined structs.
Instead replacing with this fixes the crash on our end (see snippet)
Thanks,
Hugo
void FRigDispatch_GetUserData::Execute(FRigVMExtendedExecuteContext& InContext, FRigVMMemoryHandleArray Handles, FRigVMPredicateBranchArray RigVMBranches)
{
...
if(const UNameSpacedUserData* UserDataObject = ControlRigContext.FindUserData(NameSpace))
{
FString ErrorMessage;
if(const UNameSpacedUserData::FUserData* UserData = UserDataObject->GetUserData(Path, &ErrorMessage))
{
if(UserData->GetProperty())
{
#if WITH_EDITOR
-> //const FString CPPType = RigVMTypeUtils::GetCPPTypeFromProperty(PropertyResult);
-> //const FRigVMRegistry& Registry = FRigVMRegistry::Get();
-> //const TRigVMTypeIndex ResultTypeIndex = Registry.GetTypeIndexFromCPPType(CPPType);
-> //const TRigVMTypeIndex UserDataTypeIndex = Registry.GetTypeIndexFromCPPType(UserData->GetCPPType().ToString());
-> const FString CPPType = RigVMTypeUtils::GetCPPTypeFromProperty(PropertyResult);
-> const FRigVMRegistry& Registry = FRigVMRegistry::Get();
-> FName ResultDataTypeName;
-> UObject* ResultTypeObject = nullptr;
-> RigVMPropertyUtils::GetTypeFromProperty(PropertyResult, ResultDataTypeName, ResultTypeObject);
-> const TRigVMTypeIndex ResultTypeIndex = Registry.GetTypeIndex(ResultDataTypeName, ResultTypeObject);
-> FName UserDataTypeName;
-> UObject* UserDataTypeObject = nullptr;
-> RigVMPropertyUtils::GetTypeFromProperty(UserData->GetProperty(), UserDataTypeName, UserDataTypeObject);
-> const TRigVMTypeIndex UserDataTypeIndex = Registry.GetTypeIndex(UserDataTypeName, UserDataTypeObject);
if(Registry.CanMatchTypes(ResultTypeIndex, UserDataTypeIndex, true))
#endif
{
URigVMMemoryStorage::CopyProperty(PropertyResult, Result, UserData->GetProperty(), UserData->GetMemory());
bFound = true;
return;
}
#if WITH_EDITOR
static constexpr TCHAR Format[] = TEXT("User data of type '%s' not compatible with pin of type '%s'.");
ControlRigContext.Logf(EMessageSeverity::Info, Format, *UserData->GetCPPType().ToString(), *CPPType);
#endif
}
}
...
}
...
}
[Attachment Removed]
Steps to Reproduce[Attachment Removed]
Hi, I’ve been attempting to get a repro of this issue in 5.7 but I’ve been struggling so far which has slowed down the investigation. When I run a similar setup to the one you described, I always see the user defined struct type being added to the RigVM registry on startup, even when running in -game mode, so the entry already exists by the time we call GetTypeIndexFromCPPType from FRigDispatch_GetUserData::Execute.
I did have one thing for you to test, which is a change that I made in this code for the 5.8 release. The CL is 51654502. It changes the second call to GetTypeIndexFromCPPType which appears to be the one that’s causing you problems. But the reason for the change was due to invalid string formating which was then failing in the registry code, so quite different to the root cause that you’re seeing.
If that doesn’t help, would you be able to supply a cut-down repro project that demonstrates the issue? That would make it much easier to track down the cause and validate the changes you mentioned.
[Attachment Removed]
Hi Hugo,
Thanks for attaching the repro project, with that I’ve been able to get to the bottom of what’s causing the crash. Your change to use GetTypeFromProperty is good, I’m going to implement a change based on that. The code is as follows (pretty much the same as you had just slightly refactored):
if(const FProperty* UserDataProperty = UserData->GetProperty())
{
#if WITH_EDITOR
const FRigVMRegistry& Registry = FRigVMRegistry::Get();
FName ResultTypeName, UserDataTypeName;
UObject* ResultTypeObject = nullptr, *UserDataTypeObject = nullptr;
// resolve by type name and object rather than by CPP type string: user defined structs are
// registered under a generated unique name, which the property's own CPP type does not match.
RigVMPropertyUtils::GetTypeFromProperty(PropertyResult, ResultTypeName, ResultTypeObject);
RigVMPropertyUtils::GetTypeFromProperty(UserDataProperty, UserDataTypeName, UserDataTypeObject);
if(Registry.CanMatchTypes(Registry.GetTypeIndex(ResultTypeName, ResultTypeObject), Registry.GetTypeIndex(UserDataTypeName, UserDataTypeObject), true))
#endif
{
bFound = URigVMMemoryStorage::CopyProperty(PropertyResult, Result, UserDataProperty, UserData->GetMemory());
if(bFound)
{
return;
}
}
#if WITH_EDITOR
static constexpr TCHAR Format[] = TEXT("User data of type '%s' not compatible with pin of type '%s'.");
ControlRigContext.Logf(EMessageSeverity::Info, Format, *UserData->GetCPPType().ToString(), *RigVMTypeUtils::GetCPPTypeFromProperty(PropertyResult));
#endif
}
The reason that we were crashing however, is a bit more problematic. It’s a thread safety issue with GetCPPTypeFromProperty. That calls through to FRigVMRegistry_RWLock::GetTypeIndexFromCPPType which takes a read lock. But GetTypeIndexFromCPPType can end up going down a codepath that triggers regeneration of the registry (caused in your repro case because the user defined struct isn’t found in the existing registry). So, because we are writing to the registry while other threads can be attempting to read from it, we randomly hit crashes.
We’ll need to fix that issue separately from the changes to FRigDispatch_GetUserData::Execute. We could just make FRigVMRegistry_RWLock::GetTypeIndexFromCPPType take a write lock (that prevents the crash) but it’s a bit of a hammer to fix the issue. In practice, I don’t think any other threaded code is calling that function apart from the GetUserData node so in the short term we should be okay.
Thanks again for raising this, let me know if you want to discuss things any further.
Thanks,
Euan
[Attachment Removed]
Hi Euan,
You can find a repro project where I am using `release` branch on github, at the moment UE5.8.1 (when starting with Lvl_FirstPerson -game in DevelopmentEditor_Win64).
Thanks,
Hugo
[Attachment Removed]