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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This is an awfully complex solution which in the end doesn't actually enforce that you take the lock to get the SourceMap.  You have to know to wrap the access in this WithExclusiveSourceMap.  Wouldn't it be simpler to make GetSourceMap take a reference to a std::unique_lock which it fills in as, for instance:

  ExecutionContext::ExecutionContext(const ExecutionContextRef &exe_ctx_ref,
                                     std::unique_lock<std::recursive_mutex> &lock)

does, constructing the unique_lock with its mutex:

  lock = std::unique_lock<std::recursive_mutex>(m_target_sp->GetAPIMutex());

Then the caller will have to make this lock, pass it in, and the lock hold for the scope of the caller, e.g.:

  SBCompileUnit SBFrame::GetCompileUnit() const {
    Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
    SBCompileUnit sb_comp_unit;
    std::unique_lock<std::recursive_mutex> lock;
    ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
  
  ...
  }

This is much more readable AND enforces that you have to provide a lock to call the function.


Repository:
  rL LLVM

https://reviews.llvm.org/D35083