[Lldb-commits] [PATCH] D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex

[Sean Callanan via Phabricator via lldb-commits]

spyffe created this revision.

`s_source_map` in `ClangExternalASTSourceCommon.cpp` is unguarded and therefore can break in multithreaded conditions.  This can cause crashes in particular if multiple targets are being set up at once.

This patch wraps `s_source_map` in a function that ensures exclusivity, and makes every user of it use that function instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D35083

Files:
  source/Symbol/ClangExternalASTSourceCommon.cpp


Index: source/Symbol/ClangExternalASTSourceCommon.cpp
===================================================================
--- source/Symbol/ClangExternalASTSourceCommon.cpp
+++ source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,41 +10,55 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include <mutex>
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap<clang::ExternalASTSource *,
                        ClangExternalASTSourceCommon *>
     ASTSourceMap;
 
-static ASTSourceMap &GetSourceMap() {
+template <typename FnType>
+static decltype(std::declval<FnType>()(std::declval<ASTSourceMap&>()))
+WithExclusiveSourceMap(FnType fn) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
-  return *s_source_map;
+  static std::mutex s_source_map_mutex;
+  
+  {
+    std::lock_guard<std::mutex> source_map_locker(s_source_map_mutex);
+    return fn(*s_source_map);
+  }
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap &source_map = GetSourceMap();
-
-  ASTSourceMap::iterator iter = source_map.find(source);
-
-  if (iter != source_map.end()) {
-    return iter->second;
-  } else {
-    return nullptr;
-  }
+  return WithExclusiveSourceMap(
+      [source](ASTSourceMap &source_map) -> ClangExternalASTSourceCommon * {
+    ASTSourceMap::iterator iter = source_map.find(source);
+    
+    if (iter != source_map.end()) {
+      return iter->second;
+    } else {
+      return nullptr;
+    }
+  });
 }
 
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
     : clang::ExternalASTSource() {
+  WithExclusiveSourceMap([this](ASTSourceMap &source_map) {
+    source_map[this] = this;
+  });
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  WithExclusiveSourceMap([this](ASTSourceMap &source_map) {
+    source_map.erase(this);
+  });
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35083.105516.patch
Type: text/x-patch
Size: 2184 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170706/bc839545/attachment.bin>