From 68fa15784c31cf3e3a9d0dd8432f2c5dc2fe53cb Mon Sep 17 00:00:00 2001 From: ANR2ME Date: Sun, 22 Mar 2020 03:08:01 +0700 Subject: [PATCH] Fixed UPnP port opened by PPSSPP detection, and Updated PortManager to be compliant to general purpose usage --- Core/HLE/sceNet.cpp | 8 +++++--- Core/Util/PortManager.cpp | 37 +++++++++++++++++++++---------------- Core/Util/PortManager.h | 8 ++++---- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/Core/HLE/sceNet.cpp b/Core/HLE/sceNet.cpp index 7a04f827cc..85077c1afa 100644 --- a/Core/HLE/sceNet.cpp +++ b/Core/HLE/sceNet.cpp @@ -130,7 +130,8 @@ void __NetInit() { // 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) { - g_PortManager.Init(); + // 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(); } __ResetInitNetLib(); @@ -148,10 +149,11 @@ void __NetShutdown() { __ResetInitNetLib(); - if (g_Config.bEnableUPnP) { + // Since PortManager supposed to be general purpose for whatever port forwarding PPSSPP needed, may be we shouldn't clear & restore ports in here? But currently there is an issue with UPnP functions used in PortManager's destructor :( + if (g_PortManager.GetInitState() == UPNP_INITSTATE_DONE) { g_PortManager.Clear(); g_PortManager.Restore(); - g_PortManager.Deinit(); + g_PortManager.Terminate(); } } diff --git a/Core/Util/PortManager.cpp b/Core/Util/PortManager.cpp index 8e25cdca37..0ba1f2a83b 100644 --- a/Core/Util/PortManager.cpp +++ b/Core/Util/PortManager.cpp @@ -46,12 +46,13 @@ PortManager::PortManager(): } PortManager::~PortManager() { + // FIXME: It seems using any UPnP functions in this destructor that gets triggered when exiting PPSSPP will resulting to UPNPCOMMAND_HTTP_ERROR :( Did miniUPnPc library already shutted down at this point? Clear(); Restore(); - Deinit(); + Terminate(); } -void PortManager::Deinit() { +void PortManager::Terminate() { if (urls) { FreeUPNPUrls(urls); free(urls); @@ -67,10 +68,10 @@ void PortManager::Deinit() { m_defaultDesc.clear(); m_leaseDuration.clear(); m_LocalPort = UPNP_LOCAL_PORT_ANY; - m_InitState = UPNP_INITSTATE_DONE; + m_InitState = UPNP_INITSTATE_NONE; } -bool PortManager::Init(const unsigned int timeout) { +bool PortManager::Initialize(const unsigned int timeout) { // Windows: Assuming WSAStartup already called beforehand struct UPNPDev* devlist; struct UPNPDev* dev; @@ -95,12 +96,11 @@ bool PortManager::Init(const unsigned int timeout) { WARN_LOG(SCENET, "PortManager - Initialization already in progress"); return false; } - // We should redetect UPnP just in case the player switched to a different network in the middle - /*case UPNP_INITSTATE_DONE: { + // Should we redetect UPnP? just in case the player switched to a different network in the middle + case UPNP_INITSTATE_DONE: { WARN_LOG(SCENET, "PortManager - Already Initialized"); - return false; + return true; } - */ default: break; } @@ -153,7 +153,7 @@ bool PortManager::Init(const unsigned int timeout) { // Using Game ID & Player Name as default description for mapping std::string gameID = g_paramSFO.GetDiscID(); - m_defaultDesc = "PPSSPP:" + gameID + ":" + g_Config.sNickName; + m_defaultDesc = "PPSSPP:" + gameID + ":" + g_Config.sNickName; // Some routers may automatically prefixed it with "UPnP:" freeUPNPDevlist(devlist); @@ -212,7 +212,7 @@ bool PortManager::Add(const char* protocol, unsigned short port, unsigned short if (r == UPNPCOMMAND_HTTP_ERROR) { auto n = GetI18NCategory("Networking"); host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff); - Deinit(); // 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 + 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; } } @@ -240,7 +240,7 @@ bool PortManager::Remove(const char* protocol, unsigned short port) { if (r == UPNPCOMMAND_HTTP_ERROR) { auto n = GetI18NCategory("Networking"); host->NotifyUserMessage(n->T("UPnP need to be reinitialized"), 6.0f, 0x0000ff); - Deinit(); // 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 + 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; } } @@ -316,8 +316,9 @@ bool PortManager::Clear() { do { snprintf(index, 6, "%d", i); rHost[0] = '\0'; enabled[0] = '\0'; - duration[0] = '\0'; desc[0] = '\0'; + duration[0] = '\0'; desc[0] = '\0'; protocol[0] = '\0'; extPort[0] = '\0'; intPort[0] = '\0'; intAddr[0] = '\0'; + // May gets UPNPCOMMAND_HTTP_ERROR when called while exiting PPSSPP (ie. used in destructor) r = UPNP_GetGenericPortMappingEntry(urls->controlURL, datas->first.servicetype, index, @@ -325,7 +326,7 @@ bool PortManager::Clear() { protocol, desc, enabled, rHost, duration); // Only removes port mappings created by PPSSPP for current LAN IP - if (r == 0 && intAddr == m_lanip && std::strncmp(desc, "PPSSPP", 6) == 0) { + if (r == 0 && intAddr == m_lanip && std::string(desc).find("PPSSPP:") != std::string::npos) { int r2 = UPNP_DeletePortMapping(urls->controlURL, datas->first.servicetype, extPort, protocol, rHost); if (r2 != 0) { @@ -371,7 +372,7 @@ bool PortManager::RefreshPortList() { do { snprintf(index, 6, "%d", i); rHost[0] = '\0'; enabled[0] = '\0'; - duration[0] = '\0'; desc[0] = '\0'; + duration[0] = '\0'; desc[0] = '\0'; protocol[0] = '\0'; extPort[0] = '\0'; intPort[0] = '\0'; intAddr[0] = '\0'; r = UPNP_GetGenericPortMappingEntry(urls->controlURL, datas->first.servicetype, @@ -380,13 +381,17 @@ bool PortManager::RefreshPortList() { protocol, desc, enabled, rHost, duration); if (r == 0) { + std::string desc_str = std::string(desc); + // Some router might prefix the description with "UPnP:" so we may need to truncate it to prevent it from getting multiple prefix when restored later + if (desc_str.find("UPnP:") == 0) + desc_str = desc_str.substr(5); // Only include port mappings created by PPSSPP for current LAN IP - if (intAddr == m_lanip && std::strncmp(desc, "PPSSPP", 6) == 0) { + if (intAddr == m_lanip && desc_str.find("PPSSPP:") != std::string::npos) { m_portList.push_back({ extPort, protocol }); } // Port mappings belong to others that might be taken by PPSSPP later else { - m_otherPortList.push_back({ false, protocol, extPort, intPort, intAddr, rHost, desc, duration, enabled }); + m_otherPortList.push_back({ false, protocol, extPort, intPort, intAddr, rHost, desc_str, duration, enabled }); } } i++; diff --git a/Core/Util/PortManager.h b/Core/Util/PortManager.h index 85a285a5eb..37040218dc 100644 --- a/Core/Util/PortManager.h +++ b/Core/Util/PortManager.h @@ -61,10 +61,10 @@ public: // Initialize UPnP // timeout: milliseconds to wait for a router to respond (default = 2000 ms) - bool Init(const unsigned int timeout = 2000); + bool Initialize(const unsigned int timeout = 2000); // Uninitialize/Reset the state - void Deinit(); + void Terminate(); // Get UPnP Initialization status int GetInitState(); @@ -75,13 +75,13 @@ public: // Remove a port mapping (external port) bool Remove(const char* protocol, unsigned short port); - // Removes any lingering mapped ports created by PPSSPP from previous crashes + // Removes any lingering mapped ports created by PPSSPP (including from previous crashes) bool Clear(); // Restore ports mapped by others that were taken by PPSSPP, better used after Clear() bool Restore(); - // Get port lists mapped by PPSSPP for current LAN IP & other's applications + // Retrieves port lists mapped by PPSSPP for current LAN IP & other's applications bool RefreshPortList(); protected: