clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inlined comments
================
Comment at: include/lldb/Symbol/SymbolFile.h:98
+ virtual std::recursive_mutex &GetModuleMutex() const;
+
----------------
Might add a comment here stating something like "Symbols file subclasses should
override this to return the Module that owns the TypeSystem that this symbol
file modifies type information in".
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2022-2025
+ if (m_debug_map_module_wp.expired())
+ return GetObjectFile()->GetModule()->GetMutex();
+ lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
+ return module_sp->GetMutex();
----------------
Possible race condition and crasher here. Best coded as:
```
lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
if (module_sp)
return module_sp->GetMutex();
return GetObjectFile()->GetModule()->GetMutex();
```
Otherwise some other thread could clear "m_debug_map_module_wp" after
"m_debug_map_module_wp.expired()" is called, but before
"m_debug_map_module_wp.lock()" is called and we would crash.
================
Comment at: source/Symbol/SymbolFile.cpp:160-168
+void SymbolFile::AssertModuleLock() {
+ // We assert that we have to module lock by trying to acquire the lock from a
+ // different thread. Note that we must abort if the result is true to
+ // guarantee correctness.
+ lldbassert(std::async(std::launch::async,
+ [this] { return this->GetModuleMutex().try_lock(); })
+ .get() == false &&
----------------
We should make this entire thing and all uses of it into a macro so we can
enable it for Debug builds, but disable it for Release builds and have no extra
code in release builds. We really shouldn't be starting threads up all the time
for every function in SymbolFile subclasses for Release builds. It is ok for
Debug builds or maybe manually enable this when we run into issues, but this is
too expensive to leave in for all builds.
https://reviews.llvm.org/D52543
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
