clayborg added a comment.
It is the SymbolVendor's job to do the locking. And in many cases it already
is. I stopped with inlined comments after a few comments as it would apply to
this entire patch.
It would be great if we can replace all of the locations where you added lock
guards with lldbasserts to assert that the lock is already taken. Then we might
be able to catch places where the locks are not being respected. But in general
the symbol vendor should take the lock, use N number of SymbolFile instances to
complete the request, and then release the lock. Makes things easier on the
lldb_private::SymbolFile subclasses this way.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:266-267
TypeList *SymbolFileDWARF::GetTypeList() {
+ std::lock_guard<std::recursive_mutex> guard(
+ GetObjectFile()->GetModule()->GetMutex());
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
----------------
This shouldn't require locking as it is just handing out a type list ivar from
one place or another.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:277-278
TypeSet &type_set) {
+ std::lock_guard<std::recursive_mutex> guard(
+ GetObjectFile()->GetModule()->GetMutex());
if (die) {
----------------
This is not a SymbolFile override. It shouldn't require the locking.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:442-443
TypeSystem *SymbolFileDWARF::GetTypeSystemForLanguage(LanguageType language) {
+ std::lock_guard<std::recursive_mutex> guard(
+ GetObjectFile()->GetModule()->GetMutex());
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
----------------
This shouldn't require locking as it is just handing out a type system from one
place or another.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:857-858
uint32_t SymbolFileDWARF::GetNumCompileUnits() {
+ std::lock_guard<std::recursive_mutex> guard(
+ GetObjectFile()->GetModule()->GetMutex());
DWARFDebugInfo *info = DebugInfo();
----------------
SymbolVendor::GetNumCompileUnits() already takes the module lock for us. All
other internal calls to this should be protected as well
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:866-867
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+ std::lock_guard<std::recursive_mutex> guard(
+ GetObjectFile()->GetModule()->GetMutex());
CompUnitSP cu_sp;
----------------
SymbolVendor already takes the module lock for us
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:880-881
const DWARFDIE &die) {
+ std::lock_guard<std::recursive_mutex> guard(
+ GetObjectFile()->GetModule()->GetMutex());
if (die.IsValid()) {
----------------
SymbolVendor requests that lead to this call should take the lock for us...
Repository:
rLLDB LLDB
https://reviews.llvm.org/D52543
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
