[Lldb-commits] [lldb] r178325 - Rationalize how we do Halt-ing before Destroy and Detach.

Jim Ingham jingham at apple.com
Thu Mar 28 18:18:12 PDT 2013


Author: jingham
Date: Thu Mar 28 20:18:12 2013
New Revision: 178325

URL: http://llvm.org/viewvc/llvm-project?rev=178325&view=rev
Log:
Rationalize how we do Halt-ing before Destroy and Detach.

<rdar://problem/13527167>

Modified:
    lldb/trunk/include/lldb/Target/Process.h
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
    lldb/trunk/source/Target/Process.cpp
    lldb/trunk/source/Target/StopInfo.cpp

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=178325&r1=178324&r2=178325&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Thu Mar 28 20:18:12 2013
@@ -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:
     //------------------------------------------------------------------

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=178325&r1=178324&r2=178325&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu Mar 28 20:18:12 2013
@@ -1671,99 +1671,6 @@ ProcessGDBRemote::DoHalt (bool &caused_s
 }
 
 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()
 {
     Error error;

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=178325&r1=178324&r2=178325&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Thu Mar 28 20:18:12 2013
@@ -141,10 +141,10 @@ public:
     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 ();
 

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=178325&r1=178324&r2=178325&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Thu Mar 28 20:18:12 2013
@@ -3298,13 +3298,85 @@ Process::Halt ()
 }
 
 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())
         {

Modified: lldb/trunk/source/Target/StopInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StopInfo.cpp?rev=178325&r1=178324&r2=178325&view=diff
==============================================================================
--- lldb/trunk/source/Target/StopInfo.cpp (original)
+++ lldb/trunk/source/Target/StopInfo.cpp Thu Mar 28 20:18:12 2013
@@ -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;
         }
         





More information about the lldb-commits mailing list