Move m_destroy_in_process to Process (from ProcessKDP) since it is generally useful,
authorJim Ingham <jingham@apple.com>
Fri, 1 Mar 2013 20:04:25 +0000 (20:04 +0000)
committerJim Ingham <jingham@apple.com>
Fri, 1 Mar 2013 20:04:25 +0000 (20:04 +0000)
and use it to keep from doing the OS Plugin UpdateThreadList while destroying, since
if that does anything that requires the API lock it may deadlock against whoever is
running the Process::Destroy.

<rdar://problem/13308627>

llvm-svn: 176375

lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Target/Process.cpp

index 19540ec..c4e6906 100644 (file)
@@ -3597,6 +3597,7 @@ protected:
     Predicate<bool>             m_currently_handling_event;
     bool                        m_finalize_called;
     lldb::StateType             m_last_broadcast_state;   /// This helps with the Public event coalescing in ShouldBroadcastEvent.
+    bool m_destroy_in_process;
     
     enum {
         eCanJITDontKnow= 0,
index da1e059..f27c5f8 100644 (file)
@@ -114,7 +114,6 @@ ProcessKDP::ProcessKDP(Target& target, Listener &listener) :
     m_comm("lldb.process.kdp-remote.communication"),
     m_async_broadcaster (NULL, "lldb.process.kdp-remote.async-broadcaster"),
     m_async_thread (LLDB_INVALID_HOST_THREAD),
-    m_destroy_in_process (false),
     m_dyld_plugin_name (),
     m_kernel_load_addr (LLDB_INVALID_ADDRESS),
     m_command_sp()
@@ -532,14 +531,6 @@ ProcessKDP::DoDetach()
 }
 
 Error
-ProcessKDP::WillDestroy ()
-{
-    Error error;
-    m_destroy_in_process = true;
-    return error;
-}
-
-Error
 ProcessKDP::DoDestroy ()
 {
     // For KDP there really is no difference between destroy and detach
index 520c62e..fba221e 100644 (file)
@@ -141,9 +141,6 @@ public:
     DoSignal (int signal);
     
     virtual lldb_private::Error
-    WillDestroy ();
-    
-    virtual lldb_private::Error
     DoDestroy ();
     
     virtual void
@@ -257,7 +254,6 @@ protected:
     CommunicationKDP m_comm;
     lldb_private::Broadcaster m_async_broadcaster;
     lldb::thread_t m_async_thread;
-    bool m_destroy_in_process;
     std::string m_dyld_plugin_name;
     lldb::addr_t m_kernel_load_addr;
     lldb::CommandObjectSP m_command_sp;
index 28816c9..ff42d6e 100644 (file)
@@ -1023,7 +1023,8 @@ Process::Process(Target &target, Listener &listener) :
     m_currently_handling_event(false),
     m_finalize_called(false),
     m_last_broadcast_state (eStateInvalid),
-    m_can_jit(eCanJITDontKnow)
+    m_can_jit(eCanJITDontKnow),
+    m_destroy_in_process (false)
 {
     CheckInWithManager ();
 
@@ -1512,11 +1513,17 @@ Process::UpdateThreadListIfNeeded ()
             // thread list, but only update if "true" is returned
             if (UpdateThreadList (m_thread_list, new_thread_list))
             {
-                OperatingSystem *os = GetOperatingSystem ();
-                if (os)
-                    os->UpdateThreadList (m_thread_list, new_thread_list);
-                m_thread_list.Update (new_thread_list);
-                m_thread_list.SetStopID (stop_id);
+                // Don't call into the OperatingSystem to update the thread list if we are shutting down, since
+                // that may call back into the SBAPI's, requiring the API lock which is already held by whoever is
+                // shutting us down, causing a deadlock.
+                if (!m_destroy_in_process)
+                {
+                    OperatingSystem *os = GetOperatingSystem ();
+                    if (os)
+                        os->UpdateThreadList (m_thread_list, new_thread_list);
+                    m_thread_list.Update (new_thread_list);
+                    m_thread_list.SetStopID (stop_id);
+                }
             }
         }
     }
@@ -3311,6 +3318,13 @@ Process::Detach ()
 Error
 Process::Destroy ()
 {
+    
+    // Tell ourselves we are in the process of destroying the process, so that we don't do any unnecessary work
+    // that might hinder the destruction.  Remember to set this back to false when we are done.  That way if the attempt
+    // failed and the process stays around for some reason it won't be in a confused state.
+    
+    m_destroy_in_process = true;
+
     Error error (WillDestroy());
     if (error.Success())
     {
@@ -3341,6 +3355,7 @@ Process::Destroy ()
                     {
                         // 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;
                     }
                 }
@@ -3349,6 +3364,7 @@ Process::Destroy ()
             {
                 if (log)
                     log->Printf("Process::Destroy() Halt got error: %s", error.AsCString());
+                m_destroy_in_process = false;
                 return error;
             }
         }
@@ -3390,6 +3406,9 @@ Process::Destroy ()
         // it here so when we do to tear down the process we don't get an error destroying the lock.
         m_run_lock.WriteUnlock();
     }
+    
+    m_destroy_in_process = false;
+    
     return error;
 }