[Lldb-commits] [lldb] 5752e31 - [lldb] fix dead lock in TypeCategoryMap.cpp (#87540)

[via lldb-commits]

Author: Vincent Belliard
Date: 2024-04-12T10:48:41-04:00
New Revision: 5752e3196bc52fdac70e3650abc703570ff6209b

URL: https://github.com/llvm/llvm-project/commit/5752e3196bc52fdac70e3650abc703570ff6209b
DIFF: https://github.com/llvm/llvm-project/commit/5752e3196bc52fdac70e3650abc703570ff6209b.diff

LOG: [lldb] fix dead lock in TypeCategoryMap.cpp (#87540)

FormatManager::GetCategoryForLanguage and
FormatManager::GetCategory(can_create = true) can be called concurrently
and they both take the TypeCategory::m_map_mutex and the
FormatManager::m_language_categories_mutex but in reverse order.

On one thread, GetCategoryForLanguage takes m_language_categories_mutex
and then ends calling TypeCategoryMap::Get which takes m_map_mutex

On another thread GetCategory calls TypeCategoryMap::Add which takes
m_map_mutex and then calls FormatManager::Changed() which takes
m_language_categories_mutex

If both threads are running concurrently, we have a dead lock.

The patch releases the m_map_mutex before calling Changed which avoids
the dead lock.

---------

Co-authored-by: Vincent Belliard <v-bulle at github.com>

Added: 
    

Modified: 
    lldb/source/DataFormatters/TypeCategoryMap.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index fd76bab95826af..ce2cf369b5be53 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -25,19 +25,31 @@ TypeCategoryMap::TypeCategoryMap(IFormatChangeListener *lst)
 }
 
 void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP &entry) {
-  std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
-  m_map[name] = entry;
+  {
+    std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
+    m_map[name] = entry;
+  }
+  // Release the mutex to avoid a potential deadlock between
+  // TypeCategoryMap::m_map_mutex and
+  // FormatManager::m_language_categories_mutex which can be acquired in
+  // reverse order when calling FormatManager::Changed.
   if (listener)
     listener->Changed();
 }
 
 bool TypeCategoryMap::Delete(KeyType name) {
-  std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
-  MapIterator iter = m_map.find(name);
-  if (iter == m_map.end())
-    return false;
-  m_map.erase(name);
-  Disable(name);
+  {
+    std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
+    MapIterator iter = m_map.find(name);
+    if (iter == m_map.end())
+      return false;
+    m_map.erase(name);
+    Disable(name);
+  }
+  // Release the mutex to avoid a potential deadlock between
+  // TypeCategoryMap::m_map_mutex and
+  // FormatManager::m_language_categories_mutex which can be acquired in
+  // reverse order when calling FormatManager::Changed.
   if (listener)
     listener->Changed();
   return true;
@@ -123,9 +135,15 @@ void TypeCategoryMap::DisableAllCategories() {
 }
 
 void TypeCategoryMap::Clear() {
-  std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
-  m_map.clear();
-  m_active_categories.clear();
+  {
+    std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
+    m_map.clear();
+    m_active_categories.clear();
+  }
+  // Release the mutex to avoid a potential deadlock between
+  // TypeCategoryMap::m_map_mutex and
+  // FormatManager::m_language_categories_mutex which can be acquired in
+  // reverse order when calling FormatManager::Changed.
   if (listener)
     listener->Changed();
 }