Skip to content

Commit

Permalink
Separate each AdhocMatching callback's buffer, since callback aren't …
Browse files Browse the repository at this point in the history
…immediately executed thus using shared memory address may corrupt previous data
  • Loading branch information
anr2me authored and hrydgard committed Jul 20, 2020
1 parent 4f3b473 commit da5b54e
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 36 deletions.
36 changes: 21 additions & 15 deletions Core/HLE/proAdhoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
}

Expand Down
6 changes: 4 additions & 2 deletions Core/HLE/proAdhoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
37 changes: 19 additions & 18 deletions Core/HLE/sceNetAdhoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -3491,22 +3491,24 @@ void __NetTriggerCallbacks()

for (std::map<int, AdhocctlHandler>::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 &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]);
// 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);
Expand Down Expand Up @@ -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
Expand All @@ -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;

Expand Down Expand Up @@ -4867,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;
}
Expand Down
6 changes: 5 additions & 1 deletion Core/HLE/sceNetAdhoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down

0 comments on commit da5b54e

Please sign in to comment.