Editor crash in FDebugDrawDelegateHelper::HandleDrawDebugLabels

Development Pipeline & Plugins
, ,
1

Hello there,

Since upgrading from 5.5 to 5.6, we’ve been seeing infrequent editor crashes with the included callstack.

Specifically, this is a crash with the following dereference within FDebugDrawDelegateHelper::HandleDrawDebugLabels(UCanvas* Canvas, APlayerController* PlayerController):

if (Canvas->Canvas != nullptr

&& AssociatedWorld != Canvas->Canvas->GetScene()->GetWorld())

and in particular, is caused since the FCanvas* Canvas member of the UCanvas* Canvas parameter is not null, so either it’s dereference by calling GetScene() or the subsequent derefence in the call to GetWorld() are illegal.

Is this something that has been seen by Epic? Team members report encountering this crash while performing manipulations to actor transforms, but getting a repro is tricky. It seems like the crash is mainly encountered via calls to FViewport::GetRawHitProxyData(), and not during the regular per-frame drawing.

Is it possible that this is caused because of a race condition in the handoff of the GameThreadCanvas to the RenderThreadCanvas in the FDebugCanvasDrawer?

Any help in finding the cause of this infrequent crash would be appreciated.

Thank you!

2

Steps to Reproduce
Team members report encountering this crash while performing manipulations to actor transforms, but getting a repro is tricky.

3

Hello there, I have more info on this crash and would like your input on my proposed way to go about fixing it. The key discovery that I’ve made is that we have a scene view extension which loads sparse volume textures as part of its call to ISceneViewExtension::SetupViewFamily() which is ultimately the cause of the crash. SVT loading creates a slow task modal window, which sometimes causes the focus to be taken off the viewport which causes a hit proxy query, which in turn causes the debug canvas to be recreated in

FDebugCanvasDrawer::InitDebugCanvas(). The exact operations leading to this can be seen in the following call stack:

