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

[Adhoc] Fix possible lock issue and Updated PdpStat #14060

Merged
merged 2 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions Core/HLE/proAdhoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1896,13 +1896,35 @@ uint16_t getLocalPort(int sock) {
return ntohs(localAddr.sin_port);
}

u_long getAvailToRecv(int sock) {
u_long getAvailToRecv(int sock, int udpBufferSize) {
u_long n = 0; // Typical MTU size is 1500
int err = -1;
#if defined(_WIN32) // May not be available on all platform
ioctlsocket(sock, FIONREAD, &n);
err = ioctlsocket(sock, FIONREAD, &n);
#else
ioctl(sock, FIONREAD, &n);
err = ioctl(sock, FIONREAD, &n);
#endif
if (err < 0)
return 0;

if (udpBufferSize > 0 && n > 0) {
// Cap number of bytes of full DGRAM message(s?) up to buffer size
static int lastUdpBufSize = 0;
static char* buf = NULL;
if (udpBufferSize > lastUdpBufSize) {
// Reusing temp buffer to prevent causing too many fragmentation due to repeated alloc -> free (was getting out of memory issue)
Copy link
Owner

Choose a reason for hiding this comment

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

if you were really running out of memory, you would have been leaking. I don't think fragmentation here would cause such serious issues..

Copy link
Collaborator Author

@anr2me anr2me Feb 4, 2021

Choose a reason for hiding this comment

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

It might not be fragmentation, i only gets out of memory (my monitor goes blank and a popup related to memory appeared) when debugging it through Visual Studio 2019, may be it's VS Debugger bug on memory allocation?
It's just a simple:

char *buf = (char*)malloc(size);
err = recvfrom(...);
free(buf);

So i can't really tell where the leaks came from, and i certainly don't want to get issue while debugging.

Copy link
Owner

Choose a reason for hiding this comment

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

That's really odd, something like that shouldn't run of RAM.

However, if realloc fixes it, let's go with that, I guess..

Copy link
Collaborator Author

@anr2me anr2me Feb 4, 2021

Choose a reason for hiding this comment

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

Hmm... i think we don't really need to provide a large buffer since we only need to retrieved the message size, and it seems with MSG_TRUNC we can provide small buffer and it will still return the actual message size (one full message)

char *tmp = (char*)realloc(buf, udpBufferSize);
if (tmp) {
buf = tmp;
lastUdpBufSize = udpBufferSize;
}
}
// Does each recv can only received one message?
err = recvfrom(sock, buf, udpBufferSize, MSG_PEEK | MSG_NOSIGNAL | MSG_TRUNC, NULL, NULL);
//free(buf); // Repeated alloc -> free seems to cause too many fragmentation and ended getting out of memory due to too many alloc -> free
if (err >= 0)
return (u_long)err;
}
return n;
}

Expand Down
4 changes: 3 additions & 1 deletion Core/HLE/proAdhoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -1271,8 +1271,10 @@ bool isPrivateIP(uint32_t ip);

/*
* Get Number of bytes available in buffer to be Received
* @param sock fd
* @param udpBufferSize (UDP only)
*/
u_long getAvailToRecv(int sock);
u_long getAvailToRecv(int sock, int udpBufferSize = 0);

/*
* Get UDP Socket Max Message Size
Expand Down
11 changes: 6 additions & 5 deletions Core/HLE/sceNetAdhoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2943,10 +2943,9 @@ static int sceNetAdhocGetPdpStat(u32 structSize, u32 structAddr) {
auto sock = adhocSockets[j];
if (sock != NULL && sock->type == SOCK_PDP) {
// Set available bytes to be received. With FIONREAD There might be ghosting 1 byte in recv buffer when remote peer's socket got closed (ie. Warriors Orochi 2) Attempting to recv this ghost 1 byte will result to socket error 10054 (may need to disable SIO_UDP_CONNRESET error)
sock->data.pdp.rcv_sb_cc = getAvailToRecv(sock->data.pdp.id);
// TODO: It seems real PSP respecting the socket buffer size arg, so we may need to cap the value to the buffer size arg since we use larger buffer, but may cause Warriors Orochi 2 to get slower performance.
// Note: Capping available data on UDP/PDP can causes data loss if the game tried to read the whole capped buffer size while containing partial/truncated message.
//sock->data.pdp.rcv_sb_cc = std::min(sock->data.pdp.rcv_sb_cc, (u32_le)sock->buffer_size);
// It seems real PSP respecting the socket buffer size arg, so we may need to cap the value up to the buffer size arg since we use larger buffer, for PDP/UDP the total size must not contains partial/truncated message to avoid data loss.
// TODO: We may need to manage PDP messages ourself by reading each msg 1-by-1 and moving it to our internal buffer(msg array) in order to calculate the correct messages size that can fit into buffer size when there are more than 1 messages in the recv buffer (simulate FIONREAD)
sock->data.pdp.rcv_sb_cc = getAvailToRecv(sock->data.pdp.id, sock->buffer_size);

// Copy Socket Data from Internal Memory
memcpy(&buf[i], &sock->data.pdp, sizeof(SceNetAdhocPdpStat));
Expand Down Expand Up @@ -4661,8 +4660,10 @@ int NetAdhocMatching_Start(int matchingId, int evthPri, int evthPartitionId, int
// Create PDP Socket
int sock = sceNetAdhocPdpCreate((const char*)&item->mac, static_cast<int>(item->port), item->rxbuflen, 0);
item->socket = sock;
if (sock < 1)
if (sock < 1) {
peerlock.unlock();
return hleLogError(SCENET, ERROR_NET_ADHOC_MATCHING_PORT_IN_USE, "adhoc matching port in use");
}

// Create & Start the Fake PSP Thread ("matching_ev%d" and "matching_io%d")
netAdhocValidateLoopMemory();
Expand Down