FBlueprintEditorUtils::HasFunctionBlueprintThreadSafeMetaData doesn't check MD_ThreadSafe key value

Callers of FBlueprintEditorUtils::HasFunctionBlueprintThreadSafeMetaData assume that it returns whether the given function must be thread safe: see the uses in FKismetCompilerContext::FinishCompilingFunction and FKismetCompilerContext::SetCalculatedMetaDataAndFlags, for example.

However, internally HasFunctionBlueprintThreadSafeMetaData only checks whether the MD_ThreadSafe and MD_NotThreadSafe metadata exist -- not the actual value for the metadata (i.e. “true” or “false”). This means that manually setting MD_ThreadSafe to “false” will actually cause the function to report that it wants thread safety checks.

In addition, MD_NotThreadSafe is only checked for native functions, not script functions, meaning specifying MD_NotThreadSafe on such functions is ineffective.

Any objections to fixing these issues? Do you have a preferred implementation?

At minimum, it seems we should be using GetBoolMetaData(FBlueprintMetadata::MD_ThreadSafe), as is being done by FHeaderViewFunctionListItem::GetConditionalUFunctionSpecifiers.

It looks like we don’t use GetBoolMetaData because using the BlueprintThreadSafe specifier doesn’t actually set the key value to “true” (by default anyway) -- it’s just an empty string. So we can either fix that, or check that the specified value is not explicitly “false”.

For example, in HasFunctionBlueprintThreadSafeMetaData…

const FString* const ThreadSafeMetaData = InFunction->FindMetaData(FBlueprintMetadata::MD_ThreadSafe); const bool bHasThreadSafeMetaData = ThreadSafeMetaData != nullptr && *ThreadSafeMetaData != "false";

Hey there, I’ll collect thoughts from the dev team and will report back once I know more. Thanks for bringing this up.

Actually I see that you submitted a PR for this! It’s on our radar, so I’ll close this case and updates will be posted on the PR.