FDebugCanvasDrawer::InitDebugCanvas(FViewportClient *, UWorld *) DebugCanvas.cpp:230
FSceneViewport::EnqueueBeginRenderFrame(const bool) SceneViewport.cpp:1881
FViewport::GetRawHitProxyData(TIntRect<…>) UnrealClient.cpp:1919
FViewport::GetHitProxyMap(TIntRect<…>, TArray<…> &) UnrealClient.cpp:1995
FViewport::GetHitProxy(int, int) UnrealClient.cpp:2043
FEditorViewportClient::StopTracking() EditorViewportClient.cpp:3359
FEditorViewportClient::LostFocus(FViewport *) EditorViewportClient.cpp:1500
FLevelEditorViewportClient::LostFocus(FViewport *) LevelEditorViewport.cpp:2783
FSceneViewport::OnFocusLost(const FFocusEvent &) SceneViewport.cpp:1258
SViewport::OnFocusLost(const FFocusEvent &) SViewport.cpp:315
FSlateApplication::SetUserFocus(FSlateUser &, const FWidgetPath &, EFocusCause) SlateApplication.cpp:3022
[Inlined] FSlateApplication::SetAllUserFocusAllowingDescendantFocus::__l2::<lambda_1>::operator()(FSlateUser &) SlateApplication.cpp:3094
[Inlined] Invoke(<lambda_1> &, FSlateUser &) Invoke.h:47
UE::Core::Private::Function::TFunctionRefCaller<`FSlateApplication::SetAllUserFocusAllowingDescendantFocus'::`2'::<lambda_1>,void,FSlateUser &>::Call(void *,FSlateUser &) Function.h:316
[Inlined] UE::Core::Private::Function::TFunctionRefBase::operator()(FSlateUser &) Function.h:471
[Inlined] FSlateApplication::ForEachUser(TFunctionRef<…>, bool) SlateApplication.cpp:4452
FSlateApplication::SetAllUserFocusAllowingDescendantFocus(const FWidgetPath &, EFocusCause) SlateApplication.cpp:3091
SWindow::OnIsActiveChanged(const FWindowActivateEvent &) SWindow.cpp:1721
FSlateApplication::ProcessWindowActivatedEvent(const FWindowActivateEvent &) SlateApplication.cpp:6903
FSlateApplication::OnWindowActivationChanged(const TSharedRef<…> &, EWindowActivation) SlateApplication.cpp:6869
FWindowsApplication::ProcessDeferredMessage(const FDeferredWindowsMessage &) WindowsApplication.cpp:3287
FWindowsApplication::DeferMessage(TSharedPtr<…> &, HWND__ *, unsigned int, unsigned long long, long long, int, int, unsigned int) WindowsApplication.cpp:3606
FWindowsApplication::ProcessMessage(HWND__ *, unsigned int, unsigned long long, long long) WindowsApplication.cpp:2764
[Inlined] WindowsApplication_WndProc(HWND__ *, unsigned int, unsigned long long, long long) WindowsApplication.cpp:1752
FWindowsApplication::AppWndProc(HWND__ *, unsigned int, unsigned long long, long long) WindowsApplication.cpp:1757
<unknown> 0x00007ffee1e88eb8
<unknown> 0x00007ffee1e8896c
<unknown> 0x00007ffee1e93e1d
<unknown> 0x00007ffee2114c94
<unknown> 0x00007ffedf7f1f44
FWindowsWindow::Show() WindowsWindow.cpp:743
SWindow::ShowWindow() SWindow.cpp:1450
FFeedbackContextEditor::StartSlowTask(const FText &, bool) FeedbackContextEditor.cpp:531
FSlowTask::MakeDialog(bool, bool) SlowTask.cpp:237
UStreamableSparseVolumeTexture::RecacheFrames() SparseVolumeTexture.cpp:1724
UObject::ConditionalPostLoad() Obj.cpp:1356
FAsyncPackage2::ExecuteDeferredPostLoadLinkerLoadPackageExports(FAsyncLoadingThreadState2 &) AsyncLoading2.cpp:7170
FAsyncPackage2::Event_DeferredPostLoadExportBundle(FAsyncLoadingThreadState2 &, FAsyncPackage2 *, int) AsyncLoading2.cpp:8944
FEventLoadNode2::Execute(FAsyncLoadingThreadState2 &) AsyncLoading2.cpp:5713
FAsyncLoadEventQueue2::ExecuteSyncLoadEvents(FAsyncLoadingThreadState2 &) AsyncLoading2.cpp:5919
FAsyncLoadingThread2::ProcessLoadedPackagesFromGameThread(FAsyncLoadingThreadState2 &, bool &, TArrayView<…>) AsyncLoading2.cpp:9294
FAsyncLoadingThread2::TickAsyncLoadingFromGameThread(FAsyncLoadingThreadState2 &, bool, bool, double, TArrayView<…>, bool &) AsyncLoading2.cpp:9558
FAsyncLoadingThread2::FlushLoading(TArrayView<…>) AsyncLoading2.cpp:11243
FlushAsyncLoading(TArrayView<…>) AsyncPackageLoader.cpp:362
FlushAsyncLoading(int) AsyncPackageLoader.cpp:331
LoadPackageInternal(UPackage *, const FPackagePath &, unsigned int, FLinkerLoad *, FArchive *, const FLinkerInstancingContext *, const FPackagePath *) UObjectGlobals.cpp:1771
LoadPackage(UPackage *, const FPackagePath &, unsigned int, FArchive *, const FLinkerInstancingContext *, const FPackagePath *) UObjectGlobals.cpp:2135
LoadPackage(UPackage *, const wchar_t *, unsigned int, FArchive *, const FLinkerInstancingContext *) UObjectGlobals.cpp:2111
ResolveName2(UObject *&, TStringBuilderBase<…> &, bool, bool, unsigned int, const FLinkerInstancingContext *) UObjectGlobals.cpp:1277
StaticLoadObjectInternal(UClass *, UObject *, const wchar_t *, const wchar_t *, unsigned int, UPackageMap *, bool, const FLinkerInstancingContext *) UObjectGlobals.cpp:1397
StaticLoadObject(UClass *, UObject *, const wchar_t *, const wchar_t *, unsigned int, UPackageMap *, bool, const FLinkerInstancingContext *) UObjectGlobals.cpp:1466
FSoftObjectPath::TryLoad(FUObjectSerializeContext *) SoftObjectPath.cpp:806
FSoftObjectPtr::LoadSynchronous() SoftObjectPtr.h:87
[Inlined] TSoftObjectPtr::LoadSynchronous() SoftObjectPtr.h:516
Redacted frame
Redacted frame
RedactedSceneViewExtension::SetupViewFamily(FSceneViewFamily &) RedactedSceneViewExtension.cpp:220
FEditorViewportClient::Draw(FViewport *, FCanvas *) EditorViewportClient.cpp:4426
FViewport::Draw(bool) UnrealClient.cpp:1819
UEditorEngine::UpdateSingleViewportClient(FEditorViewportClient *, const bool, bool, bool *) EditorEngine.cpp:2662
UEditorEngine::Tick(float, bool) EditorEngine.cpp:2326
UUnrealEdEngine::Tick(float, bool) UnrealEdEngine.cpp:530
FEngineLoop::Tick() LaunchEngineLoop.cpp:5644
[Inlined] EngineTick() Launch.cpp:60
GuardedMain(const wchar_t *) Launch.cpp:189
LaunchWindowsStartup(HINSTANCE__ *, HINSTANCE__ *, char *, int, const wchar_t *) LaunchWindows.cpp:271
WinMain(HINSTANCE__ *, HINSTANCE__ *, char *, int) LaunchWindows.cpp:339
[Inlined] invoke_main() 0x00007ff667ba88fa
__scrt_common_main_seh() 0x00007ff667ba88d9
<unknown> 0x00007ffee11e259d
<unknown> 0x00007ffee20caf78
4

