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 1 commit into from Apr 22, 2020


Copy link

@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 label Mar 15, 2020
@sepalani sepalani changed the title [WIP] Fix network regression [WIP] Socket: Abort pending ops on close Apr 21, 2020
Copy link
Contributor Author

@sepalani sepalani commented Apr 21, 2020

Details regarding the issue can be found here:
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.

Copy link
Contributor Author

@sepalani sepalani commented Apr 21, 2020

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

@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

@leoetlino leoetlino Apr 21, 2020

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)

Copy link

@JMC47 JMC47 commented Apr 22, 2020

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

@leoetlino leoetlino merged commit 2673280 into dolphin-emu:master Apr 22, 2020
10 checks passed
@sepalani sepalani deleted the fix-so branch Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
WIP / do not merge
3 participants