From 8af3b9ca673178f2c67846b606fcfe62a44b4e2b Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 29 Mar 2013 01:18:12 +0000 Subject: [PATCH] Rationalize how we do Halt-ing before Destroy and Detach. llvm-svn: 178325 --- lldb/include/lldb/Target/Process.h | 8 ++ .../Process/gdb-remote/ProcessGDBRemote.cpp | 93 ------------ .../Process/gdb-remote/ProcessGDBRemote.h | 11 +- lldb/source/Target/Process.cpp | 135 ++++++++++++------ lldb/source/Target/StopInfo.cpp | 2 + 5 files changed, 107 insertions(+), 142 deletions(-) diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index b0dcec91871e..d604cc965d05 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -2350,6 +2350,9 @@ public: //------------------------------------------------------------------ virtual void DidDetach () {} + + virtual bool + DetachRequiresHalt() { return false; } //------------------------------------------------------------------ /// Called before sending a signal to a process. @@ -2387,6 +2390,9 @@ public: virtual void DidDestroy () { } + + virtual bool + DestroyRequiresHalt() { return true; } //------------------------------------------------------------------ @@ -3683,6 +3689,8 @@ protected: const char *bytes, size_t bytes_len); + Error + HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp); private: //------------------------------------------------------------------ diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 05c39f97fb53..008619511168 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1670,99 +1670,6 @@ ProcessGDBRemote::DoHalt (bool &caused_stop) return error; } -Error -ProcessGDBRemote::InterruptIfRunning -( - bool discard_thread_plans, - bool catch_stop_event, - EventSP &stop_event_sp -) -{ - Error error; - - Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); - - bool paused_private_state_thread = false; - const bool is_running = m_gdb_comm.IsRunning(); - if (log) - log->Printf ("ProcessGDBRemote::InterruptIfRunning(discard_thread_plans=%i, catch_stop_event=%i) is_running=%i", - discard_thread_plans, - catch_stop_event, - is_running); - - if (discard_thread_plans) - { - if (log) - log->Printf ("ProcessGDBRemote::InterruptIfRunning() discarding all thread plans"); - m_thread_list.DiscardThreadPlans(); - } - if (is_running) - { - if (catch_stop_event) - { - if (log) - log->Printf ("ProcessGDBRemote::InterruptIfRunning() pausing private state thread"); - PausePrivateStateThread(); - paused_private_state_thread = true; - } - - bool timed_out = false; - Mutex::Locker locker; - - if (!m_gdb_comm.SendInterrupt (locker, 1, timed_out)) - { - if (timed_out) - error.SetErrorString("timed out sending interrupt packet"); - else - error.SetErrorString("unknown error sending interrupt packet"); - if (paused_private_state_thread) - ResumePrivateStateThread(); - return error; - } - - if (catch_stop_event) - { - // LISTEN HERE - TimeValue timeout_time; - timeout_time = TimeValue::Now(); - timeout_time.OffsetWithSeconds(5); - StateType state = WaitForStateChangedEventsPrivate (&timeout_time, stop_event_sp); - - timed_out = state == eStateInvalid; - if (log) - log->Printf ("ProcessGDBRemote::InterruptIfRunning() catch stop event: state = %s, timed-out=%i", StateAsCString(state), timed_out); - - if (timed_out) - error.SetErrorString("unable to verify target stopped"); - } - - if (paused_private_state_thread) - { - if (log) - log->Printf ("ProcessGDBRemote::InterruptIfRunning() resuming private state thread"); - ResumePrivateStateThread(); - } - } - return error; -} - -Error -ProcessGDBRemote::WillDetach () -{ - Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); - if (log) - log->Printf ("ProcessGDBRemote::WillDetach()"); - - bool discard_thread_plans = true; - bool catch_stop_event = true; - EventSP event_sp; - - // FIXME: InterruptIfRunning should be done in the Process base class, or better still make Halt do what is - // needed. This shouldn't be a feature of a particular plugin. - - return InterruptIfRunning (discard_thread_plans, catch_stop_event, event_sp); -} - Error ProcessGDBRemote::DoDetach() { diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 957a84f2909d..dc4ec561b031 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -140,11 +140,11 @@ public: virtual lldb_private::Error DoHalt (bool &caused_stop); - virtual lldb_private::Error - WillDetach (); - virtual lldb_private::Error DoDetach (); + + virtual bool + DetachRequiresHalt() { return true; } virtual lldb_private::Error DoSignal (int signal); @@ -383,11 +383,6 @@ protected: const char *bytes, size_t bytes_len); - lldb_private::Error - InterruptIfRunning (bool discard_thread_plans, - bool catch_stop_event, - lldb::EventSP &stop_event_sp); - lldb_private::DynamicLoader * GetDynamicLoader (); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 4725fd0e03f2..6e96b04b95e9 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3297,14 +3297,86 @@ Process::Halt () return error; } +Error +Process::HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp) +{ + Error error; + if (m_public_state.GetValue() == eStateRunning) + { + Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS)); + if (log) + log->Printf("Process::Destroy() About to halt."); + error = Halt(); + if (error.Success()) + { + // Consume the halt event. + TimeValue timeout (TimeValue::Now()); + timeout.OffsetWithSeconds(1); + StateType state = WaitForProcessToStop (&timeout, &exit_event_sp); + + // If the process exited while we were waiting for it to stop, put the exited event into + // the shared pointer passed in and return. Our caller doesn't need to do anything else, since + // they don't have a process anymore... + + if (state == eStateExited || m_private_state.GetValue() == eStateExited) + { + if (log) + log->Printf("Process::HaltForDestroyOrDetach() Process exited while waiting to Halt."); + return error; + } + else + exit_event_sp.reset(); // It is ok to consume any non-exit stop events + + if (state != eStateStopped) + { + if (log) + log->Printf("Process::HaltForDestroyOrDetach() Halt failed to stop, state is: %s", StateAsCString(state)); + // If we really couldn't stop the process then we should just error out here, but if the + // lower levels just bobbled sending the event and we really are stopped, then continue on. + StateType private_state = m_private_state.GetValue(); + if (private_state != eStateStopped) + { + return error; + } + } + } + else + { + if (log) + log->Printf("Process::HaltForDestroyOrDetach() Halt got error: %s", error.AsCString()); + } + } + return error; +} + Error Process::Detach () { - Error error (WillDetach()); + EventSP exit_event_sp; + Error error; + m_destroy_in_process = true; + + error = WillDetach(); if (error.Success()) { - DisableAllBreakpointSites(); + if (DetachRequiresHalt()) + { + error = HaltForDestroyOrDetach (exit_event_sp); + if (!error.Success()) + { + m_destroy_in_process = false; + return error; + } + else if (exit_event_sp) + { + // We shouldn't need to do anything else here. There's no process left to detach from... + StopPrivateStateThread(); + m_destroy_in_process = false; + return error; + } + } + error = DoDetach(); if (error.Success()) { @@ -3312,6 +3384,22 @@ Process::Detach () StopPrivateStateThread(); } } + m_destroy_in_process = false; + + // If we exited when we were waiting for a process to stop, then + // forward the event here so we don't lose the event + if (exit_event_sp) + { + // Directly broadcast our exited event because we shut down our + // private state thread above + BroadcastEvent(exit_event_sp); + } + + // If we have been interrupted (to kill us) in the middle of running, we may not end up propagating + // the last events through the event system, in which case we might strand the write lock. Unlock + // it here so when we do to tear down the process we don't get an error destroying the lock. + + m_run_lock.WriteUnlock(); return error; } @@ -3329,46 +3417,11 @@ Process::Destroy () if (error.Success()) { EventSP exit_event_sp; - if (m_public_state.GetValue() == eStateRunning) + if (DestroyRequiresHalt()) { - Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS)); - if (log) - log->Printf("Process::Destroy() About to halt."); - error = Halt(); - if (error.Success()) - { - // Consume the halt event. - TimeValue timeout (TimeValue::Now()); - timeout.OffsetWithSeconds(1); - StateType state = WaitForProcessToStop (&timeout, &exit_event_sp); - if (state != eStateExited) - exit_event_sp.reset(); // It is ok to consume any non-exit stop events - - if (state != eStateStopped) - { - if (log) - log->Printf("Process::Destroy() Halt failed to stop, state is: %s", StateAsCString(state)); - // If we really couldn't stop the process then we should just error out here, but if the - // lower levels just bobbled sending the event and we really are stopped, then continue on. - StateType private_state = m_private_state.GetValue(); - if (private_state != eStateStopped && private_state != eStateExited) - { - // If we exited when we were waiting for a process to stop, then - // forward the event here so we don't lose the event - m_destroy_in_process = false; - return error; - } - } - } - else - { - if (log) - log->Printf("Process::Destroy() Halt got error: %s", error.AsCString()); - m_destroy_in_process = false; - return error; - } + error = HaltForDestroyOrDetach(exit_event_sp); } - + if (m_public_state.GetValue() != eStateRunning) { // Ditch all thread plans, and remove all our breakpoints: in case we have to restart the target to @@ -3378,7 +3431,7 @@ Process::Destroy () m_thread_list.DiscardThreadPlans(); DisableAllBreakpointSites(); } - + error = DoDestroy(); if (error.Success()) { diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 54c1049f98f9..33ade1eef1ab 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -291,6 +291,8 @@ protected: { // This shouldn't ever happen, but just in case, don't do more harm. log->Printf ("PerformAction got called with an invalid thread."); + m_should_stop = true; + m_should_stop_is_valid = true; return; }