From d5e265a501231dd6f3147be11c3de8bcd6e645c0 Mon Sep 17 00:00:00 2001 From: Duncan Ogilvie Date: Tue, 16 Mar 2021 01:09:45 +0100 Subject: [PATCH] Fix a race condition when spamming restarts --- src/dbg/commands/cmd-debug-control.cpp | 11 +++--- src/dbg/debugger.cpp | 48 +++++++++++++++----------- src/dbg/debugger.h | 12 ++++--- 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/dbg/commands/cmd-debug-control.cpp b/src/dbg/commands/cmd-debug-control.cpp index 5e71a26a..4077c095 100644 --- a/src/dbg/commands/cmd-debug-control.cpp +++ b/src/dbg/commands/cmd-debug-control.cpp @@ -133,14 +133,11 @@ bool cbDebugInit(int argc, char* argv[]) strcpy_s(currentfolder, arg3); static INIT_STRUCT init; - memset(&init, 0, sizeof(INIT_STRUCT)); init.exe = arg1; init.commandline = commandline; if(*currentfolder) init.currentfolder = currentfolder; - - hDebugLoopThread = CreateThread(nullptr, 0, threadDebugLoop, &init, CREATE_SUSPENDED, nullptr); - ResumeThread(hDebugLoopThread); + dbgcreatedebugthread(&init); return true; } @@ -258,8 +255,10 @@ bool cbDebugAttach(int argc, char* argv[]) if(tid) dbgsetresumetid(tid); } - hDebugLoopThread = CreateThread(nullptr, 0, threadAttachLoop, (void*)pid, CREATE_SUSPENDED, nullptr); - ResumeThread(hDebugLoopThread); + static INIT_STRUCT init; + init.attach = true; + init.pid = (DWORD)pid; + dbgcreatedebugthread(&init); return true; } diff --git a/src/dbg/debugger.cpp b/src/dbg/debugger.cpp index 749a2a81..f64b44d0 100644 --- a/src/dbg/debugger.cpp +++ b/src/dbg/debugger.cpp @@ -2654,27 +2654,23 @@ static void* InitDLLDebugW(const wchar_t* szFileName, const wchar_t* szCommandLi return ReturnValue; } -static void debugLoopFunction(void* lpParameter, bool attach) +static void debugLoopFunction(INIT_STRUCT* init) { //initialize variables - bIsAttached = attach; + bIsAttached = init->attach; dbgsetskipexceptions(false); bFreezeStack = false; //prepare attach/createprocess - DWORD pid; - INIT_STRUCT* init; - if(attach) + if(init->attach) { gInitExe = StringUtils::Utf8ToUtf16(szDebuggeePath); - pid = DWORD(lpParameter); static PROCESS_INFORMATION pi_attached; memset(&pi_attached, 0, sizeof(pi_attached)); fdProcessInfo = &pi_attached; } else { - init = (INIT_STRUCT*)lpParameter; gInitExe = StringUtils::Utf8ToUtf16(init->exe); strcpy_s(szDebuggeePath, init->exe); } @@ -2690,7 +2686,7 @@ static void debugLoopFunction(void* lpParameter, bool attach) } DbSetPath(nullptr, szDebuggeePath); - if(!attach) + if(!init->attach) { // Load command line if it exists in DB DbLoad(DbLoadSaveType::CommandLine); @@ -2771,6 +2767,10 @@ static void debugLoopFunction(void* lpParameter, bool attach) gInitDir.clear(); } + // signal that fdProcessInfo has been set + SetEvent(init->event); + init->event = nullptr; + //set custom handlers SetCustomHandler(UE_CH_CREATEPROCESS, (void*)cbCreateProcess); SetCustomHandler(UE_CH_EXITPROCESS, (void*)cbExitProcess); @@ -2803,10 +2803,10 @@ static void debugLoopFunction(void* lpParameter, bool attach) plugincbcall(CB_INITDEBUG, &initInfo); //call plugin callback (attach) - if(attach) + if(init->attach) { PLUG_CB_ATTACH attachInfo; - attachInfo.dwProcessId = (DWORD)pid; + attachInfo.dwProcessId = init->pid; plugincbcall(CB_ATTACH, &attachInfo); } @@ -2814,9 +2814,9 @@ static void debugLoopFunction(void* lpParameter, bool attach) DbLoad(DbLoadSaveType::DebugData); //run debug loop (returns when process debugging is stopped) - if(attach) + if(init->attach) { - if(AttachDebugger(pid, true, fdProcessInfo, (void*)cbAttachDebugger) == false) + if(AttachDebugger(init->pid, true, fdProcessInfo, (void*)cbAttachDebugger) == false) { String error = stringformatinline(StringUtils::sprintf("{winerror@%d}", GetLastError())); dprintf(QT_TRANSLATE_NOOP("DBG", "Attach to process failed! GetLastError() = %s\n"), error.c_str()); @@ -2902,16 +2902,24 @@ void dbgsetforeground() SetForegroundWindow(GuiGetWindowHandle()); } -DWORD WINAPI threadDebugLoop(void* lpParameter) +void dbgcreatedebugthread(INIT_STRUCT* init) { - debugLoopFunction(lpParameter, false); - return 0; -} + auto event = init->event = CreateEventW(nullptr, false, false, nullptr); + hDebugLoopThread = CreateThread(nullptr, 0, [](LPVOID lpParameter) -> DWORD + { + auto init = (INIT_STRUCT*)lpParameter; + debugLoopFunction(init); -DWORD WINAPI threadAttachLoop(void* lpParameter) -{ - debugLoopFunction(lpParameter, true); - return 0; + // Set the event in case debugLoopFunction returned early to prevent a deadlock + if(init->event) + { + SetEvent(init->event); + init->event = nullptr; + } + return 0; + }, init, 0, nullptr); + WaitForSingleObject(event, INFINITE); + CloseHandle(event); } bool dbgrestartadmin() diff --git a/src/dbg/debugger.h b/src/dbg/debugger.h index 475b3651..ca3a68ce 100644 --- a/src/dbg/debugger.h +++ b/src/dbg/debugger.h @@ -27,9 +27,12 @@ enum class ExceptionHandledBy //structures struct INIT_STRUCT { - char* exe; - char* commandline; - char* currentfolder; + HANDLE event = nullptr; + char* exe = nullptr; + char* commandline = nullptr; + char* currentfolder = nullptr; + DWORD pid = 0; + bool attach = false; }; struct ExceptionRange @@ -103,6 +106,7 @@ void dbgtracebrowserneedsupdate(); bool dbgsetdllbreakpoint(const char* mod, DWORD type, bool singleshoot); bool dbgdeletedllbreakpoint(const char* mod, DWORD type); void dbgsetdebugflags(DWORD flags); +void dbgcreatedebugthread(INIT_STRUCT* init); void cbStep(); void cbRtrStep(); @@ -112,7 +116,6 @@ void cbMemoryBreakpoint(void* ExceptionAddress); void cbHardwareBreakpoint(void* ExceptionAddress); void cbUserBreakpoint(); void cbDebugLoadLibBPX(); -DWORD WINAPI threadDebugLoop(void* lpParameter); void cbTraceOverConditionalStep(); void cbTraceIntoConditionalStep(); void cbTraceIntoBeyondTraceRecordStep(); @@ -120,7 +123,6 @@ void cbTraceOverBeyondTraceRecordStep(); void cbTraceIntoIntoTraceRecordStep(); void cbTraceOverIntoTraceRecordStep(); void cbRunToUserCodeBreakpoint(void* ExceptionAddress); -DWORD WINAPI threadAttachLoop(void* lpParameter); bool cbSetModuleBreakpoints(const BREAKPOINT* bp); EXCEPTION_DEBUG_INFO & getLastExceptionInfo(); bool dbgrestartadmin();