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

Socket: Abort pending ops on close #8389

Merged
merged 1 commit into from Apr 22, 2020
Merged

Conversation

sepalani
Copy link
Contributor

@sepalani sepalani commented Oct 6, 2019

It seems there was a network regression, I'm investigating to find the root cause of it.

Meanwhile, here is a quick fix which prevent closesocket to hang forever and instead return -1 on close.

Do not merge.

@leoetlino leoetlino added the WIP / do not merge Work in progress (do not merge) label Mar 15, 2020
@sepalani sepalani changed the title [WIP] Fix network regression [WIP] Socket: Abort pending ops on close Apr 21, 2020
@sepalani
Copy link
Contributor Author

Details regarding the issue can be found here: https://dolp.in/i11867/6
It seems to fix Ubisoft and Activision games having issues to go online.

I still need to write a hardware test to check what does the Wii since the error I returned is a blind guess. I also haven't tested for regressions, ATM.

@sepalani
Copy link
Contributor Author

According to this hardware test, the Wii returns SO_ENOTCONN when recvfrom is interrupted via close.
wii-soread-close.zip

@sepalani sepalani changed the title [WIP] Socket: Abort pending ops on close Socket: Abort pending ops on close Apr 21, 2020
@@ -170,6 +170,12 @@ s32 WiiSocket::CloseFd()
ReturnValue = WiiSockMan::GetNetErrorCode(EITHER(WSAENOTSOCK, EBADF), "CloseFd", false);
}
fd = -1;
auto it = pending_sockops.begin();
while (it != pending_sockops.end())
Copy link
Member

Choose a reason for hiding this comment

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

for (auto it = pending_sockops.begin(); it != pending_sockops.end(); )

(I wouldn't move it = pending_sockops.erase(it); though, as it's less readable IMO)

@JMC47
Copy link
Contributor

JMC47 commented Apr 22, 2020

With Leoetlino's approval, I'm willing to merge this once you're done with it. Just ping me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP / do not merge Work in progress (do not merge)
3 participants