From: Mikhail Kurinnoi Date: Wed, 20 Jul 2022 12:55:33 +0000 (+0300) Subject: Improve debuggee process pause routine. X-Git-Tag: submit/tizen/20220804.224219~7 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9f6ec54c07b07fb2d66f5c8c65c1bf247ff37a35;p=sdk%2Ftools%2Fnetcoredbg.git Improve debuggee process pause routine. Add thread ID support to VSCode protocol "pause" command. --- diff --git a/src/debugger/managedcallback.cpp b/src/debugger/managedcallback.cpp index 4ff0b29..5e0511d 100644 --- a/src/debugger/managedcallback.cpp +++ b/src/debugger/managedcallback.cpp @@ -26,6 +26,8 @@ #include "interfaces/iprotocol.h" #include "utils/utf.h" +#include + namespace netcoredbg { @@ -275,19 +277,42 @@ HRESULT ManagedCallback::Continue(ICorDebugProcess *pProcess) return S_OK; } -HRESULT ManagedCallback::Pause(ICorDebugProcess *pProcess) +// Caller should care about m_callbacksMutex. +// Check stop status and stop, if need. +// Return S_FALSE in case already was stopped, S_OK in case stopped by this call. +static HRESULT InternalStop(ICorDebugProcess *pProcess, bool &stopEventInProcess) +{ + if (stopEventInProcess) + return S_FALSE; // Already stopped. + + HRESULT Status; + IfFailRet(pProcess->Stop(0)); + stopEventInProcess = true; + return S_OK; +} + +// Analog of "pProcess->Stop(0)" call that also care about callbacks. +HRESULT ManagedCallback::Stop(ICorDebugProcess *pProcess) { std::unique_lock lock(m_callbacksMutex); + // DO NOT reset steppers here, this is "pProcess->Stop(0)" like call, that care about callbacks. + return InternalStop(pProcess, m_stopEventInProcess); +} - if (m_stopEventInProcess) - return S_OK; +// Stop process and set last stopped thread. If `lastStoppedThread` not passed value from protocol, find best thread. +HRESULT ManagedCallback::Pause(ICorDebugProcess *pProcess, ThreadId lastStoppedThread) +{ + // Must be real thread ID or ThreadId::AllThreads. + if (!lastStoppedThread) + return E_INVALIDARG; + + std::unique_lock lock(m_callbacksMutex); // Note, in case Stop() failed, no stop event will be emitted, don't set m_stopEventInProcess to "true" in this case. HRESULT Status; - if (FAILED(Status = pProcess->Stop(0))) - return Status; - - m_stopEventInProcess = true; + IfFailRet(InternalStop(pProcess, m_stopEventInProcess)); + if (Status == S_FALSE) // Already stopped. + return S_OK; // Same logic as provide vsdbg in case of pause during stepping. m_debugger.m_uniqueSteppers->DisableAllSteppers(pProcess); @@ -297,47 +322,64 @@ HRESULT ManagedCallback::Pause(ICorDebugProcess *pProcess) std::vector threads; m_debugger.GetThreads(threads); - ThreadId lastStoppedId = m_debugger.GetLastStoppedThreadId(); - - // Reorder threads so that last stopped thread is checked first - for (size_t i = 0; i < threads.size(); ++i) + // In case `lastStoppedThread` provided, just check that we really have it. + if (lastStoppedThread != ThreadId::AllThreads) { - if (threads[i].id == lastStoppedId) + // In case VSCode protocol, user provide "pause" thread id. + if (std::find_if(threads.begin(), threads.end(), [&](Thread t){ return t.id == lastStoppedThread; }) != threads.end()) { - std::swap(threads[0], threads[i]); - break; + // VSCode protocol event must provide thread only (VSCode count on this), even if this thread don't have user code. + m_debugger.SetLastStoppedThreadId(lastStoppedThread); + m_debugger.m_sharedProtocol->EmitStoppedEvent(StoppedEvent(StopPause, lastStoppedThread)); + m_debugger.m_ioredirect.async_cancel(); + return S_OK; } } - - // Now get stack trace for each thread and find a frame with valid source location. - for (const Thread& thread : threads) + else { - int totalFrames = 0; - std::vector stackFrames; + // MI and CLI protocols provide ThreadId::AllThreads as lastStoppedThread, this protocols require thread and frame with user code. + // Note, MIEngine (MI/GDB) require frame connected to user source or it will crash Visual Studio. + + ThreadId lastStoppedId = m_debugger.GetLastStoppedThreadId(); - if (FAILED(m_debugger.GetStackTrace(thread.id, FrameLevel(0), 0, stackFrames, totalFrames))) - continue; + // Reorder threads so that last stopped thread is checked first + for (size_t i = 0; i < threads.size(); ++i) + { + if (threads[i].id == lastStoppedId) + { + std::swap(threads[0], threads[i]); + break; + } + } - for (const StackFrame& stackFrame : stackFrames) + // Now get stack trace for each thread and find a frame with valid source location. + for (const Thread& thread : threads) { - if (stackFrame.source.IsNull()) + int totalFrames = 0; + std::vector stackFrames; + + if (FAILED(m_debugger.GetStackTrace(thread.id, FrameLevel(0), 0, stackFrames, totalFrames))) continue; - StoppedEvent event(StopPause, thread.id); - event.frame = stackFrame; - m_debugger.SetLastStoppedThreadId(thread.id); - m_debugger.m_sharedProtocol->EmitStoppedEvent(event); - m_debugger.m_ioredirect.async_cancel(); - return S_OK; + for (const StackFrame& stackFrame : stackFrames) + { + if (stackFrame.source.IsNull()) + continue; + + StoppedEvent event(StopPause, thread.id); + event.frame = stackFrame; + m_debugger.SetLastStoppedThreadId(thread.id); + m_debugger.m_sharedProtocol->EmitStoppedEvent(event); + m_debugger.m_ioredirect.async_cancel(); + return S_OK; + } } } - assert(threads.size() > 0); - // Event must provide thread (VSCode and MI/GDB protocols count on this), even if this thread don't have user code. - // Note, provide thread without user code also legit for this event. - m_debugger.m_sharedProtocol->EmitStoppedEvent(StoppedEvent(StopPause, threads[0].id)); - m_debugger.m_ioredirect.async_cancel(); - return S_OK; + // Fatal error during stop, just fail Pause request and don't stop process. + m_stopEventInProcess = false; + IfFailRet(pProcess->Continue(0)); + return E_FAIL; } ManagedCallback::~ManagedCallback() diff --git a/src/debugger/managedcallback.h b/src/debugger/managedcallback.h index bfc45bd..e8147a2 100644 --- a/src/debugger/managedcallback.h +++ b/src/debugger/managedcallback.h @@ -87,10 +87,13 @@ public: ~ManagedCallback(); ULONG GetRefCount(); - // Called from ManagedDebugger by protocol request (Continue/StepCommand/Pause). + // Called from ManagedDebugger by protocol request (Continue/Pause). bool IsRunning(); HRESULT Continue(ICorDebugProcess *pProcess); - HRESULT Pause(ICorDebugProcess *pProcess); + // Stop process and set last stopped thread. If `lastStoppedThread` not passed value from protocol, find best thread. + HRESULT Pause(ICorDebugProcess *pProcess, ThreadId lastStoppedThread); + // Analog of "pProcess->Stop(0)" call that also care about callbacks. + HRESULT Stop(ICorDebugProcess *pProcess); // IUnknown diff --git a/src/debugger/manageddebugger.cpp b/src/debugger/manageddebugger.cpp index 164b71c..1d02346 100644 --- a/src/debugger/manageddebugger.cpp +++ b/src/debugger/manageddebugger.cpp @@ -363,7 +363,7 @@ HRESULT ManagedDebugger::Continue(ThreadId threadId) return Status; } -HRESULT ManagedDebugger::Pause() +HRESULT ManagedDebugger::Pause(ThreadId lastStoppedThread) { LogFuncEntry(); @@ -371,7 +371,7 @@ HRESULT ManagedDebugger::Pause() HRESULT Status; IfFailRet(CheckDebugProcess(m_iCorProcess, m_processAttachedMutex, m_processAttachedState)); - return m_managedCallback->Pause(m_iCorProcess); + return m_managedCallback->Pause(m_iCorProcess, lastStoppedThread); } HRESULT ManagedDebugger::GetThreads(std::vector &threads) @@ -1326,16 +1326,15 @@ HRESULT ManagedDebugger::HotReloadApplyDeltas(const std::string &dllFileName, co // Deltas can be applied only on stopped debuggee process. For Hot Reload scenario we temporary stop it and continue after deltas applied. HRESULT Status; - BOOL procRunning = FALSE; - IfFailRet(m_iCorProcess->IsRunning(&procRunning)); - if (procRunning == TRUE) - IfFailRet(m_iCorProcess->Stop(0)); + IfFailRet(m_managedCallback->Stop(m_iCorProcess)); + bool continueProcess = (Status == S_OK); // Was stopped by m_managedCallback->Stop() call. + IfFailRet(ApplyMetadataAndILDeltas(m_sharedModules.get(), dllFileName, deltaMD, deltaIL)); IfFailRet(ApplyPdbDeltaAndLineUpdates(dllFileName, deltaPDB, lineUpdates)); - if (procRunning == TRUE) - IfFailRet(m_iCorProcess->Continue(0)); + if (continueProcess) + IfFailRet(m_managedCallback->Continue(m_iCorProcess)); return S_OK; } diff --git a/src/debugger/manageddebugger.h b/src/debugger/manageddebugger.h index e9353c6..60a8b60 100644 --- a/src/debugger/manageddebugger.h +++ b/src/debugger/manageddebugger.h @@ -147,7 +147,7 @@ public: ThreadId GetLastStoppedThreadId() override; void InvalidateLastStoppedThreadId(); HRESULT Continue(ThreadId threadId) override; - HRESULT Pause() override; + HRESULT Pause(ThreadId lastStoppedThread) override; HRESULT GetThreads(std::vector &threads) override; HRESULT SetLineBreakpoints(const std::string& filename, const std::vector &lineBreakpoints, std::vector &breakpoints) override; HRESULT SetFuncBreakpoints(const std::vector &funcBreakpoints, std::vector &breakpoints) override; diff --git a/src/interfaces/idebugger.h b/src/interfaces/idebugger.h index 5ae51e3..515caaf 100644 --- a/src/interfaces/idebugger.h +++ b/src/interfaces/idebugger.h @@ -83,7 +83,7 @@ public: virtual HRESULT Disconnect(DisconnectAction action = DisconnectDefault) = 0; virtual ThreadId GetLastStoppedThreadId() = 0; virtual HRESULT Continue(ThreadId threadId) = 0; - virtual HRESULT Pause() = 0; + virtual HRESULT Pause(ThreadId lastStoppedThread) = 0; virtual HRESULT GetThreads(std::vector &threads) = 0; virtual HRESULT SetLineBreakpoints(const std::string& filename, const std::vector &lineBreakpoints, std::vector &breakpoints) = 0; virtual HRESULT SetFuncBreakpoints(const std::vector &funcBreakpoints, std::vector &breakpoints) = 0; diff --git a/src/protocols/cliprotocol.cpp b/src/protocols/cliprotocol.cpp index 2c03b05..7c47ff3 100644 --- a/src/protocols/cliprotocol.cpp +++ b/src/protocols/cliprotocol.cpp @@ -1552,7 +1552,7 @@ HRESULT CLIProtocol::doCommand(const std::vectorPause()); + IfFailRet(m_sharedDebugger->Pause(ThreadId::AllThreads)); output = "^stopped"; return S_OK; } @@ -2535,7 +2535,7 @@ void CLIProtocol::Pause() if (m_processStatus == Running) { lock.unlock(); - m_sharedDebugger->Pause(); + m_sharedDebugger->Pause(ThreadId::AllThreads); } } diff --git a/src/protocols/miprotocol.cpp b/src/protocols/miprotocol.cpp index 5386c24..5245189 100644 --- a/src/protocols/miprotocol.cpp +++ b/src/protocols/miprotocol.cpp @@ -704,7 +704,7 @@ HRESULT MIProtocol::HandleCommand(const std::string& command, const std::vector< } }, { "exec-interrupt", [this](const std::vector &, std::string &output){ HRESULT Status; - IfFailRet(m_sharedDebugger->Pause()); + IfFailRet(m_sharedDebugger->Pause(ThreadId::AllThreads)); output = "^done"; return S_OK; } }, diff --git a/src/protocols/vscodeprotocol.cpp b/src/protocols/vscodeprotocol.cpp index 277a9b8..4b8aa5b 100644 --- a/src/protocols/vscodeprotocol.cpp +++ b/src/protocols/vscodeprotocol.cpp @@ -635,8 +635,9 @@ static HRESULT HandleCommand(std::shared_ptr &sharedDebugger, std::st return sharedDebugger->Continue(threadId); } }, { "pause", [&](const json &arguments, json &body){ - // Ignore `threadId` argument, since only pause for all threads are supported now. - return sharedDebugger->Pause(); + ThreadId threadId{int(arguments.at("threadId"))}; + body["threadId"] = int(threadId); + return sharedDebugger->Pause(threadId); } }, { "next", [&](const json &arguments, json &body){ return sharedDebugger->StepCommand(ThreadId{int(arguments.at("threadId"))}, IDebugger::StepType::STEP_OVER);