Bug in this Program?

Hello,

I have a bug in my program, I believe it probably exists for everyone. It happens when the FindSessions callback completes, and the played is no longer in the main menu. To reproduce:

1 - Launch the game
2 - Hit “Join” button to request a FindSessions.
3 - Quickly press “Cancel”, then “Host” before the delegate returns and the callback is called.
4 - When the callback UPuzzlePlatformsGameInstance::OnFindSessionsComplete(bool Success) is called, the program will crash when calling this function:

Menu->SetServerList(ServerNames);

The Menu pointer is not null. But the program crashes at the GetWorld line:

void UMainMenu::SetServerList(TArray ServerNames)
{
-------> UWorld* World = this->GetWorld(); <---------- CRASH HERE
if (!ensure(World != nullptr)) return;

Can anyone verify they also have this bug and explain why it is crashing ?

Thanks

I downloaded the course project from GitHub and tested it. It has the exact same issue. When the findsessions completes and the callback is called, the program will crash if you are no longer in the main menu.

Does anyone know why this happens?

I am having this bug as well
Very interested in the cause

Is the menu being destroyed as a result of the cancel? If it is then the “this” pointer might be null. Would be doing some extra checks to cover that cancellation case.

Where/how would you recommend protecting “this”? I inserted a simple nullptr check for “this”, but it then threw an error further down in DelegateInstanceInterface. I’m assuming there is something more comprehensive that we would need to do in order to manage when an entire level gets unloaded. Additionally, it does seem odd that code associated with MainMenu appears to be running after the new level has loaded.

if (!this)

{
	UE_LOG(LogTemp, Warning, TEXT("mainMenu SetServerList: Can't find my 'this' MainMenuObject"));
	return;
}

You shouldn’t really have to protect this. The protection should happen before calling the function.

Exactly what Sam said. You typically won’t see a crash because this is null, you would see the crash happen when calling a method on a class pointer that is null (or the instance was destroyed and the pointer now points to garbage). Since you are already in SetServerList, this instance of UMainMenu is still valid, otherwise the game would have crashed on your call to SetServerList (probably MainMenu->SetServerList(ServerList); in OnFindSessionsComplete.

The actual line that is reporting the crash is misleading - probably because you are running in DevelopmentEditor, which I believe is the UE4 equivalent of running in ‘Release’ mode. This means that you aren’t going to get accurate debug info because of compiler optimizations. However, running the project in debug reveals the real culprit:
image

The problem is that World is nullptr, and you are hitting the ensure macro. Instead of using the macro, you can simply do if (!World) return; and the method will exit cleanly (I tested this myself and no crash).

As to why World is null here? Well, my assumption is because we’re doing a ServerTravel when we host a game. I think this might actually de-register the UMainMenu instance with the current world or a new World is created after the ServerTravel call returns and our UMainMenu instance hasn’t registered itself yet. I’m going to do some further debugging on this and will update this post if I can pinpoint the exact reason.

Update

After running the debugger a couple times, it looks like AActor::GetWorld() might be getting called - note the comment:

UWorld* AActor::GetWorld() const
{
	// CDO objects do not belong to a world
	// If the actors outer is destroyed or unreachable we are shutting down and the world should be nullptr
	if (!HasAnyFlags(RF_ClassDefaultObject) && !GetOuter()->HasAnyFlags(RF_BeginDestroyed) && !GetOuter()->IsUnreachable())
	{
		if (ULevel* Level = GetLevel())
		{
			return Level->OwningWorld;
		}
	}
	return nullptr;
}

So this makes sense as to why World could be nullptr. What doesn’t make sense is why AActor’s GetWorld() is being called because our UMainMenu is actually a UUserWidget which also implements GetWorld(). It’s hard to say exactly what’s happening here but as I mentioned I did have to run the debugger several times before I was even able to step into the GetWorld() function. Usually random behavior indicates some sort of race condition, perhaps UMainMenu is in an undefined state when SetServerList() is called because we’ve transitioned to new map.

Granted, I had to click Join, then quickly go back to the main menu and click Host (and Host again from the set server name menu) to get into a map before the server list returned. This is probably not the typical use-case, and there are likely other ways we could break the game by performing an unintentional sequence of actions. I highly doubt Sam had an entire team of QA performing exhaustive stability tests on these demo apps. :wink: That being said, one thing we could do to improve the game’s stability is disable the Host button while a search is pending.

1 Like

Thanks Pyro, Appreciate the detail!

I actually had similar code in place there in SetServerList, but I’m still getting the crash. So perhaps some other element of the SetServerList is hanging.

UWorld* World = this->GetWorld();
if (!World) return;

So, two questions, if you know offhand:

  1. how would one go about “suspending” the functionality of the host button if a given event is in progress, i.e. the “Find servers” method", and
  2. Is there a way to uniformly find and squash these… what I assume are asynchronous methods… if the parent object gets destroyed?

Thanks again for the additional direction, always great to know I’m not the only one seeing these issues.

No problem! Sorry for the delay in response. To address your questions:

  1. Take a look at the UWidget::SetIsEnabled method. You have access to this method from your UButton members in MainMenu (hint: you can expose a method for enabling/disabling the Host button that can be called from your GameInstance methods). Have a go at it and let me know if you are still stuck.

  2. I’m not sure I completely understand your question, however if you want to simply stop a search in progress, take a look at the IOnlineSession::CancelFindSessions method (note that this is an interface method - if you are curious about the implementation details you will need to take a look at one of the derived classes e.g. FOnlineSessionNull::CancelFindSessions).

Just to add. We don’t use the destructor much in the course (if at all?) but this would be an excellent use case for it.

I’ve done some regression-testing, and CancelFindSessions is effective at squashing the crash bug. I thought about using SetEnabled, but then realized that waiting for the Host button to re-enable every time I left the Join menu would eventually get pretty annoying.

Since you are already in SetServerList , this instance of UMainMenu is still valid, otherwise the game would have crashed on your call to SetServerList (probably MainMenu->SetServerList(ServerList); in OnFindSessionsComplete .

Actually, I think that’s exactly what’s happening. ‘GetWorld’ is a getter for the cached world pointer, which returns null if the player isn’t actually spawned in the level.

In this case, the player starts a session search, leaves the menu and clicks Host, is transferred out of the MainMenu level and into the ExampleLevel, and OnFindSessionsComplete calls SetServerList and tries to attach a Row to a widget that’s now behind a null pointer.

That’s why CancelFindSessions works to stop the crash – we’ve stopped the search, so SetServerList is never called.

:Edit: I just realized that there’s a pointer guard on World in SetServerList…oh, well. It was nice to have an excuse to do some more research. XD

1 Like

Can you please upload an image of your code with the implementation of "CancelFindSessions ". I would appreciate it!