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

[Vincent Belliard via lldb-commits]

https://github.com/v-bulle updated https://github.com/llvm/llvm-project/pull/87540

>From 5234f6873d894dd80b2c1fef40fd18e4b722a2c9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard <v-bulle at github.com>
Date: Wed, 3 Apr 2024 11:31:06 -0700
Subject: [PATCH 1/2] [lldb] fix dead lock in TypeCategoryMap.cpp

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

Without the patch, we can have a dead lock.
---
 .../source/DataFormatters/TypeCategoryMap.cpp | 28 +++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index fd76bab95826af..0d1f55fff473d1 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -25,19 +25,23 @@ 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;
+  }
   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);
+  }
   if (listener)
     listener->Changed();
   return true;
@@ -123,9 +127,11 @@ 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();
+  }
   if (listener)
     listener->Changed();
 }

>From 7305de06ab0425631fdf554cd1ad0498ac9fc7ed Mon Sep 17 00:00:00 2001
From: Vincent Belliard <v-bulle at github.com>
Date: Thu, 4 Apr 2024 15:20:59 -0700
Subject: [PATCH 2/2] Adds comments

---
 lldb/source/DataFormatters/TypeCategoryMap.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index 0d1f55fff473d1..ecbdcde98d07d3 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -29,6 +29,7 @@ void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP &entry) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     m_map[name] = entry;
   }
+  // The lock is now released for the eventual call to Changed.
   if (listener)
     listener->Changed();
 }
@@ -42,6 +43,7 @@ bool TypeCategoryMap::Delete(KeyType name) {
     m_map.erase(name);
     Disable(name);
   }
+  // The lock is now released for the eventual call to Changed.
   if (listener)
     listener->Changed();
   return true;
@@ -132,6 +134,7 @@ void TypeCategoryMap::Clear() {
     m_map.clear();
     m_active_categories.clear();
   }
+  // The lock is now released for the eventual call to Changed.
   if (listener)
     listener->Changed();
 }