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

@@ -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");

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) {

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;

Core/Util/PortManager.cpp

@@ -32,11 +32,18 @@
 #include <Core/ELF/ParamSFO.h>
 #include "Core/Util/PortManager.h"
 #include <Common/Log.h>
+#include <thread>
 #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<UPnPArgs> 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<std::recursive_mutex> upnpGuard(upnpLock);
+	upnpReqs.push_back({ UPNP_CMD_ADD, protocol, port, intport });
+}
+
+void UPnP_Remove(const char* protocol, unsigned short port) {
+	std::lock_guard<std::recursive_mutex> upnpGuard(upnpLock);
+	upnpReqs.push_back({ UPNP_CMD_REMOVE, protocol, port, port });
+}
+

Core/Util/PortManager.h

@@ -33,12 +33,28 @@
 #include <string>
 #include <deque>
 
+#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);