[Lldb-commits] [PATCH] D124029: Add a mutex to the ThreadPlanStackMap to prevent it getting corrupted

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We were seeing very occasional crashes that looked like corrupted ThreadPlans.   This happens when we go to destroy a process which is running, and the attempt to interrupt the process fails so we proceed to tear down the process anyway, but in the middle of doing that the stop event comes in and we crash trying to handle it.  We fixed that by protecting the ThreadPlanStack with a mutex so that either the tear down would win, and the event would get discarded, or the event would get handled, and then we would continue with the teardown.

After that change was promulgated we didn't see any more of these crashes but last week I got one crash report that can only be explained by corruption of the ThreadPlanStackMap - which I hadn't added a mutex to.  It looks like we need one there as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124029

Files:
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Target/ThreadPlanStack.cpp


Index: lldb/source/Target/ThreadPlanStack.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -400,6 +400,7 @@
                                 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()) {
@@ -434,6 +435,7 @@
                                    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;
@@ -470,6 +472,7 @@
                                          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);
 
@@ -509,6 +512,7 @@
 
 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;
Index: lldb/include/lldb/Target/ThreadPlanStack.h
===================================================================
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ lldb/include/lldb/Target/ThreadPlanStack.h
@@ -123,11 +123,13 @@
               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();
     m_plans_list.emplace(tid, thread);
   }
 
   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;
@@ -137,6 +139,7 @@
   }
 
   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;
@@ -154,6 +157,7 @@
   }
 
   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();
@@ -172,8 +176,10 @@
 
 private:
   Process &m_process;
+  mutable std::recursive_mutex m_stack_map_mutex;
   using PlansList = std::unordered_map<lldb::tid_t, ThreadPlanStack>;
   PlansList m_plans_list;
+  
 };
 
 } // namespace lldb_private


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124029.423692.patch
Type: text/x-patch
Size: 3010 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220419/fc45e681/attachment.bin>