Hi,
I’m just testing out the FastGeo plugin on 5.6.1 and finding that once i add a few classes like static mesh actor and then press play into pie that is it deadlocking trying to take the lock for that component, but it doesn’t look like its in use on any other thread. Has this been seen or reported by anyone else? Is it possible that the component or at least its lock is now corrupted somehow and that’s why its deadlocking?
thanks
Sam
Steps to Reproduce
I’ve not managed to create an isolated reproduction of this, but on our project, it 100% hangs on press play in PIE.
[External Code]
> [Inline Frame] DinnerEditor-FastGeoStreaming.dll!Windows::AcquireSRWLockExclusive(Windows::SRWLOCK *) Line 158 C++
[Inline Frame] DinnerEditor-FastGeoStreaming.dll!UE::FWindowsSharedMutex::Lock() Line 87 C++
[Inline Frame] DinnerEditor-FastGeoStreaming.dll!UE::FPlatformRWLock::WriteLock() Line 23 C++
[Inline Frame] DinnerEditor-FastGeoStreaming.dll!FWriteScopeLock::{ctor}(UE::FPlatformRWLock &) Line 119 C++
DinnerEditor-FastGeoStreaming.dll!FFastGeoComponentCluster::ForceUpdateVisibility(const TArray<FFastGeoPrimitiveComponent *,TSizedDefaultAllocator<32>> & Components, int UpdateCounter) Line 140 C++
[Inline Frame] DinnerEditor-FastGeoStreaming.dll!FFastGeoComponentCluster::UpdateVisibility_Internal::__l5::<lambda_1>::()::__l8::<lambda_2>::operator()() Line 204 C++
[Inline Frame] DinnerEditor-FastGeoStreaming.dll!Invoke(FFastGeoComponentCluster::UpdateVisibility_Internal::__l5::<lambda_1>::()::__l8::<lambda_2> &) Line 47 C++
DinnerEditor-FastGeoStreaming.dll!UE::Tasks::Private::TExecutableTaskBase<``FFastGeoComponentCluster::UpdateVisibility_Internal’::`5’::<lambda_1>::operator()‘::`8’::<lambda_2>,void,void>::ExecuteTask() Line 904 C++
DinnerEditor-Core.dll!UE::Tasks::Private::FTaskBase::TryExecuteTask() Line 527 C++
[Inline Frame] DinnerEditor-Core.dll!FBaseGraphTask::Execute(TArray<FBaseGraphTask *,TSizedDefaultAllocator<32>> &) Line 505 C++
DinnerEditor-Core.dll!FNamedTaskThread::ProcessTasksNamedThread(int QueueIndex, bool bAllowStall) Line 779 C++
DinnerEditor-Core.dll!FNamedTaskThread::ProcessTasksUntilIdle(int QueueIndex) Line 679 C++
DinnerEditor-RenderCore.dll!GameThreadWaitForTask(const UE::Tasks::Private::FTaskHandle & Task, bool bEmptyGameThreadTasks) Line 1277 C++
[Inline Frame] DinnerEditor-RenderCore.dll!FRenderCommandFence::Wait(bool) Line 1319 C++
DinnerEditor-RenderCore.dll!FFrameEndSync::FRenderThreadFence::~FRenderThreadFence() Line 2043 C++
[Inline Frame] DinnerEditor-RenderCore.dll!DestructItem(FFrameEndSync::FRenderThreadFence *) Line 76 C++
[Inline Frame] DinnerEditor-RenderCore.dll!TArray<FFrameEndSync::FRenderThreadFence,TSizedInlineAllocator<2,32,TSizedDefaultAllocator<32>>>::RemoveAtImpl(int) Line 2060 C++
[Inline Frame] DinnerEditor-RenderCore.dll!TArray<FFrameEndSync::FRenderThreadFence,TSizedInlineAllocator<2,32,TSizedDefaultAllocator<32>>>::RemoveAt(int) Line 2101 C++
DinnerEditor-RenderCore.dll!FFrameEndSync::Sync(FFrameEndSync::EFlushMode FlushMode) Line 2094 C++
DinnerEditor-RenderCore.dll!FlushRenderingCommands() Line 1365 C++
DinnerEditor-Engine.dll!FMaterialUpdateContext::FMaterialUpdateContext(unsigned int Options, EShaderPlatform InShaderPlatform) Line 4701 C++
DinnerEditor-Engine.dll!UMaterial::SetMaterialUsage(bool & bNeedsRecompile, const EMaterialUsage Usage, UMaterialInterface * MaterialInstance) Line 1948 C++
DinnerEditor-Engine.dll!UMaterial::CheckMaterialUsage(const EMaterialUsage Usage) Line 1832 C++
DinnerEditor-Engine.dll!UMaterial::CheckMaterialUsage_Concurrent(const EMaterialUsage Usage) Line 1844 C++
DinnerEditor-Engine.dll!FInstancedStaticMeshSceneProxy::SetupProxy(const FInstancedStaticMeshSceneProxyDesc & InProxyDesc) Line 1448 C++
DinnerEditor-Engine.dll!FInstancedStaticMeshSceneProxy::FInstancedStaticMeshSceneProxy(const FInstancedStaticMeshSceneProxyDesc & InProxyDesc, ERHIFeatureLevel::Type InFeatureLevel) Line 1422 C++
DinnerEditor-FastGeoStreaming.dll!FFastGeoInstancedStaticMeshComponent::CreateStaticMeshSceneProxy(const Nanite::FMaterialAudit & NaniteMaterials, bool bCreateNanite) Line 175 C++
DinnerEditor-FastGeoStreaming.dll!FStaticMeshComponentHelper::CreateSceneProxy<FFastGeoStaticMeshComponentBase,1>(FFastGeoStaticMeshComponentBase & Component, FStaticMeshComponentHelper::ESceneProxyCreationError * OutError) Line 537 C++
DinnerEditor-FastGeoStreaming.dll!FFastGeoStaticMeshComponentBase::CreateSceneProxy(ESceneProxyCreationError * OutError) Line 261 C++
DinnerEditor-FastGeoStreaming.dll!FFastGeoPrimitiveComponent::CreateRenderState(FRegisterComponentContext * Context) Line 327 C++
DinnerEditor-FastGeoStreaming.dll!UFastGeoWorldSubsystem::ProcessPendingRecreate() Line 333 C++
[External Code]
[Inline Frame] DinnerEditor-Engine.dll!TMulticastDelegateBase<FDefaultDelegateUserPolicy>::Broadcast(UWorld *) Line 258 C++
[Inline Frame] DinnerEditor-Engine.dll!TMulticastDelegate<void __cdecl(UWorld *),FDefaultDelegateUserPolicy>::Broadcast(UWorld *) Line 1080 C++
DinnerEditor-Engine.dll!UWorld::SendAllEndOfFrameUpdates() Line 990 C++
DinnerEditor-UnrealEd.dll!UEditorEngine::Tick(float DeltaSeconds, bool bIdleMode) Line 2300 C++
DinnerEditor-UnrealEd.dll!UUnrealEdEngine::Tick(float DeltaSeconds, bool bIdleMode) Line 533 C++
DinnerEditor-TtUnrealEditor.dll!UTtEditorEngine::Tick(float DeltaSeconds, bool bIdleMode) Line 221 C++
DinnerEditor.exe!FEngineLoop::Tick() Line 5673 C++
[Inline Frame] DinnerEditor.exe!EngineTick() Line 60 C++
DinnerEditor.exe!GuardedMain(const wchar_t * CmdLine) Line 187 C++
DinnerEditor.exe!LaunchWindowsStartup(HINSTANCE__ * hInInstance, HINSTANCE__ * hPrevInstance, char * __formal, int nCmdShow, const wchar_t * CmdLine) Line 271 C++
DinnerEditor.exe!WinMain(HINSTANCE__ * hInInstance, HINSTANCE__ * hPrevInstance, char * pCmdLine, int nCmdShow) Line 339 C++
[External Code]
Hi Sam,
I actually fixed a problem that could explain your issue.
The fix is submitted in our main branch (CL 45754905)
I don’t know if you have access. If not here are the diffs:
In FastGeoPrimitiveComponent.h:
Change
TDontCopy<FRWLock> Lock;for
TUniquePtr<FRWLock> Lock;In FastGeoComponentCluster.cpp
In
Change:
FWriteScopeLock WriteLock(Component.Lock.Get());for
FWriteScopeLock WriteLock(*Component.Lock.Get());In FastGeoPrimitiveComponent.cpp:
FFastGeoPrimitiveComponent::FFastGeoPrimitiveComponent(int32 InComponentIndex, FFastGeoElementType InType)
: Super(InComponentIndex, InType)
, LocalBounds(ForceInit)
, WorldBounds(ForceInit)
>>> , Lock(MakeUnique<FRWLock>())
{
}
Same here:
FFastGeoPrimitiveComponent::FFastGeoPrimitiveComponent(const FFastGeoPrimitiveComponent& Other)
: FFastGeoComponent(Other)
, LocalTransform(Other.LocalTransform)
, WorldTransform(Other.WorldTransform)
, LocalBounds(Other.LocalBounds)
, WorldBounds(Other.WorldBounds)
, bIsVisible(Other.bIsVisible)
, bStaticWhenNotMoveable(Other.bStaticWhenNotMoveable)
, bFillCollisionUnderneathForNavmesh(Other.bFillCollisionUnderneathForNavmesh)
, bRasterizeAsFilledConvexVolume(Other.bRasterizeAsFilledConvexVolume)
, bCanEverAffectNavigation(Other.bCanEverAffectNavigation)
, CustomPrimitiveData(Other.CustomPrimitiveData)
, DetailMode(Other.DetailMode)
, bHasCustomNavigableGeometry(Other.bHasCustomNavigableGeometry)
, BodyInstance(Other.BodyInstance)
, RuntimeVirtualTextures(Other.RuntimeVirtualTextures)
, BodyInstanceOwner()
, PrimitiveSceneData()
, AsyncTermBodyPayload()
, ProxyState(EProxyCreationState::None)
, bRenderStateDirty(false)
#if !WITH_EDITOR && UE_WITH_PSO_PRECACHING
, MaterialPSOPrecacheRequestIDs()
, LatestPSOPrecacheJobSetCompleted(0)
, LatestPSOPrecacheJobSet(0)
, bPSOPrecacheCalled(false)
, bPSOPrecacheRequired(false)
, PSOPrecacheRequestPriority(EPSOPrecachePriority::Medium)
#endif
>>> , Lock(MakeUnique<FRWLock>())
{
}
void FFastGeoPrimitiveComponent::CreateRenderState(FRegisterComponentContext* Context)
{
FWriteScopeLock WriteLock(*Lock.Get());void FFastGeoPrimitiveComponent::CreateRenderState(FRegisterComponentContext* Context)
{
>>> FWriteScopeLock WriteLock(*Lock.Get());
...
}
void FFastGeoPrimitiveComponent::DestroyRenderState(FFastGeoDestroyRenderStateContext* Context)
{
>>> FWriteScopeLock WriteLock(*Lock.Get());
if (GetSceneProxy())
{
...
}
Let me know if it fixes your issue.
Richard
Hi Sam,
I see in the callstack that it’s trying to lock a 2nd time the same Lock.
We didn’t see that issue yet.
I will investigate and propose a solution.
Richard
| Is there an expectation that the code paths should never allow for the lock to be taken more than once recursively on the stack? As its not using re-entrant locks.
Yes that was the assumption.
Do you think you prepare a small map that reproduces this exact case and send it over?
Hi Richard,
I honestly dont know why its happening, so wouldnt know how to reproduce it on a standalone build, you have more chance than I do at this point.
Im going to have a look at the code and see if I can wind the recursion thats causing the lock to be taken again.
Hi Sam,
I had a bit of time to look at this.
You are hitting a case where a material is used by an ISM but doesn’t have its flag bUsedWithInstancedStaticMeshes and we end up in that special case.
Unfortunately I’m still not able to reproduce that locally.
But I still implemented something to delay the execution of ForceUpdateVisibility.
It’s a bit convoluted, but let me know if it unblocks you.
FastGeoWorldSubsystem.h
public:
bool IsFlushRenderingCommandsRunning() const { return bIsFlushRenderingCommandsRunning; }
private:
void OnFlushRenderingCommandsStart();
void OnFlushRenderingCommandsEnd();
FDelegateHandle Handle_OnFlushRenderingCommandsStart;
FDelegateHandle Handle_OnFlushRenderingCommandsEnd;
bool bIsFlushRenderingCommandsRunning = false;
FastGeoWorldSubsystem.cpp
void UFastGeoWorldSubsystem::PostInitialize()
{
Super::PostInitialize();
UWorld* World = GetWorld();
if (World->IsPartitionedWorld())
{
World->OnAllLevelsChanged().AddUObject(this, &UFastGeoWorldSubsystem::OnUpdateLevelStreaming);
World->OnAddLevelToWorldExtension().AddUObject(this, &UFastGeoWorldSubsystem::OnAddLevelToWorldExtension);
World->OnRemoveLevelFromWorldExtension().AddUObject(this, &UFastGeoWorldSubsystem::OnRemoveLevelFromWorldExtension);
FWorldDelegates::LevelComponentsUpdated.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelComponentsUpdated);
FWorldDelegates::LevelComponentsCleared.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelComponentsCleared);
#if DO_CHECK
FWorldDelegates::LevelAddedToWorld.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelAddedToWorld);
FWorldDelegates::LevelRemovedFromWorld.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelRemovedFromWorld);
#endif
Handle_OnLevelStreamingStateChanged = FLevelStreamingDelegates::OnLevelStreamingStateChanged.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelStreamingStateChanged);
Handle_OnLevelBeginAddToWorld = FLevelStreamingDelegates::OnLevelBeginMakingVisible.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelStartedAddToWorld);
Handle_OnLevelBeginRemoveFromWorld = FLevelStreamingDelegates::OnLevelBeginMakingInvisible.AddUObject(this, &UFastGeoWorldSubsystem::OnLevelStartedRemoveFromWorld);
Handle_OnForEachHLODObjectInCell = World->GetSubsystem<UWorldPartitionHLODRuntimeSubsystem>()->GetForEachHLODObjectInCellEvent().AddUObject(this, &UFastGeoWorldSubsystem::ForEachHLODObjectInCell);
>>> Handle_OnFlushRenderingCommandsStart = FCoreRenderDelegates::OnFlushRenderingCommandsStart.AddUObject(this, &UFastGeoWorldSubsystem::OnFlushRenderingCommandsStart);
>>> Handle_OnFlushRenderingCommandsEnd = FCoreRenderDelegates::OnFlushRenderingCommandsEnd.AddUObject(this, &UFastGeoWorldSubsystem::OnFlushRenderingCommandsEnd);
}
}
void UFastGeoWorldSubsystem::Deinitialize()
{
UWorld* World = GetWorld();
World->OnAllLevelsChanged().RemoveAll(this);
World->OnAddLevelToWorldExtension().RemoveAll(this);
World->OnRemoveLevelFromWorldExtension().RemoveAll(this);
FWorldDelegates::LevelComponentsUpdated.RemoveAll(this);
FWorldDelegates::LevelComponentsCleared.RemoveAll(this);
#if DO_CHECK
FWorldDelegates::LevelAddedToWorld.RemoveAll(this);
FWorldDelegates::LevelRemovedFromWorld.RemoveAll(this);
#endif
FLevelStreamingDelegates::OnLevelStreamingStateChanged.Remove(Handle_OnLevelStreamingStateChanged);
FLevelStreamingDelegates::OnLevelBeginMakingVisible.Remove(Handle_OnLevelBeginAddToWorld);
FLevelStreamingDelegates::OnLevelBeginMakingInvisible.Remove(Handle_OnLevelBeginRemoveFromWorld);
>>>FCoreRenderDelegates::OnFlushRenderingCommandsStart.Remove(Handle_OnFlushRenderingCommandsStart);
>>>FCoreRenderDelegates::OnFlushRenderingCommandsEnd.Remove(Handle_OnFlushRenderingCommandsEnd);
Handle_OnLevelStreamingStateChanged.Reset();
Handle_OnLevelBeginAddToWorld.Reset();
Handle_OnLevelBeginRemoveFromWorld.Reset();
>>> Handle_OnFlushRenderingCommandsStart.Reset();
>>> Handle_OnFlushRenderingCommandsEnd.Reset();
World->GetSubsystem<UWorldPartitionHLODRuntimeSubsystem>()->GetForEachHLODObjectInCellEvent().Remove(Handle_OnForEachHLODObjectInCell);
Handle_OnForEachHLODObjectInCell.Reset();
Super::Deinitialize();
}
>>>void UFastGeoWorldSubsystem::OnFlushRenderingCommandsStart()
>>>{
>>> bIsFlushRenderingCommandsRunning = true;
>>>}
>>>void UFastGeoWorldSubsystem::OnFlushRenderingCommandsEnd()
>>>{
>>> bIsFlushRenderingCommandsRunning = false;
>>>}
FastGeoComponentCluster.h
// Forward declare FWeakFastGeoComponent
class FWeakFastGeoComponent;
private:
//void ForceUpdateVisibility(const TArray<FFastGeoPrimitiveComponent*>& Components, int32 UpdateCounter);
void ForceUpdateVisibility(const TArray<FWeakFastGeoComponent>& Components, int32 UpdateCounter);
FastGeoComponentCluster.cpp
void FFastGeoComponentCluster::ForceUpdateVisibility(const TArray<FWeakFastGeoComponent>& Components, int32 UpdateCounter)
{
UFastGeoWorldSubsystem* WorldSubsystem = UWorld::GetSubsystem<UFastGeoWorldSubsystem>(GetOwnerContainer()->GetWorld());
if (WorldSubsystem && WorldSubsystem->IsFlushRenderingCommandsRunning())
{
FWeakFastGeoComponentCluster ClusterWeak(this);
TSharedPtr<FDelegateHandle, ESPMode::ThreadSafe> FlushHandle = MakeShared<FDelegateHandle, ESPMode::ThreadSafe>();
*FlushHandle = FCoreRenderDelegates::OnFlushRenderingCommandsEnd.AddLambda([ClusterWeak, Components, UpdateCounter, FlushHandle]()
{
check(IsInGameThread());
if (FlushHandle.IsValid())
{
FCoreRenderDelegates::OnFlushRenderingCommandsEnd.Remove(*FlushHandle);
}
UE::Tasks::Launch(TEXT("ForceUpdateVisibility"), [ClusterWeak, Components, UpdateCounter]()
{
if (FFastGeoComponentCluster* Cluster = ClusterWeak.Get())
{
Cluster->ForceUpdateVisibility(Components, UpdateCounter);
}
}, LowLevelTasks::ETaskPriority::Normal, UE::Tasks::EExtendedTaskPriority::GameThreadNormalPri);
});
return;
}
TRACE_CPUPROFILER_EVENT_SCOPE(FFastGeoComponentCluster::ForceUpdateVisibility);
TArray<FFastGeoPrimitiveComponent*> ShowComponents;
TArray<FFastGeoPrimitiveComponent*> HideComponents;
//for (FFastGeoPrimitiveComponent* Component : Components)
for (const FWeakFastGeoComponent& WeakComponent : Components)
{
if (FFastGeoPrimitiveComponent* Component = WeakComponent.Get<FFastGeoPrimitiveComponent>())
{
// Same code
}
}
UpdateVisibility_Internal(MoveTemp(ShowComponents), MoveTemp(HideComponents), ++UpdateCounter);
}
void FFastGeoComponentCluster::UpdateVisibility_Internal(TArray<FFastGeoPrimitiveComponent*>&& ShowComponents, TArray<FFastGeoPrimitiveComponent*>&& HideComponents, int32 UpdateCounter)
{
if (!ShowComponents.IsEmpty() || !HideComponents.IsEmpty())
{
FWeakFastGeoComponentCluster ClusterWeak(this);
ENQUEUE_RENDER_COMMAND()([ClusterWeak, UpdateCounter, ShowComponents = MoveTemp(ShowComponents), HideComponents = MoveTemp(HideComponents)](FRHICommandListBase&) mutable
{
TRACE_CPUPROFILER_EVENT_SCOPE(FFastGeoComponentCluster::UpdateVisibility_RenderThread);
if (!ClusterWeak.Get())
{
return;
}
//TArray<FFastGeoPrimitiveComponent*> NotReadyComponents;
TArray<FWeakFastGeoComponent> NotReadyComponents;
// Same code...
}
Richard
Hi Sam,
For porting to the latest, there should not be that many changes. If you need help merging, let us know.
About the fix I sent you, could you try a different approach?
Instead of the - somewhat - big change I sent you, could you try instead changing UMaterial::SetMaterialUsage like this :
replace this line:
if( GIsEditor && !FApp::IsGame() && bAutomaticallySetUsageInEditor )with:
bool bIsGame = FApp::IsGame();
#if WITH_EDITOR
bIsGame = bIsGame || (GEditor && GEditor->IsPlayingSessionInEditor());
#endif
if( GIsEditor && !bIsGame && bAutomaticallySetUsageInEditor )
Note that you hit the case where some materials are missing some usage flags (in your case it seems to be the “UsedWithInstancedStaticMeshes”.
You should see some warnings like this:
“Warning: Material <YourMaterialPath> missing bUsedWithInstancedStaticMeshes=True! Default Material will be used in game.”
If it fixes your deadlock, I will go ahead and submit this as an official fix.
Thanks,
Richard
Hi Richard,
So as im now on newer revision of our project I cannot reproduce the issue, it seems the data must have changed, im going to revert back to a couple of days ago and try your latest change once i confirm I can reproduce it again.
I previously tried to merge over the latest FastGeo, it needed small interface changes to the physics system, but it also needed a load of other changes for fetching transforms and stuff from the world partition, so looked like i was going to need to bring over world partiton stuff, and its impossible for me to know how much of the changes I can safely bring over.
thanks
Sam
I can confirm this simplified fix does fix the issue for me, ill promote this into our version control.
Ok perfect, thanks for the feedback, I’ll submit this in our main branch.
Cheers,
Richard
Not sure if you are planning a 5.6.2 release but if so can you merge it there also, thanks
Hi Sam,
I don’t think there are any plans for 5.6.2.
Richard
Hi,
I have already found these changes and applied them and it sadly has made no difference. One thing I did notice is that the version we have from 5.6.1 doesn’t have a copy constructor, so couldn’t just add that one like to create the lock, I had to paste the entire copy constructor over, but the issue still persists.
We think its unlikely we will be able to move to 5.7, are you able provide some changelists to allow us to take the latest version of FastGeo onto 5.6.1? I copied over the last version of FastGeo, but it wouldn’t compile as it needs to missing functions in the physics system, so wanted to reach out for a list rather than trying to guess which changes might be required. Can you assist with this?
thanks
Sam
Hi,
I did notice this took, I have other changes where I extended the logic to reduce the locking on mainthreads by adding TryBegin logic to the locks and allowing it to deffer to the next frame or re-queue the work. Is there an expectation that the code paths should never allow for the lock to be taken more than once recursively on the stack? As its not using re-entrant locks.
Hi Richard,
This fixes the hang for me 
Thanks
Can you also assist with patching/back porting the latest FastGeo into 5.6.1 or is it too many changes required, I started todo it and see it needed a few new things from the world partition system?