From f0825321d46366a169076bdbdc5a3172a9a94779 Mon Sep 17 00:00:00 2001 From: RadWolfie Date: Fri, 15 Jun 2018 17:23:17 -0500 Subject: [PATCH] Fixes of Cxbx's GUI mode to Debugger The following fixes contains: * Launch one Cxbx Debugger at a time. If one already exist, then it will close previous Cxbx Debugger and launch new debugger. * Close Cxbx Debugger when Cxbx is closed. * Also close Cxbx Debugger when xbe is closed. * Fixed another false positive for title is emulating when debugger is opened then close. * (Changed from no-risk to low-risk solution. Low-end computers may will have med-risk.) * This will only happen on start of emulating, during emulation has no risk for multi-xbe launches. * Use CreateProcess instead of ShellExecutable function. * Replaced: Using std string instead of fixed char array to prevent buffer overflow. * It will use more process Known issue: * Cxbx Debugger is not aware of multi-xbe launch. --- src/Cxbx/WndMain.cpp | 125 +++++++++++++++++++++++++++++++++---------- src/Cxbx/WndMain.h | 17 +++++- 2 files changed, 111 insertions(+), 31 deletions(-) diff --git a/src/Cxbx/WndMain.cpp b/src/Cxbx/WndMain.cpp index 42c52553b..a8c796475 100644 --- a/src/Cxbx/WndMain.cpp +++ b/src/Cxbx/WndMain.cpp @@ -42,6 +42,7 @@ #include "DlgXboxControllerPortMapping.h" #include "Common/XbePrinter.h" // For DumpInformation #include "CxbxKrnl/EmuShared.h" +#include "CxbxKrnl/CxbxKrnl.h" // For CxbxConvertArgToString #include "ResCxbx.h" #include "CxbxVersion.h" #include "Shlwapi.h" @@ -155,13 +156,16 @@ WndMain::WndMain(HINSTANCE x_hInstance) : m_Xbe(nullptr), m_bXbeChanged(false), m_bIsStarted(false), - m_hwndChild(NULL), + m_hwndChild(nullptr), m_KrnlDebug(DM_NONE), m_CxbxDebug(DM_NONE), m_FlagsLLE(0), m_StorageToggle(CXBX_DATA_APPDATA), m_StorageLocation(""), - m_dwRecentXbe(0) + m_dwRecentXbe(0), + m_hDebuggerProc(nullptr), + m_hDebuggerThread(nullptr), + m_hDebuggerMonitorThread(nullptr) { // initialize members { @@ -459,6 +463,9 @@ WndMain::~WndMain() } } + // Close opened debugger monitor if there is one + DebuggerMonitorClose(); + // cleanup allocations { delete m_Xbe; @@ -564,7 +571,8 @@ LRESULT CALLBACK WndMain::WndProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lP else { m_hwndChild = GetWindow(hwnd, GW_CHILD); } - CreateThread(NULL, NULL, CrashMonitorWrapper, (void*)this, NULL, NULL); // create the crash monitoring thread + HANDLE hThreadTemp = CreateThread(NULL, NULL, CrashMonitorWrapper, (void*)this, NULL, NULL); // create the crash monitoring thread + CloseHandle(hThreadTemp); } break; @@ -586,6 +594,7 @@ LRESULT CALLBACK WndMain::WndProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lP // NOTE: If anything need to set before kernel process start do anything, do it here. case ID_KRNL_IS_READY: { g_EmuShared->SetFlagsLLE(&m_FlagsLLE); + g_EmuShared->SetIsEmulating(true); // NOTE: Putting in here raise to low or medium risk due to debugger will launch itself. (Current workaround) g_EmuShared->SetIsReady(true); break; } @@ -2221,6 +2230,8 @@ void WndMain::CloseXbe() UpdateCaption(); RefreshMenus(); + DebuggerMonitorClose(); + // clear logo bitmap { uint32 v=0; @@ -2323,7 +2334,6 @@ void WndMain::SaveXbeAs() // start emulation void WndMain::StartEmulation(HWND hwndParent, DebuggerState LocalDebuggerState /*= debuggerOff*/) { - char szBuffer[MAX_PATH]; bool isEmulating = false; g_EmuShared->GetIsEmulating(&isEmulating); @@ -2334,8 +2344,6 @@ void WndMain::StartEmulation(HWND hwndParent, DebuggerState LocalDebuggerState / return; } - g_EmuShared->SetIsEmulating(true); - // register xbe path with emulator process g_EmuShared->SetXbePath(m_Xbe->m_szPath); @@ -2369,55 +2377,68 @@ void WndMain::StartEmulation(HWND hwndParent, DebuggerState LocalDebuggerState / // shell exe { - GetModuleFileName(NULL, szBuffer, MAX_PATH); - - char *spot = strrchr(szBuffer, '\\'); - if (spot != NULL) - *spot = '\0'; char szExeFileName[MAX_PATH]; GetModuleFileName(GetModuleHandle(NULL), szExeFileName, MAX_PATH); - char szArgsBuffer[4096]; - snprintf(szArgsBuffer, 4096, "/load \"%s\" %d %d \"%s\"", m_XbeFilename, (int)hwndParent, (int)m_KrnlDebug, m_KrnlDebugFilename); - bool AttachLocalDebugger = (LocalDebuggerState == debuggerOn); g_EmuShared->SetDebuggingFlag(&AttachLocalDebugger); - if (AttachLocalDebugger) - { + STARTUPINFO startupInfo = { 0 }; + PROCESS_INFORMATION processInfo = { 0 }; + char* szArgsBufferOutput; + size_t szSize; + + std::string szProcArgsBuffer; + XTL::CxbxConvertArgToString(szProcArgsBuffer, szExeFileName, m_XbeFilename, hwndParent, m_KrnlDebug, m_KrnlDebugFilename); + + if (AttachLocalDebugger) { + + // Check then close existing debugger monitor. + DebuggerMonitorClose(); + // TODO: Set a configuration variable for this. For now it will be within the same folder as Cxbx.exe - const char* szDebugger = "CxbxDebugger.exe"; + std::string szProcDbgArgsBuffer = "CxbxDebugger.exe " + szProcArgsBuffer; + szSize = szProcDbgArgsBuffer.size(); - char szDbgArgsBuffer[4096]; - snprintf(szDbgArgsBuffer, 4096, "%s %s", szExeFileName, szArgsBuffer); + szArgsBufferOutput = new char[szSize + 1]; + strncpy(szArgsBufferOutput, szProcDbgArgsBuffer.c_str(), szSize); + szArgsBufferOutput[szSize] = '\0'; - if ((int)ShellExecute(NULL, "open", szDebugger, szDbgArgsBuffer, szBuffer, SW_SHOWDEFAULT) <= 32) - { + if (CreateProcess(nullptr, szArgsBufferOutput, nullptr, nullptr, false, 0, nullptr, nullptr, &startupInfo, &processInfo) == 0) { MessageBox(m_hwnd, "Failed to start emulation with the debugger.\n\nYou will need to build CxbxDebugger manually.", "Cxbx-Reloaded", MB_ICONSTOP | MB_OK); printf("WndMain: %s debugger shell failed.\n", m_Xbe->m_szAsciiTitle); } - else - { + else { m_bIsStarted = true; + m_hDebuggerProc = processInfo.hProcess; + m_hDebuggerThread = processInfo.hThread; printf("WndMain: %s emulation started with debugger.\n", m_Xbe->m_szAsciiTitle); + m_hDebuggerMonitorThread = CreateThread(nullptr, 0, DebuggerMonitor, (void*)this, 0, nullptr); // create the debugger monitoring thread } } - else - { - if ((int)ShellExecute(NULL, "open", szExeFileName, szArgsBuffer, szBuffer, SW_SHOWDEFAULT) <= 32) - { + else { + szSize = szProcArgsBuffer.size(); + szArgsBufferOutput = new char[szSize + 1]; + strncpy(szArgsBufferOutput, szProcArgsBuffer.c_str(), szSize); + szArgsBufferOutput[szSize] = '\0'; + + if (CreateProcess(nullptr, szArgsBufferOutput, nullptr, nullptr, false, 0, nullptr, nullptr, &startupInfo, &processInfo) == 0) { MessageBox(m_hwnd, "Emulation failed.\n\n If this message repeats, the Xbe is not supported.", "Cxbx-Reloaded", MB_ICONSTOP | MB_OK); printf("WndMain: %s shell failed.\n", m_Xbe->m_szAsciiTitle); } - else - { + else { m_bIsStarted = true; + CloseHandle(processInfo.hProcess); + CloseHandle(processInfo.hThread); printf("WndMain: %s emulation started.\n", m_Xbe->m_szAsciiTitle); } } + + // Clean up + delete[] szArgsBufferOutput; } } @@ -2501,6 +2522,52 @@ void WndMain::CrashMonitor() RefreshMenus(); } +// monitor for Debugger to close then set as "available" (For limit to 1 debugger per Cxbx GUI.) +DWORD WINAPI WndMain::DebuggerMonitor(LPVOID lpVoid) +{ + CxbxSetThreadName("Cxbx Debugger Monitor"); + WndMain* pThis = static_cast(lpVoid); + + if (pThis->m_hDebuggerProc != nullptr) { + + // Peform a wait until Debugger is closed. + WaitForSingleObject(pThis->m_hDebuggerProc, INFINITE); + + if (pThis->m_hDebuggerProc != nullptr) { + CloseHandle(pThis->m_hDebuggerProc); + pThis->m_hDebuggerProc = nullptr; + } + + if (pThis->m_hDebuggerThread != nullptr) { + CloseHandle(pThis->m_hDebuggerProc); + pThis->m_hDebuggerProc = nullptr; + } + } + CloseHandle(pThis->m_hDebuggerMonitorThread); + pThis->m_hDebuggerMonitorThread = nullptr; + + return 0; +} +void WndMain::DebuggerMonitorClose() +{ + + if (m_hDebuggerProc != nullptr) { + HANDLE hDebuggerProcTemp = m_hDebuggerProc; + HANDLE hDebuggerThreadTemp = m_hDebuggerThread; + HANDLE hDebuggerMonitorThreadTemp = m_hDebuggerMonitorThread; + + // Set member to null pointer before terminate, this way debugger monitor thread will remain thread-safe. + m_hDebuggerProc = nullptr; + m_hDebuggerThread = nullptr; + + + TerminateProcess(hDebuggerProcTemp, EXIT_SUCCESS); + CloseHandle(hDebuggerThreadTemp); + + WaitForSingleObject(hDebuggerMonitorThreadTemp, INFINITE); + } +} + // draw Xbox LED bitmap void WndMain::DrawLedBitmap(HWND hwnd, bool bdefault) { diff --git a/src/Cxbx/WndMain.h b/src/Cxbx/WndMain.h index c3d64d14f..d3d3a4708 100644 --- a/src/Cxbx/WndMain.h +++ b/src/Cxbx/WndMain.h @@ -144,6 +144,16 @@ class WndMain : public Wnd // ****************************************************************** void CrashMonitor(); + // ****************************************************************** + // * Debugger monitoring function thread + // ****************************************************************** + static DWORD WINAPI DebuggerMonitor(LPVOID lpVoid); + + // ****************************************************************** + // * Close debugger monitoring function + // ****************************************************************** + void DebuggerMonitorClose(); + // ****************************************************************** // * clear registry values and keys // ****************************************************************** @@ -191,9 +201,12 @@ class WndMain : public Wnd char *m_XbeFilename; // ****************************************************************** - // * cached child window handle + // * cached window, process, and thread handle // ****************************************************************** - HWND m_hwndChild; + HWND m_hwndChild; + HANDLE m_hDebuggerProc; + HANDLE m_hDebuggerThread; + HANDLE m_hDebuggerMonitorThread; // ****************************************************************** // * Recent Xbe files