ShuriZma
/
Cxbx-Reloaded
mirror of https://github.com/Cxbx-Reloaded/Cxbx-Reloaded.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove hStartedEvent 

It doesn't seem to be useful anymore (does not protect any thread-unsafe
initialization), and it was encouraging the OS scheduler to keep executing
the child thread before returning the parent thread from PsCreateSystemThreadEx

Test case: The Warriors and its race condition on event creation
(child thread waits on the event parent thread creates AFTER spawning
the child thread)
This commit is contained in:
Silent 2020-10-19 21:49:46 +02:00
parent 6f27d335f7
commit b04980a150
No known key found for this signature in database
GPG Key ID: AE53149BB0C45AF1
1 changed files with 14 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
src/core/kernel/exports/EmuKrnlPs.cpp
View File

@ -55,7 +55,6 @@ typedef struct _PCSTProxyParam
IN PVOID StartContext;
IN PVOID SystemRoutine;
IN BOOL StartSuspended;
IN HANDLE hStartedEvent;
}
PCSTProxyParam;
@ -69,8 +68,7 @@ void LOG_PCSTProxy
PVOID StartRoutine,
PVOID StartContext,
PVOID SystemRoutine,
BOOL StartSuspended,
HANDLE hStartedEvent
BOOL StartSuspended
)
{
LOG_FUNC_BEGIN
@ -78,7 +76,6 @@ void LOG_PCSTProxy
LOG_FUNC_ARG(StartContext)
LOG_FUNC_ARG(SystemRoutine)
LOG_FUNC_ARG(StartSuspended)
LOG_FUNC_ARG(hStartedEvent)
LOG_FUNC_END;
}
@ -105,40 +102,32 @@ static unsigned int WINAPI PCSTProxy
{
CxbxSetThreadName("PsCreateSystemThread Proxy");
PCSTProxyParam *iPCSTProxyParam = (PCSTProxyParam*)Parameter;
PCSTProxyParam *iPCSTProxyParam = static_cast<PCSTProxyParam*>(Parameter);
PVOID StartRoutine = iPCSTProxyParam->StartRoutine;
PVOID StartContext = iPCSTProxyParam->StartContext;
PVOID SystemRoutine = iPCSTProxyParam->SystemRoutine;
BOOL StartSuspended = iPCSTProxyParam->StartSuspended;
HANDLE hStartedEvent = iPCSTProxyParam->hStartedEvent;
// Once deleted, unable to directly access iPCSTProxyParam in remainder of function.
free(iPCSTProxyParam);
// Copy params to the stack so they can be freed
PCSTProxyParam params = *iPCSTProxyParam;
delete iPCSTProxyParam;
LOG_PCSTProxy(
StartRoutine,
StartContext,
SystemRoutine,
StartSuspended,
hStartedEvent);
params.StartRoutine,
params.StartContext,
params.SystemRoutine,
params.StartSuspended);
// Do minimal thread initialization
InitXboxThread(g_CPUXbox);
SetEvent(hStartedEvent);
if (StartSuspended == TRUE) {
if (params.StartSuspended == TRUE) {
SuspendThread(GetCurrentThread());
}
auto routine = (xbox::PKSYSTEM_ROUTINE)SystemRoutine;
auto routine = (xbox::PKSYSTEM_ROUTINE)params.SystemRoutine;
// Debugging notice : When the below line shows up with an Exception dialog and a
// message like: "Exception thrown at 0x00026190 in cxbx.exe: 0xC0000005: Access
// violation reading location 0xFD001804.", then this is AS-DESIGNED behaviour!
// (To avoid repetitions, uncheck "Break when this exception type is thrown").
routine(xbox::PKSTART_ROUTINE(StartRoutine), StartContext);
routine(xbox::PKSTART_ROUTINE(params.StartRoutine), params.StartContext);
// This will also handle thread notification :
LOG_TEST_CASE("Thread returned from SystemRoutine");
@ -245,22 +234,15 @@ XBSYSAPI EXPORTNUM(255) xbox::ntstatus_xt NTAPI xbox::PsCreateSystemThreadEx
// create thread, using our special proxy technique
{
DWORD dwThreadId = 0, dwThreadWait;
bool bWait = true;
HANDLE hStartedEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
if (hStartedEvent == NULL) {
std::string errorMessage = CxbxGetLastErrorString("PsCreateSystemThreadEx could not create PCSTProxyEvent");
CxbxKrnlCleanup(errorMessage.c_str());
}
DWORD dwThreadId = 0;
// PCSTProxy is responsible for cleaning up this pointer
PCSTProxyParam *iPCSTProxyParam = (PCSTProxyParam*)malloc(sizeof(PCSTProxyParam));
PCSTProxyParam *iPCSTProxyParam = new PCSTProxyParam;
iPCSTProxyParam->StartRoutine = (PVOID)StartRoutine;
iPCSTProxyParam->StartContext = StartContext;
iPCSTProxyParam->SystemRoutine = (PVOID)SystemRoutine; // NULL, XapiThreadStartup or unknown?
iPCSTProxyParam->StartSuspended = CreateSuspended;
iPCSTProxyParam->hStartedEvent = hStartedEvent;
/*
// call thread notification routine(s)
@ -290,37 +272,6 @@ XBSYSAPI EXPORTNUM(255) xbox::ntstatus_xt NTAPI xbox::PsCreateSystemThreadEx
// Note : DO NOT use iPCSTProxyParam anymore, since ownership is transferred to the proxy (which frees it too)
EmuLog(LOG_LEVEL::DEBUG, "Waiting for Xbox proxy thread to start...");
while (bWait) {
dwThreadWait = WaitForSingleObject(hStartedEvent, INFINITE);
switch (dwThreadWait) {
case WAIT_TIMEOUT: { // The time-out interval elapsed, and the object's state is nonsignaled.
EmuLog(LOG_LEVEL::WARNING, "Timeout while waiting for Xbox proxy thread to start...\n");
bWait = false;
break;
}
case WAIT_OBJECT_0: { // The state of the specified object is signaled.
EmuLog(LOG_LEVEL::DEBUG, "Xbox proxy thread is started.");
bWait = false;
break;
}
default: {
if (dwThreadWait == WAIT_FAILED) // The function has failed
bWait = false;
std::string ErrorStr = CxbxGetLastErrorString("While waiting for Xbox proxy thread to start");
EmuLog(LOG_LEVEL::WARNING, "%s\n", ErrorStr.c_str());
break;
}
}
}
// Release the event
CloseHandle(hStartedEvent);
hStartedEvent = NULL;
// Log ThreadID identical to how GetCurrentThreadID() is rendered :
EmuLog(LOG_LEVEL::DEBUG, "Created Xbox proxy thread. Handle : 0x%X, ThreadId : [0x%.4X]", *ThreadHandle, dwThreadId);