[Lldb-commits] [lldb] r203233 - Don't hold the ThreadList lock over calls to the GetStatus (Process or Thread) calls

Jim Ingham jingham at apple.com
Fri Mar 7 03:20:03 PST 2014


Author: jingham
Date: Fri Mar  7 05:20:03 2014
New Revision: 203233

URL: http://llvm.org/viewvc/llvm-project?rev=203233&view=rev
Log:
Don't hold the ThreadList lock over calls to the GetStatus (Process or Thread) calls
or the lower levels of the Process won't be able to restart.

<rdar://problem/16244835>

Modified:
    lldb/trunk/source/Core/Debugger.cpp
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Core/Debugger.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=203233&r1=203232&r2=203233&view=diff
==============================================================================
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Fri Mar  7 05:20:03 2014
@@ -2864,66 +2864,71 @@ Debugger::HandleProcessEvent (const Even
                     }
                     else
                     {
-                        // Lock the thread list so it doesn't change on us
-                        ThreadList &thread_list = process_sp->GetThreadList();
-                        Mutex::Locker locker (thread_list.GetMutex());
-                        
-                        ThreadSP curr_thread (thread_list.GetSelectedThread());
-                        ThreadSP thread;
-                        StopReason curr_thread_stop_reason = eStopReasonInvalid;
-                        if (curr_thread)
-                            curr_thread_stop_reason = curr_thread->GetStopReason();
-                        if (!curr_thread ||
-                            !curr_thread->IsValid() ||
-                            curr_thread_stop_reason == eStopReasonInvalid ||
-                            curr_thread_stop_reason == eStopReasonNone)
+                        // Lock the thread list so it doesn't change on us, this is the scope for the locker:
                         {
-                            // Prefer a thread that has just completed its plan over another thread as current thread.
-                            ThreadSP plan_thread;
-                            ThreadSP other_thread;
-                            const size_t num_threads = thread_list.GetSize();
-                            size_t i;
-                            for (i = 0; i < num_threads; ++i)
+                            ThreadList &thread_list = process_sp->GetThreadList();
+                            Mutex::Locker locker (thread_list.GetMutex());
+                            
+                            ThreadSP curr_thread (thread_list.GetSelectedThread());
+                            ThreadSP thread;
+                            StopReason curr_thread_stop_reason = eStopReasonInvalid;
+                            if (curr_thread)
+                                curr_thread_stop_reason = curr_thread->GetStopReason();
+                            if (!curr_thread ||
+                                !curr_thread->IsValid() ||
+                                curr_thread_stop_reason == eStopReasonInvalid ||
+                                curr_thread_stop_reason == eStopReasonNone)
                             {
-                                thread = thread_list.GetThreadAtIndex(i);
-                                StopReason thread_stop_reason = thread->GetStopReason();
-                                switch (thread_stop_reason)
+                                // Prefer a thread that has just completed its plan over another thread as current thread.
+                                ThreadSP plan_thread;
+                                ThreadSP other_thread;
+                                const size_t num_threads = thread_list.GetSize();
+                                size_t i;
+                                for (i = 0; i < num_threads; ++i)
                                 {
-                                    case eStopReasonInvalid:
-                                    case eStopReasonNone:
-                                        break;
-                                        
-                                    case eStopReasonTrace:
-                                    case eStopReasonBreakpoint:
-                                    case eStopReasonWatchpoint:
-                                    case eStopReasonSignal:
-                                    case eStopReasonException:
-                                    case eStopReasonExec:
-                                    case eStopReasonThreadExiting:
-                                        if (!other_thread)
-                                            other_thread = thread;
-                                        break;
-                                    case eStopReasonPlanComplete:
-                                        if (!plan_thread)
-                                            plan_thread = thread;
-                                        break;
+                                    thread = thread_list.GetThreadAtIndex(i);
+                                    StopReason thread_stop_reason = thread->GetStopReason();
+                                    switch (thread_stop_reason)
+                                    {
+                                        case eStopReasonInvalid:
+                                        case eStopReasonNone:
+                                            break;
+                                            
+                                        case eStopReasonTrace:
+                                        case eStopReasonBreakpoint:
+                                        case eStopReasonWatchpoint:
+                                        case eStopReasonSignal:
+                                        case eStopReasonException:
+                                        case eStopReasonExec:
+                                        case eStopReasonThreadExiting:
+                                            if (!other_thread)
+                                                other_thread = thread;
+                                            break;
+                                        case eStopReasonPlanComplete:
+                                            if (!plan_thread)
+                                                plan_thread = thread;
+                                            break;
+                                    }
                                 }
-                            }
-                            if (plan_thread)
-                                thread_list.SetSelectedThreadByID (plan_thread->GetID());
-                            else if (other_thread)
-                                thread_list.SetSelectedThreadByID (other_thread->GetID());
-                            else
-                            {
-                                if (curr_thread && curr_thread->IsValid())
-                                    thread = curr_thread;
+                                if (plan_thread)
+                                    thread_list.SetSelectedThreadByID (plan_thread->GetID());
+                                else if (other_thread)
+                                    thread_list.SetSelectedThreadByID (other_thread->GetID());
                                 else
-                                    thread = thread_list.GetThreadAtIndex(0);
-                                
-                                if (thread)
-                                    thread_list.SetSelectedThreadByID (thread->GetID());
+                                {
+                                    if (curr_thread && curr_thread->IsValid())
+                                        thread = curr_thread;
+                                    else
+                                        thread = thread_list.GetThreadAtIndex(0);
+                                    
+                                    if (thread)
+                                        thread_list.SetSelectedThreadByID (thread->GetID());
+                                }
                             }
                         }
+                        // Drop the ThreadList mutex by here, since GetThreadStatus below might have to run code,
+                        // e.g. for Data formatters, and if we hold the ThreadList mutex, then the process is going to
+                        // have a hard time restarting the process.
 
                         if (GetTargetList().GetSelectedTarget().get() == &process_sp->GetTarget())
                         {

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=203233&r1=203232&r2=203233&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri Mar  7 05:20:03 2014
@@ -5897,25 +5897,47 @@ Process::GetThreadStatus (Stream &strm,
 {
     size_t num_thread_infos_dumped = 0;
     
-    Mutex::Locker locker (GetThreadList().GetMutex());
-    const size_t num_threads = GetThreadList().GetSize();
+    // You can't hold the thread list lock while calling Thread::GetStatus.  That very well might run code (e.g. if we need it
+    // to get return values or arguments.)  For that to work the process has to be able to acquire it.  So instead copy the thread
+    // ID's, and look them up one by one:
+    
+    uint32_t num_threads;
+    std::vector<uint32_t> thread_index_array;
+    //Scope for thread list locker;
+    {
+        Mutex::Locker locker (GetThreadList().GetMutex());
+        ThreadList &curr_thread_list = GetThreadList();
+        num_threads = curr_thread_list.GetSize();
+        uint32_t idx;
+        thread_index_array.resize(num_threads);
+        for (idx = 0; idx < num_threads; ++idx)
+            thread_index_array[idx] = curr_thread_list.GetThreadAtIndex(idx)->GetID();
+    }
+    
     for (uint32_t i = 0; i < num_threads; i++)
     {
-        Thread *thread = GetThreadList().GetThreadAtIndex(i).get();
-        if (thread)
+        ThreadSP thread_sp(GetThreadList().FindThreadByID(thread_index_array[i]));
+        if (thread_sp)
         {
             if (only_threads_with_stop_reason)
             {
-                StopInfoSP stop_info_sp = thread->GetStopInfo();
+                StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
                 if (stop_info_sp.get() == NULL || !stop_info_sp->IsValid())
                     continue;
             }
-            thread->GetStatus (strm, 
+            thread_sp->GetStatus (strm,
                                start_frame, 
                                num_frames, 
                                num_frames_with_source);
             ++num_thread_infos_dumped;
         }
+        else
+        {
+            Log *log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PROCESS));
+            if (log)
+                log->Printf("Process::GetThreadStatus - thread 0x" PRIu64 " vanished while running Thread::GetStatus.");
+            
+        }
     }
     return num_thread_infos_dumped;
 }





More information about the lldb-commits mailing list