It is possible that any modal pop up could cause such an issue.

However, this is problematic due to the event ordering of FEditorViewportClient::Draw(FViewport* InViewport, FCanvas* Canvas):

  1. Early in its execution, FCanvas* DebugCanvas = Viewport->GetDebugCanvas() gets the debug canvas pointer from the viewport.
  2. Later, SetupViewFamily(ViewFamily) is called for every scene view extension, which in our case, may recreate the debug canvas thus causing DebugCanvas to dangle
  3. DebugCanvas is next used much later in UDebugDrawService::Draw(ViewFamily.EngineShowFlags, Viewport, View, DebugCanvas) and a few subsequent lines.

This is erroneous because DebugCanvas may be dangling by event (3).

There are a few ways we can go about fixing this:

  • Adding a method to ISceneViewExtension that happens even earlier on the game thread in which we can load SVTs, bypassing this problem. This would likely be more effort to call correctly and maintain than the preferred solution.
  • Removing the modal loading dialogue for sparse volume textures, either in some cases or entirely. This isn’t preferred because there may be cases where we want this dialogue and doing this selectively will be awkward/error prone, and this wouldn’t fix the issue for other modal dialogues that might happen to be created in a likewise manner.
  • Redesigning our system and our scene view extension to not load SVTs during ISceneViewExtension::SetupViewFamily(). This isn’t preferred because this is a convenient spot to do this loading only as required, avoiding a more convoluted communication scheme, plus this wouldn’t fix the issue for other modal dialogues as before
  • My preference would simply be to modify FEditorViewportClient::Draw() to move event (1) after event (2), as close to event (3) as possible to prevent any chance of this dangling. So instead of this:
