From bd36fdda0e49aaba57057b75c482a045dc3d5034 Mon Sep 17 00:00:00 2001 From: ANR2ME Date: Sat, 18 Jul 2020 08:19:43 +0700 Subject: [PATCH] Do UPnP stuff on it's own thread to prevent causing stutters/lags during multiplayer game due to blocking socket behavior on miniUPnP --- Core/HLE/proAdhocServer.cpp | 4 +- Core/HLE/sceNet.cpp | 19 +++----- Core/HLE/sceNetAdhoc.cpp | 6 +-- Core/Util/PortManager.cpp | 97 +++++++++++++++++++++++++++++++++++-- Core/Util/PortManager.h | 25 ++++++++++ 5 files changed, 130 insertions(+), 21 deletions(-) diff --git a/Core/HLE/proAdhocServer.cpp b/Core/HLE/proAdhocServer.cpp index 7d663ba1d4..85d4f2d56a 100644 --- a/Core/HLE/proAdhocServer.cpp +++ b/Core/HLE/proAdhocServer.cpp @@ -1685,13 +1685,13 @@ int proAdhocServerThread(int port) // (int argc, char * argv[]) INFO_LOG(SCENET, "AdhocServer: Listening for Connections on TCP Port %u", port); //SERVER_PORT // Port forward - g_PortManager.Add(IP_PROTOCOL_TCP, port); + UPnP_Add(IP_PROTOCOL_TCP, port); // g_PortManager.Add(IP_PROTOCOL_TCP, port); // Enter Server Loop result = server_loop(server); // Remove Port mapping - g_PortManager.Remove(IP_PROTOCOL_TCP, port); + UPnP_Remove(IP_PROTOCOL_TCP, port); // g_PortManager.Remove(IP_PROTOCOL_TCP, port); // Notify User INFO_LOG(SCENET, "AdhocServer: Shutdown complete"); diff --git a/Core/HLE/sceNet.cpp b/Core/HLE/sceNet.cpp index b001caeeed..fd1f4830ec 100644 --- a/Core/HLE/sceNet.cpp +++ b/Core/HLE/sceNet.cpp @@ -126,13 +126,10 @@ void __NetInit() { char tmpmac[18]; SceNetEtherAddr mac; getLocalMac(&mac); - INFO_LOG(SCENET, "LocalHost IP will be %s [MAC: %s]", inet_ntoa(((sockaddr_in*)&LocalhostIP)->sin_addr), mac2str(&mac, tmpmac)); - - // Only initialize when UPnP is enabled since it takes a few seconds to detect UPnP device (may affect people who don't have UPnP device) - if (g_Config.bEnableUPnP) { - // TODO: May be we should initialize & cleanup somewhere else than here for PortManager to be used as general purpose for whatever port forwarding PPSSPP needed - g_PortManager.Initialize(); - } + INFO_LOG(SCENET, "LocalHost IP will be %s [%s]", inet_ntoa(((sockaddr_in*)&LocalhostIP)->sin_addr), mac2str(&mac, tmpmac)); + + // TODO: May be we should initialize & cleanup somewhere else than here for PortManager to be used as general purpose for whatever port forwarding PPSSPP needed + __UPnPInit(); __ResetInitNetLib(); } @@ -140,15 +137,11 @@ void __NetInit() { void __NetShutdown() { // Network Cleanup Net_Term(); - + __ResetInitNetLib(); // Since PortManager supposed to be general purpose for whatever port forwarding PPSSPP needed, may be we shouldn't clear & restore ports in here? it will be cleared and restored by PortManager's destructor when exiting PPSSPP anyway - if (g_PortManager.GetInitState() == UPNP_INITSTATE_DONE) { - g_PortManager.Clear(); - g_PortManager.Restore(); - g_PortManager.Terminate(); - } + __UPnPShutdown(); } static void __UpdateApctlHandlers(int oldState, int newState, int flag, int error) { diff --git a/Core/HLE/sceNetAdhoc.cpp b/Core/HLE/sceNetAdhoc.cpp index 40803220cd..95aa652051 100644 --- a/Core/HLE/sceNetAdhoc.cpp +++ b/Core/HLE/sceNetAdhoc.cpp @@ -384,7 +384,7 @@ static int sceNetAdhocPdpCreate(const char *mac, int port, int bufferSize, u32 u // Forward Port on Router //sceNetPortOpen("UDP", port); - g_PortManager.Add(IP_PROTOCOL_UDP, isOriPort ? port : port + portOffset, port + portOffset); + UPnP_Add(IP_PROTOCOL_UDP, isOriPort ? port : port + portOffset, port + portOffset); // g_PortManager.Add(IP_PROTOCOL_UDP, isOriPort ? port : port + portOffset, port + portOffset); // Success return i + 1; @@ -1992,7 +1992,7 @@ static int sceNetAdhocPtpOpen(const char *srcmac, int sport, const char *dstmac, // Add Port Forward to Router. We may not even need to forward this local port, since PtpOpen usually have port 0 (any port) as source port and followed by PtpConnect (which mean acting as Client), right? //sceNetPortOpen("TCP", sport); if (!isClient) - g_PortManager.Add(IP_PROTOCOL_TCP, isOriPort ? sport : sport + portOffset, sport + portOffset); + UPnP_Add(IP_PROTOCOL_TCP, isOriPort ? sport : sport + portOffset, sport + portOffset); // g_PortManager.Add(IP_PROTOCOL_TCP, isOriPort ? sport : sport + portOffset, sport + portOffset); // Return PTP Socket Pointer return i + 1; @@ -2525,7 +2525,7 @@ static int sceNetAdhocPtpListen(const char *srcmac, int sport, int bufsize, int // Add Port Forward to Router //sceNetPortOpen("TCP", sport); - g_PortManager.Add(IP_PROTOCOL_TCP, isOriPort ? sport : sport + portOffset, sport + portOffset); + UPnP_Add(IP_PROTOCOL_TCP, isOriPort ? sport : sport + portOffset, sport + portOffset); // g_PortManager.Add(IP_PROTOCOL_TCP, isOriPort ? sport : sport + portOffset, sport + portOffset); // Return PTP Socket Pointer return i + 1; diff --git a/Core/Util/PortManager.cpp b/Core/Util/PortManager.cpp index 34819be5bc..d91e4fb428 100644 --- a/Core/Util/PortManager.cpp +++ b/Core/Util/PortManager.cpp @@ -32,11 +32,18 @@ #include #include "Core/Util/PortManager.h" #include +#include #include "i18n/i18n.h" #include "net/resolve.h" +#include "thread/threadutil.h" +#include "base/timeutil.h" PortManager g_PortManager; +bool upnpServiceRunning = false; +std::thread upnpServiceThread; +std::recursive_mutex upnpLock; +std::deque upnpReqs; PortManager::PortManager(): urls(0), @@ -170,7 +177,7 @@ bool PortManager::Initialize(const unsigned int timeout) { ERROR_LOG(SCENET, "PortManager - upnpDiscover failed (error: %i) or No UPnP device detected", error); auto n = GetI18NCategory("Networking"); - host->NotifyUserMessage(n->T("Unable to find UPnP device"), 6.0f, 0x0000ff); + host->NotifyUserMessage(n->T("Unable to find UPnP device"), 2.0f, 0x0000ff); m_InitState = UPNP_INITSTATE_NONE; return false; } @@ -216,7 +223,7 @@ bool PortManager::Add(const char* protocol, unsigned short port, unsigned short ERROR_LOG(SCENET, "PortManager - AddPortMapping failed (error: %i)", r); if (r == UPNPCOMMAND_HTTP_ERROR) { auto n = GetI18NCategory("Networking"); - host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff); + host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 2.0f, 0x0000ff); Terminate(); // Most of the time errors occurred because the router is no longer reachable (ie. changed networks) so we should invalidate the state to prevent further lags due to timeouts return false; } @@ -244,7 +251,7 @@ bool PortManager::Remove(const char* protocol, unsigned short port) { ERROR_LOG(SCENET, "PortManager - DeletePortMapping failed (error: %i)", r); if (r == UPNPCOMMAND_HTTP_ERROR) { auto n = GetI18NCategory("Networking"); - host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff); + host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 2.0f, 0x0000ff); Terminate(); // Most of the time errors occurred because the router is no longer reachable (ie. changed networks) so we should invalidate the state to prevent further lags due to timeouts return false; } @@ -403,3 +410,87 @@ bool PortManager::RefreshPortList() { } while (r == 0); return true; } + +int upnpService(const unsigned int timeout) +{ + setCurrentThreadName("UPnPService"); + INFO_LOG(SCENET, "UPnPService: Begin of UPnPService Thread"); + + // Service Loop + while (upnpServiceRunning && coreState != CORE_POWERDOWN) { + // Attempts to reconnect if not connected yet or got disconnected + if (g_Config.bEnableUPnP && g_PortManager.GetInitState() == UPNP_INITSTATE_NONE) { + g_PortManager.Initialize(timeout); + } + + if (g_Config.bEnableUPnP && g_PortManager.GetInitState() == UPNP_INITSTATE_DONE && !upnpReqs.empty()) { + upnpLock.lock(); + UPnPArgs arg = upnpReqs.front(); + upnpLock.unlock(); + + bool ok = true; + switch (arg.cmd) { + case UPNP_CMD_ADD: + ok = g_PortManager.Add(arg.protocol.c_str(), arg.port, arg.intport); + break; + case UPNP_CMD_REMOVE: + ok = g_PortManager.Remove(arg.protocol.c_str(), arg.port); + break; + default: + break; + } + + // It's only considered failed when disconnected (should be retried when reconnected) + if (ok) { + upnpLock.lock(); + upnpReqs.pop_front(); + upnpLock.unlock(); + } + } + + // Sleep for 1ms for faster response + sleep_ms(1); + } + + // Cleaning up regardless of g_Config.bEnableUPnP to prevent lingering open ports on the router + if (g_PortManager.GetInitState() == UPNP_INITSTATE_DONE) { + g_PortManager.Clear(); + g_PortManager.Restore(); + g_PortManager.Terminate(); + } + + // Should we ingore any leftover UPnP requests? instead of processing it on the next game start + upnpLock.lock(); + upnpReqs.clear(); + upnpLock.unlock(); + + INFO_LOG(SCENET, "UPnPService: End of UPnPService Thread"); + return 0; +} + +void __UPnPInit(const unsigned int timeout) { + if (!upnpServiceRunning) { + upnpServiceRunning = true; + upnpServiceThread = std::thread(upnpService, timeout); + } +} + +void __UPnPShutdown() { + if (upnpServiceRunning) { + upnpServiceRunning = false; + if (upnpServiceThread.joinable()) { + upnpServiceThread.join(); + } + } +} + +void UPnP_Add(const char* protocol, unsigned short port, unsigned short intport) { + std::lock_guard upnpGuard(upnpLock); + upnpReqs.push_back({ UPNP_CMD_ADD, protocol, port, intport }); +} + +void UPnP_Remove(const char* protocol, unsigned short port) { + std::lock_guard upnpGuard(upnpLock); + upnpReqs.push_back({ UPNP_CMD_REMOVE, protocol, port, port }); +} + diff --git a/Core/Util/PortManager.h b/Core/Util/PortManager.h index 37040218dc..a4c7df6c27 100644 --- a/Core/Util/PortManager.h +++ b/Core/Util/PortManager.h @@ -33,12 +33,28 @@ #include #include +#ifdef _MSC_VER +#pragma pack(push,1) +#endif +typedef struct UPnPArgs { + int cmd; + std::string protocol; + unsigned short port; + unsigned short intport; +} PACK; +#ifdef _MSC_VER +#pragma pack(pop) +#endif + #define IP_PROTOCOL_TCP "TCP" #define IP_PROTOCOL_UDP "UDP" #define UPNP_INITSTATE_NONE 0 #define UPNP_INITSTATE_BUSY 1 #define UPNP_INITSTATE_DONE 2 +#define UPNP_CMD_ADD 0 +#define UPNP_CMD_REMOVE 1 + struct UPNPUrls; struct IGDdatas; @@ -98,3 +114,12 @@ protected: }; extern PortManager g_PortManager; + +void __UPnPInit(const unsigned int timeout = 2000); +void __UPnPShutdown(); + +// Add a port & protocol (TCP, UDP or vendor-defined) to map for forwarding (intport = 0 : same as [external] port) +void UPnP_Add(const char* protocol, unsigned short port, unsigned short intport = 0); + +// Remove a port mapping (external port) +void UPnP_Remove(const char* protocol, unsigned short port);