From: Mikhail Kurinnoi Date: Wed, 31 Aug 2022 12:29:32 +0000 (+0300) Subject: Add debuggee process stop at attach for CLI protocol. X-Git-Tag: accepted/tizen/unified/20220922.062128~4 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3c29b95d405ead9cb97656dc5e52fdc0faffdeb9;p=sdk%2Ftools%2Fnetcoredbg.git Add debuggee process stop at attach for CLI protocol. Fix CLI and MI/GDB protocols exit behavior in case debuggee process was attached, will detach now instead of terminate attached debuggee process. --- diff --git a/src/debugger/managedcallback.cpp b/src/debugger/managedcallback.cpp index 1f8b957..141703f 100644 --- a/src/debugger/managedcallback.cpp +++ b/src/debugger/managedcallback.cpp @@ -140,6 +140,12 @@ bool ManagedCallback::CallbacksWorkerException(ICorDebugAppDomain *pAppDomain, I return true; } +bool ManagedCallback::CallbacksWorkerCreateProcess() +{ + m_debugger.NotifyProcessCreated(); + return false; +} + void ManagedCallback::CallbacksWorker() { std::unique_lock lock(m_callbacksMutex); @@ -169,6 +175,9 @@ void ManagedCallback::CallbacksWorker() case CallbackQueueCall::Exception: m_stopEventInProcess = CallbacksWorkerException(c.iCorAppDomain, c.iCorThread, c.EventType, c.ExcModule); break; + case CallbackQueueCall::CreateProcess: + m_stopEventInProcess = CallbacksWorkerCreateProcess(); + break; default: // finish loop // called from destructor only, don't need call pop() @@ -531,10 +540,32 @@ HRESULT STDMETHODCALLTYPE ManagedCallback::EvalException(ICorDebugAppDomain *pAp // https://docs.microsoft.com/en-us/dotnet/framework/unmanaged-api/debugging/icordebugmanagedcallback-createprocess-method // Notifies the debugger when a process has been attached or started for the first time. +// Remarks +// This method is not called until the common language runtime is initialized. Most of the ICorDebug methods will return CORDBG_E_NOTREADY before the CreateProcess callback. HRESULT STDMETHODCALLTYPE ManagedCallback::CreateProcess(ICorDebugProcess *pProcess) { LogFuncEntry(); - m_debugger.NotifyProcessCreated(); + + // Important! Care about callback queue before NotifyProcessCreated() call. + // In case of `attach`, NotifyProcessCreated() call will notify debugger that debuggee process attached and debugger + // should stop debuggee process by dirrect `Pause()` call. From another side, callback queue have bunch of asynchronous + // added entries and, for example, `CreateThread()` could be called after this callback and broke our debugger logic. + ToRelease domains; + ICorDebugAppDomain *pAppDomain; + ULONG domainsFetched; + if (SUCCEEDED(pProcess->EnumerateAppDomains(&domains))) + { + // At this point we have only one domain for sure. + if (SUCCEEDED(domains->Next(1, &pAppDomain, &domainsFetched)) && domainsFetched == 1) + { + // Don't AddRef() here for pAppDomain! We get it with AddRef() from Next() and will release in m_callbacksQueue by ToRelease destructor. + return AddCallbackToQueue(pAppDomain, [&]() + { + m_callbacksQueue.emplace_back(CallbackQueueCall::CreateProcess, pAppDomain, nullptr, nullptr, STEP_NORMAL, ExceptionCallbackType::FIRST_CHANCE); + }); + } + } + return ContinueProcessWithCallbacksQueue(pProcess); } diff --git a/src/debugger/managedcallback.h b/src/debugger/managedcallback.h index e8147a2..2864bde 100644 --- a/src/debugger/managedcallback.h +++ b/src/debugger/managedcallback.h @@ -34,7 +34,8 @@ class ManagedCallback final : public ICorDebugManagedCallback, ICorDebugManagedC Breakpoint, StepComplete, Break, - Exception + Exception, + CreateProcess }; struct CallbackQueueEntry @@ -75,6 +76,7 @@ class ManagedCallback final : public ICorDebugManagedCallback, ICorDebugManagedC bool CallbacksWorkerStepComplete(ICorDebugAppDomain *pAppDomain, ICorDebugThread *pThread, CorDebugStepReason reason); bool CallbacksWorkerBreak(ICorDebugAppDomain *pAppDomain, ICorDebugThread *pThread); bool CallbacksWorkerException(ICorDebugAppDomain *pAppDomain, ICorDebugThread *pThread, ExceptionCallbackType eventType, const std::string &excModule); + bool CallbacksWorkerCreateProcess(); HRESULT AddCallbackToQueue(ICorDebugAppDomain *pAppDomain, std::function callback); bool HasQueuedCallbacks(ICorDebugProcess *pProcess); HRESULT ContinueAppDomainWithCallbacksQueue(ICorDebugAppDomain *pAppDomain); diff --git a/src/debugger/manageddebugger.cpp b/src/debugger/manageddebugger.cpp index d6e6178..67f8345 100644 --- a/src/debugger/manageddebugger.cpp +++ b/src/debugger/manageddebugger.cpp @@ -63,6 +63,7 @@ extern "C" const IID IID_IUnknown = { 0x00000000, 0x0000, 0x0000, {0xC0, 0x00, 0 namespace { + const auto startupWaitTimeout = std::chrono::milliseconds(5000); const std::string envDOTNET_STARTUP_HOOKS = "DOTNET_STARTUP_HOOKS"; #ifdef FEATURE_PAL @@ -117,8 +118,10 @@ namespace void ManagedDebugger::NotifyProcessCreated() { - std::lock_guard lock(m_processAttachedMutex); + std::unique_lock lock(m_processAttachedMutex); m_processAttachedState = ProcessAttachedState::Attached; + lock.unlock(); + m_processAttachedCV.notify_one(); } void ManagedDebugger::NotifyProcessExited() @@ -183,8 +186,6 @@ ManagedDebugger::ManagedDebugger() : m_justMyCode(true), m_stepFiltering(true), m_hotReload(false), - m_startupReady(false), - m_startupResult(S_OK), m_unregisterToken(nullptr), m_processId(0), m_ioredirect( @@ -400,19 +401,13 @@ VOID ManagedDebugger::StartupCallback(IUnknown *pCordb, PVOID parameter, HRESULT { ManagedDebugger *self = static_cast(parameter); - std::unique_lock lock(self->m_startupMutex); - - self->m_startupResult = FAILED(hr) ? hr : self->Startup(pCordb, self->m_processId); - self->m_startupReady = true; + self->Startup(pCordb, self->m_processId); if (self->m_unregisterToken) { self->m_dbgshim.UnregisterForRuntimeStartup(self->m_unregisterToken); self->m_unregisterToken = nullptr; } - - lock.unlock(); - self->m_startupCV.notify_one(); } // From dbgshim.cpp @@ -640,7 +635,6 @@ static void PrepareSystemEnvironmentArg(const std::map HRESULT ManagedDebugger::RunProcess(const std::string& fileExec, const std::vector& execArgs) { - static const auto startupCallbackWaitTimeout = std::chrono::milliseconds(5000); HRESULT Status; IfFailRet(CheckNoProcess()); @@ -652,7 +646,6 @@ HRESULT ManagedDebugger::RunProcess(const std::string& fileExec, const std::vect ss << " \"" << EscapeShellArg(arg) << "\""; } - m_startupReady = false; m_clrPath.clear(); HANDLE resumeHandle = 0; // Fake thread handle for the process resume @@ -689,26 +682,13 @@ HRESULT ManagedDebugger::RunProcess(const std::string& fileExec, const std::vect IfFailRet(m_dbgshim.ResumeProcess(resumeHandle)); m_dbgshim.CloseResumeHandle(resumeHandle); - // Wait for ManagedDebugger::StartupCallback to complete - - /// FIXME: if the process exits too soon the ManagedDebugger::StartupCallback() - /// is never called (bug in dbgshim?). - /// The workaround is to wait with timeout. - const auto now = std::chrono::system_clock::now(); - - std::unique_lock lock(m_startupMutex); - if (!m_startupCV.wait_until(lock, now + startupCallbackWaitTimeout, [this](){return m_startupReady;})) - { - // Timed out - m_dbgshim.UnregisterForRuntimeStartup(m_unregisterToken); - m_unregisterToken = nullptr; + std::unique_lock lockAttachedMutex(m_processAttachedMutex); + if (!m_processAttachedCV.wait_for(lockAttachedMutex, startupWaitTimeout, [this]{return m_processAttachedState == ProcessAttachedState::Attached;})) return E_FAIL; - } - if (SUCCEEDED(m_startupResult)) - m_sharedProtocol->EmitExecEvent(PID{m_processId}, fileExec); + m_sharedProtocol->EmitExecEvent(PID{m_processId}, fileExec); - return m_startupResult; + return S_OK; } HRESULT ManagedDebugger::CheckNoProcess() @@ -838,7 +818,13 @@ HRESULT ManagedDebugger::AttachToProcess(DWORD pid) IfFailRet(m_dbgshim.CreateDebuggingInterfaceFromVersionEx(CorDebugVersion_4_0, pBuffer, &pCordb)); m_unregisterToken = nullptr; - return Startup(pCordb, pid); + IfFailRet(Startup(pCordb, pid)); + + std::unique_lock lockAttachedMutex(m_processAttachedMutex); + if (!m_processAttachedCV.wait_for(lockAttachedMutex, startupWaitTimeout, [this]{return m_processAttachedState == ProcessAttachedState::Attached;})) + return E_FAIL; + + return S_OK; } HRESULT ManagedDebugger::GetExceptionInfo(ThreadId threadId, ExceptionInfo &exceptionInfo) diff --git a/src/debugger/manageddebugger.h b/src/debugger/manageddebugger.h index f9e489d..4a1670b 100644 --- a/src/debugger/manageddebugger.h +++ b/src/debugger/manageddebugger.h @@ -91,11 +91,6 @@ private: bool m_stepFiltering; bool m_hotReload; - std::mutex m_startupMutex; - std::condition_variable m_startupCV; - bool m_startupReady; - HRESULT m_startupResult; - PVOID m_unregisterToken; DWORD m_processId; std::string m_clrPath; diff --git a/src/main.cpp b/src/main.cpp index 31ec999..2fb589c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -496,6 +496,9 @@ int if (run || pidDebuggee) cliProtocol->SetRunningState(); + if (pidDebuggee != 0) + cliProtocol->Pause(); + // run commands passed in command line via '-ex' option cliProtocol->Source({initCommands}); } diff --git a/src/protocols/cliprotocol.cpp b/src/protocols/cliprotocol.cpp index 777d649..ce1701c 100644 --- a/src/protocols/cliprotocol.cpp +++ b/src/protocols/cliprotocol.cpp @@ -1566,7 +1566,7 @@ HRESULT CLIProtocol::doCommand(const std::vector // no mutex locking needed here m_exit = true; m_sources.reset(nullptr); - m_sharedDebugger->Disconnect(IDebugger::DisconnectAction::DisconnectTerminate); + m_sharedDebugger->Disconnect(); // Terminate debuggee process if debugger ran this process and detach in case debugger was attached to it. return S_OK; } @@ -1645,16 +1645,9 @@ HRESULT CLIProtocol::doCommand(const std::vectorConfigurationDone(); - if (SUCCEEDED(Status)) - { - output = "^running"; - - lock.lock(); - m_processStatus = Running; - m_state_cv.notify_all(); - } - return Status; + IfFailRet(m_sharedDebugger->ConfigurationDone()); + IfFailRet(m_sharedDebugger->Pause(ThreadId::AllThreads)); + return S_OK; } template <> @@ -2323,7 +2316,7 @@ void CLIProtocol::CommandLoop() printf("^exit\n"); - m_sharedDebugger->Disconnect(IDebugger::DisconnectAction::DisconnectTerminate); + m_sharedDebugger->Disconnect(); // Terminate debuggee process if debugger ran this process and detach in case debugger was attached to it. linenoiseHistorySave(HistoryFileName); linenoiseHistoryFree(); diff --git a/src/protocols/cliprotocol.h b/src/protocols/cliprotocol.h index cdc9698..367ff31 100644 --- a/src/protocols/cliprotocol.h +++ b/src/protocols/cliprotocol.h @@ -202,6 +202,9 @@ public: virtual ~LineReader() {} }; + // pause debugee execution + void Pause(); + private: // This function should be used by any others CLIProtocol's functions // to read input lines for interpreting (commands, etc...) @@ -228,9 +231,6 @@ private: static CLIProtocol* g_console_owner; static std::mutex g_console_mutex; // mutex which protect g_console_owner - // pause debugee execution - void Pause(); - // process Ctrl-C events static void interruptHandler(); diff --git a/src/protocols/miprotocol.cpp b/src/protocols/miprotocol.cpp index f082ee8..003ba8d 100644 --- a/src/protocols/miprotocol.cpp +++ b/src/protocols/miprotocol.cpp @@ -784,7 +784,7 @@ static HRESULT HandleCommand(std::shared_ptr &sharedDebugger, Breakpo return variablesHandle.DeleteVar(args.at(0)); }}, { "gdb-exit", [&](const std::vector &args, std::string &output) -> HRESULT { - sharedDebugger->Disconnect(IDebugger::DisconnectAction::DisconnectTerminate); + sharedDebugger->Disconnect(); // Terminate debuggee process if debugger ran this process and detach in case debugger was attached to it. return S_OK; }}, { "file-exec-and-symbols", [&](const std::vector &args, std::string &output) -> HRESULT { @@ -1048,7 +1048,7 @@ void MIProtocol::CommandLoop() } if (!m_exit) - m_sharedDebugger->Disconnect(IDebugger::DisconnectAction::DisconnectTerminate); + m_sharedDebugger->Disconnect(); // Terminate debuggee process if debugger ran this process and detach in case debugger was attached to it. Printf("%s^exit\n", token.c_str()); Printf("(gdb)\n");