From da5b54e630ba98b3837666156dc213c156db69cc Mon Sep 17 00:00:00 2001 From: ANR2ME Date: Wed, 13 Nov 2019 12:56:29 +0700 Subject: [PATCH] Separate each AdhocMatching callback's buffer, since callback aren't immediately executed thus using shared memory address may corrupt previous data --- Core/HLE/proAdhoc.cpp | 36 +++++++++++++++++++++--------------- Core/HLE/proAdhoc.h | 6 ++++-- Core/HLE/sceNetAdhoc.cpp | 37 +++++++++++++++++++------------------ Core/HLE/sceNetAdhoc.h | 6 +++++- 4 files changed, 49 insertions(+), 36 deletions(-) diff --git a/Core/HLE/proAdhoc.cpp b/Core/HLE/proAdhoc.cpp index a7619de4a9..2a770c2d7f 100644 --- a/Core/HLE/proAdhoc.cpp +++ b/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,7 +951,7 @@ 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); @@ -957,33 +959,37 @@ void notifyAdhocctlHandlers(u32 flag, u32 error) { // 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 } diff --git a/Core/HLE/proAdhoc.h b/Core/HLE/proAdhoc.h index 21470fc7a3..239f275e80 100644 --- a/Core/HLE/proAdhoc.h +++ b/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 @@ public: //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; diff --git a/Core/HLE/sceNetAdhoc.cpp b/Core/HLE/sceNetAdhoc.cpp index 13edc82328..52dbcd9d8f 100644 --- a/Core/HLE/sceNetAdhoc.cpp +++ b/Core/HLE/sceNetAdhoc.cpp @@ -50,7 +50,7 @@ SceUID threadAdhocID; std::mutex adhocEvtMtx; std::vector> adhocctlEvents; -std::vector matchingEvents; +std::vector 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 adhocGuard(adhocEvtMtx); matchingEvents.push_back(ArgsPtr); } @@ -2667,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; @@ -3491,22 +3491,24 @@ void __NetTriggerCallbacks() for (std::map::iterator it = adhocctlHandlers.begin(); it != adhocctlHandlers.end(); ++it) { args[2] = it->second.argument; - __KernelSwitchToThread(threadAdhocID, "AdhocctlHandler Mipscall"); - __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 ¶m : matchingEvents) { - u32_le *args = (u32_le *) param; + u32_le *args = (u32_le*)¶m; AfterMatchingMipsCall *after = (AfterMatchingMipsCall *) __KernelCreateAction(actionAfterMatchingMipsCall); - after->SetContextID(args[0], args[1]); - // Need to make sure currentThread is AdhocMatching's eventThread before calling the callback - __KernelSwitchToThread(threadAdhocID, "AdhocMatchingEvent Mipscall"); - __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); @@ -4775,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 @@ -4790,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; @@ -4867,6 +4868,9 @@ 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; } @@ -4874,9 +4878,6 @@ int matchingEventThread(int matchingId) // 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; } diff --git a/Core/HLE/sceNetAdhoc.h b/Core/HLE/sceNetAdhoc.h index 829c973ebd..0f30ebe6b8 100644 --- a/Core/HLE/sceNetAdhoc.h +++ b/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);