void FEditorViewportClient::Draw(FViewport* InViewport, FCanvas* Canvas)
{
	...
 
	// Allow HMD to modify the view later, just before rendering
	const bool bStereoRendering = GEngine->IsStereoscopic3D( InViewport );
	FCanvas* DebugCanvas = Viewport->GetDebugCanvas(); // WILL MOVE
	if (DebugCanvas) // WILL MOVE
	{ // WILL MOVE
		DebugCanvas->SetScaledToRenderTarget(bStereoRendering); // WILL MOVE
		DebugCanvas->SetStereoRendering(bStereoRendering); // WILL MOVE
	} // WILL MOVE
...

we would have this:

void FEditorViewportClient::Draw(FViewport* InViewport, FCanvas* Canvas)
{
	...
	...
	...
	...
	...
	FCanvas* DebugCanvas = Viewport->GetDebugCanvas();
	if (DebugCanvas)
	{
		DebugCanvas->SetScaledToRenderTarget(bStereoRendering);
		DebugCanvas->SetStereoRendering(bStereoRendering);
	}
	// NOTE: DebugCanvasObject will be created by UDebugDrawService::Draw() if it doesn't already exist.
	UDebugDrawService::Draw(ViewFamily.EngineShowFlags, Viewport, View, DebugCanvas);
	UCanvas* DebugCanvasObject = FindObjectChecked<UCanvas>(GetTransientPackage(),TEXT("DebugCanvasObject"));
	DebugCanvasObject->Canvas = DebugCanvas;
	DebugCanvasObject->Init( Viewport->GetSizeXY().X, Viewport->GetSizeXY().Y, View , DebugCanvas);
...

Please let me know if there are any issues you foresee with this solution by modifying FEditorViewportClient::Draw(), or if you think this fix should be sent back to UE mainline.

5

Hello!

Apologies for the delay, the FEditorViewportClient::Draw() code in question is rather old but likely OK to move, and you’re right that adding modifications to ISceneViewExtension would likely be a higher maintenance cost. We’re still taking a look at the options here and appreciate your patience.

6

After discussing with the team, we think it’d be good to have this change (move the DebugCanvas declaration lower) as a PR. If you provide the PR link on Github, I can link it to this ticket and tag the correct team internally.

7

Looked at the code here a while.

A concern:

Are we confident that nothing uses the DebugCanvas between where it is currently created, and where you propose to create it? If so moving it would break that debug draw. Also the recreation behavior would probably be breaking that debug draw (throwing away the canvas on which it was drawn), though perhaps it does not matter that debug draw may be missing information for one frame when a dialog switch happens.

Another option:

We could stop using that DebugCanvas local variable way down the function.

CHANGE FROM:
	FCanvas* DebugCanvas = Viewport->GetDebugCanvas();
	if (DebugCanvas)
	{
		DebugCanvas->SetScaledToRenderTarget(bStereoRendering);
		DebugCanvas->SetStereoRendering(bStereoRendering);
	}
 
	// ... //
 
	// NOTE: DebugCanvasObject will be created by UDebugDrawService::Draw() if it doesn't already exist.
	UDebugDrawService::Draw(ViewFamily.EngineShowFlags, Viewport, View, DebugCanvas);
	UCanvas* DebugCanvasObject = FindObjectChecked<UCanvas>(GetTransientPackage(),TEXT("DebugCanvasObject"));
	DebugCanvasObject->Canvas = DebugCanvas;
	DebugCanvasObject->Init( Viewport->GetSizeXY().X, Viewport->GetSizeXY().Y, View , DebugCanvas);
 
 
TO: 
	if (FCanvas* DebugCanvas = Viewport->GetDebugCanvas())
	{
		DebugCanvas->SetScaledToRenderTarget(bStereoRendering);
		DebugCanvas->SetStereoRendering(bStereoRendering);
	}
 
	// ... //
 
	// NOTE: DebugCanvasObject will be created by UDebugDrawService::Draw() if it doesn't already exist.
	{
		FCanvas* DebugCanvas = Viewport->GetDebugCanvas()
		UDebugDrawService::Draw(ViewFamily.EngineShowFlags, Viewport, View, DebugCanvas);
		UCanvas* DebugCanvasObject = FindObjectChecked<UCanvas>(GetTransientPackage(),TEXT("DebugCanvasObject"));
		DebugCanvasObject->Canvas = DebugCanvas;
		DebugCanvasObject->Init( Viewport->GetSizeXY().X, Viewport->GetSizeXY().Y, View , DebugCanvas);
	}*** actually a few more lines need to go in before the close brace

Such that in the later block of code we are using the Viewport’s current debug canvas, not the one we got earlier.