Merge pull request #13173 from hrydgard/callback-stuff-ANR2ME

Make sure to run callback on AdhocThread (extracted from #13132).

Core/HLE/proAdhoc.cpp

@@ -935,11 +935,13 @@ void AfterMatchingMipsCall::run(MipsCall &call) {
 	}
 	context->eventlock->unlock();  //peerlock.unlock();
 	//call.setReturnValue(v0);
+	if (Memory::IsValidAddress(bufAddr)) userMemory.Free(bufAddr);
 	DEBUG_LOG(SCENET, "Leaving AfterMatchingMipsCall::run [ID=%i][Event=%d] [retV0: %08x]", context->id, EventID, currentMIPS->r[MIPS_REG_V0]);
 }
 
-void AfterMatchingMipsCall::SetContextID(u32 ContextID, u32 eventId) {
+void AfterMatchingMipsCall::SetContextID(u32 ContextID, u32 eventId, u32_le BufAddr) {
 	EventID = eventId;
+	bufAddr = BufAddr;
 	peerlock.lock();
 	context = findMatchingContext(ContextID);
 	peerlock.unlock();
@@ -949,41 +951,45 @@ void AfterMatchingMipsCall::SetContextID(u32 ContextID, u32 eventId) {
 void notifyAdhocctlHandlers(u32 flag, u32 error) {
 	__UpdateAdhocctlHandlers(flag, error);
 	// TODO: We should use after action instead of guessing the time like this
-	sleep_ms(20); // Ugly workaround to give time for the mips callback to fully executed, usually only need <16ms
+	//sleep_ms(20); // Ugly workaround to give time for the mips callback to fully executed, usually only need <16ms
 }
 
 // Matching callback is void function: typedef void(*SceNetAdhocMatchingHandler)(int id, int event, SceNetEtherAddr * peer, int optlen, void * opt);
 // Important! The MIPS call need to be fully executed before the next MIPS call invoked, as the game (ie. DBZ Tag Team) may need to prepare something for the next callback event to use
 // Note: Must not lock peerlock within this function to prevent race-condition with other thread whos owning peerlock and trying to lock context->eventlock owned by this thread
 void notifyMatchingHandler(SceNetAdhocMatchingContext * context, ThreadMessage * msg, void * opt, u32 &bufAddr, u32 &bufLen, u32_le * args) {
 	//u32_le args[5] = { 0, 0, 0, 0, 0 };
-	if ((s32)bufLen < (msg->optlen + 8)) {
+	/*if ((s32)bufLen < (msg->optlen + 8)) {
 		bufLen = msg->optlen + 8;
 		if (Memory::IsValidAddress(bufAddr)) userMemory.Free(bufAddr);
-		bufAddr = userMemory.Alloc(bufLen);
+		bufAddr = userMemory.Alloc(bufLen); // Max bufLen should be context->rxbuflen
 		INFO_LOG(SCENET, "MatchingHandler: Alloc(%i -> %i) = %08x", msg->optlen + 8, bufLen, bufAddr);
-	}
+	}*/
+	MatchingArgs argsNew;
+	bufAddr = userMemory.Alloc(bufLen); // We will free this after returning from mipscall
 	u8 * optPtr = Memory::GetPointer(bufAddr);
 	memcpy(optPtr, &msg->mac, sizeof(msg->mac));
 	if (msg->optlen > 0) memcpy(optPtr + 8, opt, msg->optlen);
-	args[0] = context->id;
-	args[1] = msg->opcode;
-	args[2] = bufAddr; // PSP_GetScratchpadMemoryBase() + 0x6000; 
-	args[3] = msg->optlen;
-	args[4] = args[2] + 8;
-	args[5] = context->handler.entryPoint; //not part of callback argument, just borrowing a space to store callback address so i don't need to search the context first later
+	argsNew.data[0] = context->id;
+	argsNew.data[1] = msg->opcode;
+	argsNew.data[2] = bufAddr; // PSP_GetScratchpadMemoryBase() + 0x6000; 
+	argsNew.data[3] = msg->optlen;
+	argsNew.data[4] = argsNew.data[2] + 8; // OptData Addr
+	argsNew.data[5] = context->handler.entryPoint; //not part of callback argument, just borrowing a space to store callback address so i don't need to search the context first later
 
+	context->eventlock->lock();
 	context->IsMatchingInCB = true;
+	context->eventlock->unlock();
 	// ScheduleEvent_Threadsafe_Immediate seems to get mixed up with interrupt (returning from mipscall inside an interrupt) and getting invalid address before returning from interrupt
-	__UpdateMatchingHandler((u64) args);
+	__UpdateMatchingHandler(argsNew);
 
 	// Make sure MIPS call have been fully executed before the next notifyMatchingHandler
-	int count = 0;
-	while (/*(after != NULL) &&*/ IsMatchingInCallback(context) && (count < 250)) {
+	/*int count = 0;
+	while ( IsMatchingInCallback(context) && (count < 250)) {
 		sleep_ms(1);
 		count++;
 	}
-	if (count >= 250) ERROR_LOG(SCENET, "MatchingHandler: Callback Failed to Return within %dms!", count);
+	if (count >= 250) ERROR_LOG(SCENET, "MatchingHandler: Callback Failed to Return within %dms! [ID=%i][Opcode=%d][OptSize=%d][MAC=%012X]", count, context->id, msg->opcode, msg->optlen, htonl(*(u_long*)&msg->mac));*/
 	//sleep_ms(20); // Wait a little more (for context switching may be?) to prevent DBZ Tag Team from getting connection lost, but this will cause lags on Lord of Arcana
 }
 

Core/HLE/proAdhoc.h

@@ -443,7 +443,7 @@ typedef struct SceNetAdhocMatchingContext {
   SceNetAdhocMatchingHandler handler;
 
   // Event Handler Args
-  u32_le handlerArgs[6]; // actual arguments only 5, the 6th one is just for borrowing a space to store the callback address to use later
+  u32_le handlerArgs[6]; //MatchingArgs handlerArgs; // actual arguments only 5, the 6th one is just for borrowing a space to store the callback address to use later
   //SceNetAdhocMatchingHandlerArgs handlerArgs;
 
   // Hello Data Length
@@ -776,11 +776,13 @@ class AfterMatchingMipsCall : public PSPAction {
 		//context = NULL;
 	}
 	void run(MipsCall &call) override;
-	void SetContextID(u32 ContextID, u32 eventId);
+	void SetContextID(u32 ContextID, u32 eventId, u32_le BufAddr);
+	void SetContext(SceNetAdhocMatchingContext* Context, u32 eventId, u32_le BufAddr) { context = Context; EventID = eventId; bufAddr = BufAddr; }
 
 private:
 	u32 EventID = 0;
 	SceNetAdhocMatchingContext *context = nullptr;
+	u32_le bufAddr = 0;
 };
 
 extern int actionAfterMatchingMipsCall;

Core/HLE/sceNetAdhoc.cpp

@@ -50,7 +50,7 @@ SceUID threadAdhocID;
 
 std::mutex adhocEvtMtx;
 std::vector<std::pair<u32, u32>> adhocctlEvents;
-std::vector<u64> matchingEvents;
+std::vector<MatchingArgs> matchingEvents;
 u32 dummyThreadHackAddr = 0;
 u32_le dummyThreadCode[3];
 
@@ -124,7 +124,7 @@ void __UpdateAdhocctlHandlers(u32 flag, u32 error) {
 }
 
 // TODO: MipsCall needs to be called from it's own PSP Thread instead of from any random PSP Thread
-void __UpdateMatchingHandler(u64 ArgsPtr) {
+void __UpdateMatchingHandler(MatchingArgs ArgsPtr) {
 	std::lock_guard<std::mutex> adhocGuard(adhocEvtMtx);
 	matchingEvents.push_back(ArgsPtr);
 }
@@ -168,15 +168,6 @@ u32 sceNetAdhocInit() {
 		// Library initialized
 		netAdhocInited = true;
 
-		// Create fake PSP Thread for callback
-		// TODO: Should use a separated threads for friendFinder, matchingEvent, and matchingInput and created on AdhocctlInit & AdhocMatchingStart instead of here
-		#define PSP_THREAD_ATTR_KERNEL 0x00001000 // PSP_THREAD_ATTR_KERNEL is located in sceKernelThread.cpp instead of sceKernelThread.h :(
-		// TODO: This should probably be a user thread, but maybe from sceNetAdhocctlInit?
-		threadAdhocID = __KernelCreateThread("AdhocThread", __KernelGetCurThreadModuleId(), dummyThreadHackAddr, 0x10, 0x1000, PSP_THREAD_ATTR_KERNEL, 0, true);
-		if (threadAdhocID > 0) {
-			__KernelStartThread(threadAdhocID, 0, 0);
-		}
-
 		// Return Success
 		return hleLogSuccessInfoI(SCENET, 0, "at %08x", currentMIPS->pc);
 	}
@@ -189,9 +180,20 @@ static u32 sceNetAdhocctlInit(int stackSize, int prio, u32 productAddr) {
 
 	if (netAdhocctlInited)
 		return ERROR_NET_ADHOCCTL_ALREADY_INITIALIZED;
+
+	// Create fake PSP Thread for callback
+	// TODO: Should use a separated threads for friendFinder, matchingEvent, and matchingInput and created on AdhocctlInit & AdhocMatchingStart instead of here
+#define PSP_THREAD_ATTR_KERNEL 0x00001000 // Using constants instead of numbers for readability reason, since PSP_THREAD_ATTR_KERNEL/USER is located in sceKernelThread.cpp instead of sceKernelThread.h
+#define PSP_THREAD_ATTR_USER 0x80000000
+
+	threadAdhocID = __KernelCreateThread("AdhocThread", __KernelGetCurThreadModuleId(), dummyThreadHackAddr, prio, stackSize, PSP_THREAD_ATTR_USER, 0, false);
+	if (threadAdhocID > 0) {
+		__KernelStartThread(threadAdhocID, 0, 0);
+	}
 
 	if(g_Config.bEnableWlan) {
 		if (initNetwork((SceNetAdhocctlAdhocId *)Memory::GetPointer(productAddr)) == 0) {
+			// TODO: Merging friendFinder (real) thread to AdhocThread (fake) thread on PSP side
 			if (!friendFinderRunning) {
 				friendFinderRunning = true;
 				friendFinderThread = std::thread(friendFinder);
@@ -1138,6 +1140,12 @@ int sceNetAdhocctlTerm() {
 		// Free stuff here
 		closesocket(metasocket);
 		metasocket = (int)INVALID_SOCKET;
+		// Delete fake PSP Thread
+		if (threadAdhocID != 0) {
+			__KernelStopThread(threadAdhocID, SCE_KERNEL_ERROR_THREAD_TERMINATED, "AdhocThread stopped");
+			__KernelDeleteThread(threadAdhocID, SCE_KERNEL_ERROR_THREAD_TERMINATED, "AdhocThread deleted");
+			threadAdhocID = 0;
+		}
 /*#ifdef _MSC_VER
 		WSACleanup(); // Might be better to call WSAStartup/WSACleanup from sceNetInit/sceNetTerm isn't? since it's the first/last network function being used, even better to put it in __NetInit/__NetShutdown as it's only called once
 #endif*/
@@ -1426,12 +1434,6 @@ int sceNetAdhocTerm() {
 
 	// Library is initialized
 	if (netAdhocInited) {
-		// Delete fake PSP Thread
-		if (threadAdhocID != 0) {
-			__KernelStopThread(threadAdhocID, SCE_KERNEL_ERROR_THREAD_TERMINATED, "AdhocThread stopped");
-			__KernelDeleteThread(threadAdhocID, SCE_KERNEL_ERROR_THREAD_TERMINATED, "AdhocThread deleted");
-		}
-
 		// Delete PDP Sockets
 		deleteAllPDP();
 
@@ -2665,7 +2667,7 @@ static int sceNetAdhocMatchingCreate(int mode, int maxnum, int port, int rxbufle
 								context->hello_int = hello_int; // client might set this to 0
 								if (keepalive_int < 1) context->keepalive_int = PSP_ADHOCCTL_PING_TIMEOUT; else context->keepalive_int = keepalive_int; // client might set this to 0
 								context->keepalivecounter = init_count; // used to multiply keepalive_int as timeout
-								context->timeout = (keepalive_int * init_count);
+								context->timeout = ((u64_le)keepalive_int * (u64_le)init_count);
 								if (context->timeout < 5000000) context->timeout = 5000000; // For internet play we need higher timeout than what the game wanted
 								context->handler = handler;
 
@@ -3489,19 +3491,24 @@ void __NetTriggerCallbacks()
 
 			for (std::map<int, AdhocctlHandler>::iterator it = adhocctlHandlers.begin(); it != adhocctlHandlers.end(); ++it) {
 				args[2] = it->second.argument;
-				__KernelDirectMipsCall(it->second.entryPoint, NULL, args, 3, true);
+				__KernelSwitchToThread(threadAdhocID, "AdhocctlHandler Mipscall"); // it's not guaranteed to switch successfully tho
+				//while (__KernelInCallback()) sleep_ms(1);
+				__KernelDirectMipsCall(it->second.entryPoint, NULL, args, 3, false);
 			}
 		}
-		adhocctlEvents.clear();
+		adhocctlEvents.clear(); // We should only clear this after making sure all callbacks are placed on the right thread tho
 
 		for (auto &param : matchingEvents)
 		{
-			u32_le *args = (u32_le *) param;
+			u32_le *args = (u32_le*)&param;
 			AfterMatchingMipsCall *after = (AfterMatchingMipsCall *) __KernelCreateAction(actionAfterMatchingMipsCall);
-			after->SetContextID(args[0], args[1]);
-			__KernelDirectMipsCall(args[5], after, args, 5, true);
+			after->SetContextID(args[0], args[1], args[2]);
+			// Need to make sure currentThread is AdhocMatching's eventThread before calling the callback, and not in the middle of callback
+			__KernelSwitchToThread(threadAdhocID, "AdhocMatchingEvent Mipscall"); // it's not guaranteed to switch successfully tho
+			//while (__KernelInCallback()) sleep_ms(1);
+			__KernelDirectMipsCall(args[5], after, args, 5, false);
 		}
-		matchingEvents.clear();
+		matchingEvents.clear(); // We should only clear this after making sure all callbacks are placed on the right thread tho
 	}
 	//magically make this work
 	hleDelayResult(0, "Prevent Adhoc thread from blocking", 1000);
@@ -4770,9 +4777,6 @@ void actOnByePacket(SceNetAdhocMatchingContext * context, SceNetEtherAddr * send
 */
 int matchingEventThread(int matchingId) 
 {
-	u32 bufLen = 0;
-	u32 bufAddr = 0;
-
 	// Multithreading Lock
 	peerlock.lock();
 	// Cast Context
@@ -4785,6 +4789,8 @@ int matchingEventThread(int matchingId)
 
 	// Run while needed...
 	if (context != NULL) {
+		u32 bufLen = context->rxbuflen; //0;
+		u32 bufAddr = 0; //userMemory.Alloc(bufLen);
 		//static u32_le args[5] = { 0, 0, 0, 0, 0 }; // Need to be global/static so it can be accessed from a different thread
 		u32_le * args = context->handlerArgs;
 
@@ -4862,16 +4868,16 @@ int matchingEventThread(int matchingId)
 			context->eventlock->unlock();
 		}
 
+		// Free memory
+		//if (Memory::IsValidAddress(bufAddr)) userMemory.Free(bufAddr);
+
 		// Delete Pointer Reference (and notify caller about finished cleanup)
 		//context->eventThread = NULL;
 	}
 
 	// Log Shutdown
 	INFO_LOG(SCENET, "EventLoop: End of EventLoop[%i] Thread", matchingId);
 
-	// Free memory
-	if (Memory::IsValidAddress(bufAddr)) userMemory.Free(bufAddr);
-
 	// Return Zero to shut up Compiler (never reached anyway)
 	return 0;
 }

Core/HLE/sceNetAdhoc.h

@@ -17,6 +17,10 @@
 
 #pragma once
 
+typedef struct MatchingArgs {
+	u32_le data[6]; //ContextID, Opcode, bufAddr[ to MAC], OptLen, OptAddr[, EntryPoint]
+} PACK;
+
 class PointerWrap;
 
 void Register_sceNetAdhoc();
@@ -25,7 +29,7 @@ void __NetAdhocInit();
 void __NetAdhocShutdown();
 void __NetAdhocDoState(PointerWrap &p);
 void __UpdateAdhocctlHandlers(u32 flags, u32 error);
-void __UpdateMatchingHandler(u64 params);
+void __UpdateMatchingHandler(MatchingArgs params);
 
 // I have to call this from netdialog
 int sceNetAdhocctlCreate(const char * groupName);