[Lldb-commits] [lldb] 862a311 - [lldb] Tighten lock in Language::ForEach

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2021-08-31T15:45:38-07:00
New Revision: 862a311301f51214d405eefd5bc4aa7289241f86

URL: https://github.com/llvm/llvm-project/commit/862a311301f51214d405eefd5bc4aa7289241f86
DIFF: https://github.com/llvm/llvm-project/commit/862a311301f51214d405eefd5bc4aa7289241f86.diff

LOG: [lldb] Tighten lock in Language::ForEach

It is easy to accidentally introduce a deadlock by having the callback
passed to Language::ForEach also attempt to acquire the same lock. It
is easy enough to disallow the callback from calling anything in
Language directly, but it may happen through a series of other
function/method calls.

The solution I am proposing is to tighten the lock in Language::ForEach
so that it is only held as we gather the currently loaded language
plugins. We store them in a vector and then iterate through them with
the callback so that the callback can't introduce a deadlock.

Differential Revision: https://reviews.llvm.org/D109013

Added: 
    

Modified: 
    lldb/source/Target/Language.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index 7b35da5028ace..eee1ff1512d95 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -108,10 +108,21 @@ void Language::ForEach(std::function<bool(Language *)> callback) {
     }
   });
 
-  std::lock_guard<std::mutex> guard(GetLanguagesMutex());
-  LanguagesMap &map(GetLanguagesMap());
-  for (const auto &entry : map) {
-    if (!callback(entry.second.get()))
+  // callback may call a method in Language that attempts to acquire the same
+  // lock (such as Language::ForEach or Language::FindPlugin). To avoid a
+  // deadlock, we do not use callback while holding the lock.
+  std::vector<Language *> loaded_plugins;
+  {
+    std::lock_guard<std::mutex> guard(GetLanguagesMutex());
+    LanguagesMap &map(GetLanguagesMap());
+    for (const auto &entry : map) {
+      if (entry.second)
+        loaded_plugins.push_back(entry.second.get());
+    }
+  }
+
+  for (auto *lang : loaded_plugins) {
+    if (!callback(lang))
       break;
   }
 }