Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENetUtil: Add check for valid socket in SendPacket(). #11526

Merged
merged 1 commit into from Feb 10, 2023

Conversation

AdmiralCurtiss
Copy link
Contributor

I got a crash report on Discord yesterday that I traced back to here.

Unfortunately this isn't a real fix, more like a bandaid. I'm guessing what actually happens is that m_server in NetPlayClient becomes null through a disconnect but something is still trying to send stuff afterwards, but the stack in the crashdump I have looks borked so I'm not actually sure.

enetcrash
enetcrash2

@@ -39,6 +39,12 @@ int ENET_CALLBACK InterceptCallback(ENetHost* host, ENetEvent* event)

bool SendPacket(ENetPeer* socket, const sf::Packet& packet, u8 channel_id)
{
if (!socket)
{
ERROR_LOG_FMT(NETPLAY, "No socket to send to given.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ERROR_LOG_FMT(NETPLAY, "No socket to send to given.");
ERROR_LOG_FMT(NETPLAY, "No socket to send to, given.");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion makes no sense.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree, without the comma, it seems like you are trying to send a socket to given.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean but that's not how English grammar works. There's probably a better way to phrase this that's clearer.

@AdmiralCurtiss
Copy link
Contributor Author

Anyone opposed to merging this? It's not really a fix to the issue but it still prevents a potential crash.

@AdmiralCurtiss AdmiralCurtiss merged commit fbb3db7 into dolphin-emu:master Feb 10, 2023
14 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the netplay-crash branch February 10, 2023 20:24
@AdmiralCurtiss
Copy link
Contributor Author

I merged this so the user on Discord that this report is from can report back whether this helps them in any way, and maybe provide more info about the actual underlying issue now that Dolphin doesn't crash when this happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants