Add a mutex to the thread plan stack map class (#4988)

This PR consisted of:

1. 46575a6: Cherry picks [D124029](https://reviews.llvm.org/D124029) which adds a mutex to `ThreadPlanStackMap`
2. 8e34300: Adds a follow up refactoring to preserve the mutex in a use case that is currently swift-only

The refactoring replaces `GetDetachedPlanStacks`, which was a simple getter and had one caller (`Process::FindDetachedPlanExplainingStop`).  The new function is `FindThreadPlanInStack`, which is a search function that takes a lambda. This refactoring is done to ensure the new mutex is held over the course of the search. Without this, the mutex would only be held for the duration of the getter, after which the search could potentially search over the unlocked/unprotected vector.

To make this change, I also had to tweak an existing `friend` declaration.

Author: kastiglione

lldb/include/lldb/Target/Process.h

@@ -2257,8 +2257,16 @@ void PruneThreadPlans();
 
   void SynchronizeThreadPlans();
 
+  /// From the detached thread plan stacks, find the first stack that explains
+  /// the stop represented by the thread and the event.
   lldb::ThreadPlanSP FindDetachedPlanExplainingStop(Thread &thread, Event *event_ptr);
 
+  /// Helper function for FindDetachedPlanExplainingStop. Exists only to be
+  /// marked as a C++ friend of `ThreadPlan`.
+  lldb::ThreadPlanSP DoesStackExplainStopNoLock(ThreadPlanStack &stack,
+                                                Thread &thread,
+                                                Event *event_ptr);
+
   /// Find the thread plan stack associated with thread with \a tid.
   ///
   /// \param[in] tid

lldb/include/lldb/Target/ThreadPlan.h

@@ -490,7 +490,8 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
   void SetTID(lldb::tid_t tid) { m_tid = tid; }
 
   friend lldb::ThreadPlanSP
-  Process::FindDetachedPlanExplainingStop(Thread &thread, Event *event_ptr);
+  Process::DoesStackExplainStopNoLock(ThreadPlanStack &stack, Thread &thread,
+                                      Event *event_ptr);
 
 protected:
   // Constructors and Destructors

lldb/include/lldb/Target/ThreadPlanStack.h

@@ -132,6 +132,7 @@ class ThreadPlanStackMap {
               bool check_for_new = true);
 
   void AddThread(Thread &thread) {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     lldb::tid_t tid = thread.GetID();
     // If we already have a ThreadPlanStack for this thread, use it.
     if (m_plans_list.find(tid) != m_plans_list.end())
@@ -143,6 +144,7 @@ class ThreadPlanStackMap {
   }
 
   bool RemoveTID(lldb::tid_t tid) {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     auto result = m_plans_list.find(tid);
     if (result == m_plans_list.end())
       return false;
@@ -165,6 +167,7 @@ class ThreadPlanStackMap {
   }
 
   ThreadPlanStack *Find(lldb::tid_t tid) {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     auto result = m_plans_list.find(tid);
     if (result == m_plans_list.end())
       return nullptr;
@@ -177,12 +180,14 @@ class ThreadPlanStackMap {
   /// This is useful in situations like when a new Thread list is being
   /// generated.
   void ClearThreadCache() {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     for (auto &plan_list : m_plans_list)
       plan_list.second->ClearThreadCache();
   }
 
   // rename to Reactivate?
   void Activate(ThreadPlanStack &stack) {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     // Remove this from the detached plan list:
     auto end = m_detached_plans.end();    
     auto iter = std::find_if(m_detached_plans.begin(), end, 
@@ -198,6 +203,7 @@ class ThreadPlanStackMap {
   }
 
   void ScanForDetachedPlanStacks() {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     llvm::SmallVector<lldb::tid_t, 2> invalidated_tids;
     for (auto &pair : m_plans_list)
       if (pair.second->GetTID() != pair.first)
@@ -216,11 +222,17 @@ class ThreadPlanStackMap {
   // plans that represent asynchronous operations waiting to be
   // scheduled.
   // The vector will never have null ThreadPlanStacks in it.
-  std::vector<ThreadPlanStack *> &GetDetachedPlanStacks() {
-    return m_detached_plans;
+  lldb::ThreadPlanSP FindThreadPlanInStack(
+      llvm::function_ref<lldb::ThreadPlanSP(ThreadPlanStack &)> fn) {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
+    for (auto *stack : m_detached_plans)
+      if (auto plan = fn(*stack))
+        return plan;
+    return {};
   }
-  
+
   void Clear() {
+    std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
     for (auto &plan : m_plans_list)
       plan.second->ThreadDestroyed(nullptr);
     m_plans_list.clear();
@@ -255,6 +267,7 @@ class ThreadPlanStackMap {
   PlansStore m_plans_up_container;
   std::vector<ThreadPlanStack *> m_detached_plans;
 
+  mutable std::recursive_mutex m_stack_map_mutex;
   using PlansList = std::unordered_map<lldb::tid_t, ThreadPlanStack *>;
   PlansList m_plans_list;
 };

lldb/source/Target/Process.cpp

@@ -1339,20 +1339,24 @@ void Process::SynchronizeThreadPlans() {
 
 ThreadPlanSP Process::FindDetachedPlanExplainingStop(Thread &thread,
                                                      Event *event_ptr) {
-  std::vector<ThreadPlanStack *> &detached_plans 
-      = m_thread_plans.GetDetachedPlanStacks();
-  size_t num_detached_plans = detached_plans.size();
-  for (size_t idx = 0; idx < num_detached_plans; idx++) {
-    ThreadPlanStack *cur_stack = detached_plans[idx];
-    ThreadPlanSP plan_sp = cur_stack->GetCurrentPlan();
-    plan_sp->SetTID(thread.GetID());
-    if (!plan_sp->DoPlanExplainsStop(event_ptr)) {
-      plan_sp->ClearTID();
-      continue;
-    }
-    m_thread_plans.Activate(*cur_stack);
+  return m_thread_plans.FindThreadPlanInStack(
+      [&](ThreadPlanStack &stack) -> ThreadPlanSP {
+        return DoesStackExplainStopNoLock(stack, thread, event_ptr);
+      });
+}
+
+// This extracted function only exists so that it can be marked a friend of
+// `ThreadPlan`, which is needed to call `DoPlanExplainsStop`.
+ThreadPlanSP Process::DoesStackExplainStopNoLock(ThreadPlanStack &stack,
+                                                 Thread &thread,
+                                                 Event *event_ptr) {
+  ThreadPlanSP plan_sp = stack.GetCurrentPlan();
+  plan_sp->SetTID(thread.GetID());
+  if (plan_sp->DoPlanExplainsStop(event_ptr)) {
+    m_thread_plans.Activate(stack);
     return plan_sp;
   }
+  plan_sp->ClearTID();
   return {};
 }
 

lldb/source/Target/ThreadPlanStack.cpp

@@ -410,6 +410,7 @@ void ThreadPlanStackMap::Update(ThreadList &current_threads,
                                 bool delete_missing,
                                 bool check_for_new) {
 
+  std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
   // Now find all the new threads and add them to the map:
   if (check_for_new) {
     for (auto thread : current_threads.Threads()) {
@@ -444,6 +445,7 @@ void ThreadPlanStackMap::DumpPlans(Stream &strm,
                                    lldb::DescriptionLevel desc_level,
                                    bool internal, bool condense_if_trivial,
                                    bool skip_unreported) {
+  std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
   for (auto &elem : m_plans_list) {
     lldb::tid_t tid = elem.first;
     uint32_t index_id = 0;
@@ -480,6 +482,7 @@ bool ThreadPlanStackMap::DumpPlansForTID(Stream &strm, lldb::tid_t tid,
                                          bool internal,
                                          bool condense_if_trivial,
                                          bool skip_unreported) {
+  std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
   uint32_t index_id = 0;
   ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(tid);
 
@@ -519,6 +522,7 @@ bool ThreadPlanStackMap::DumpPlansForTID(Stream &strm, lldb::tid_t tid,
 
 bool ThreadPlanStackMap::PrunePlansForTID(lldb::tid_t tid) {
   // We only remove the plans for unreported TID's.
+  std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
   ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(tid);
   if (thread_sp)
     return false;