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

[Sean Callanan via Phabricator via lldb-commits]

spyffe updated this revision to Diff 106087.
spyffe added a comment.

Upon reflection, it's not worth coming up with a new pattern if

- that pattern does not considerably reduce the incidence of bugs in the source file I'm applying it in (and indeed that seems quite unlikely), and
- that pattern is unlikely to be adopted elsewhere.

I'm going to simply pass in a `std::unique_lock` and swap into that.  This has the pleasant side effect of guarding the `g_totalSizeOfMetadata` variable against at least some overrides, although to be honest I'd accept a separate patch that removes this variable.


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,23 +10,29 @@
 #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() {
+static ASTSourceMap &GetSourceMap(std::unique_lock<std::mutex> &guard) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
+  static std::mutex s_mutex;
+  std::unique_lock<std::mutex> locked_guard(s_mutex);
+  guard.swap(locked_guard);
   return *s_source_map;
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap &source_map = GetSourceMap();
+  std::unique_lock<std::mutex> guard;
+  ASTSourceMap &source_map = GetSourceMap(guard);
 
   ASTSourceMap::iterator iter = source_map.find(source);
 
@@ -40,11 +46,13 @@
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
     : clang::ExternalASTSource() {
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
+  std::unique_lock<std::mutex> guard;
+  GetSourceMap(guard)[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  std::unique_lock<std::mutex> guard;
+  GetSourceMap(guard).erase(this);
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35083.106087.patch
Type: text/x-patch
Size: 1745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170711/846e5d21/attachment.bin>