UOnlineSessionClient OnDestroyForMainMenu Logic Bug

Code inside of UOnlineSessionClient is causing game crashes for us when a disconnect is handled by the above class.

The following occurs:

When DestroyExistingSession_Impl is called, the following code is run:

		Result = SessionInt->AddOnDestroySessionCompleteDelegate_Handle(Delegate);
		SessionInt->DestroySession(SessionName);

Notice here the resulting handle from the delegate addition is assigned to a local variable, and does not have a chance to be stored in the variable OnDestroyForMainMenuCompleteDelegateHandle before the call to DestroySession immediately after. This is because OnDestroyForMainMenuCompleteDelegateHandle is assigned via DestroyExistingSession_Impl’s return.

The problem occurs when the DestroySessionCompleteDelegate is called synchronously during the call to DestroySession which happens right after the delegate’s addition, but before it is saved to OnDestroyForMainMenuCompleteDelegateHandle.

The Delegate has OnDestroyForMainMenuComplete assigned to it, and in that call to OnDestroyForMainMenuComplete which occurs synchronously during the DestroySession call,

the class tries to clear the delegate like so:

SessionInt->ClearOnDestroySessionCompleteDelegate_Handle(OnDestroyForMainMenuCompleteDelegateHandle);

Because it passes an unassigned DelegateHandle, the delegate assigned previously is not actually removed.

This does not crash itself, but the problem occurs if a player reconnects to a server again and disconnects again. The uncleaned handle causes a crash when the class tries to add the OnDestroyForMainMenuCompleteDelegateHandle again during the call to DestroyExistingSession_Impl. As the previous delegate was not removed, the call to AddDelegateInstance in DelegateSignatureImpl_Variadics.inl thows an error on the following check:

check(!DelegateInstanceInterface->IsSameFunction(*InDelegateInstance));

MOST LIKELY SOLUTION:

It looks like the issue would not occur if the handle addition above assigned the handle variable via an output variable argument to DestroyExistingSession_Impl instead of via the result of the call.

Namely instead of

FDelegateHandle UOnlineSessionClient::DestroyExistingSession_Impl(FName SessionName, FOnDestroySessionCompleteDelegate& Delegate)
{
...
		Result = SessionInt->AddOnDestroySessionCompleteDelegate_Handle(Delegate);
		SessionInt->DestroySession(SessionName);
...
	return Result;
}

doing

void UOnlineSessionClient::DestroyExistingSession_Impl(FName SessionName, FOnDestroySessionCompleteDelegate& Delegate, FDelegateHandle* DelegateHandleOut)
 {
 ...
         FDelegateHandle Result = SessionInt->AddOnDestroySessionCompleteDelegate_Handle(Delegate);
         if (DelegateHandleOut) *DelegateHandleOut = Result;
         
         SessionInt->DestroySession(SessionName);
 ...
 }

Hope that helps!

Hey Merlin-

Could you explain how you’re using the OnlineSessionClient class in your project? Is it possible to reproduce the crashes you mentioned in a new project and, if so, please list the steps you took that caused the crash. Addditionally, if you could post the callstack and log files from the crash it would give me a better understanding of what is happening.

Cheers

Doug Wilson

We are using a custom OnlineSubsystem implementation closely based on the Null implementation. From what I could tell the issue should occur even on a new game project using the Null subsytem as long as the game in question’s online code is set up to use the engine’s OnlineSessionClient (which it seems ours is).

Sadly I cannot devote additional time to looking into this as we have already worked around the issue by making the call to DestroySession mentioned above return it’s result asynchronously.

I posted the brief explanation above so you may investigate it as a potential bug should you choose to commit the time to it.