<rdar://problem/11034170>
Simplify the locking strategy for Module and its owned objects to always use the Module's mutex to avoid A/B deadlocks. We had a case where a symbol vendor was locking itself and then calling a function that would try to get it's Module's mutex and at the same time another thread had the Module mutex that was trying to get the SymbolVendor mutex. Now any classes that inherit from ModuleChild should use the module lock using code like:
void
ModuleChildSubclass::Function
{
ModuleSP module_sp(GetModule());
if (module_sp)
{
lldb_private::Mutex::Locker locker(module_sp->GetMutex());
... do work here...
}
}
This will help avoid deadlocks by using as few locks as possible for a module and all its child objects and also enforce detecting if a module has gone away (the ModuleSP will be returned empty if the weak_ptr does refer to a valid object anymore).
llvm-svn: 152679
diff --git a/lldb/source/Symbol/SymbolVendor.cpp b/lldb/source/Symbol/SymbolVendor.cpp
index 1b03eb5..506ee19 100644
--- a/lldb/source/Symbol/SymbolVendor.cpp
+++ b/lldb/source/Symbol/SymbolVendor.cpp
@@ -68,7 +68,6 @@
//----------------------------------------------------------------------
SymbolVendor::SymbolVendor(const lldb::ModuleSP &module_sp) :
ModuleChild (module_sp),
- m_mutex (Mutex::eMutexTypeRecursive),
m_type_list(),
m_compile_units(),
m_sym_file_ap()
@@ -88,29 +87,37 @@
void
SymbolVendor::AddSymbolFileRepresentation(const ObjectFileSP &objfile_sp)
{
- Mutex::Locker locker(m_mutex);
- if (objfile_sp)
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
{
- m_objfile_sp = objfile_sp;
- m_sym_file_ap.reset(SymbolFile::FindPlugin(objfile_sp.get()));
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (objfile_sp)
+ {
+ m_objfile_sp = objfile_sp;
+ m_sym_file_ap.reset(SymbolFile::FindPlugin(objfile_sp.get()));
+ }
}
}
bool
SymbolVendor::SetCompileUnitAtIndex (CompUnitSP& cu, uint32_t idx)
{
- Mutex::Locker locker(m_mutex);
- const uint32_t num_compile_units = GetNumCompileUnits();
- if (idx < num_compile_units)
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
{
- // Fire off an assertion if this compile unit already exists for now.
- // The partial parsing should take care of only setting the compile
- // unit once, so if this assertion fails, we need to make sure that
- // we don't have a race condition, or have a second parse of the same
- // compile unit.
- assert(m_compile_units[idx].get() == NULL);
- m_compile_units[idx] = cu;
- return true;
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ const uint32_t num_compile_units = GetNumCompileUnits();
+ if (idx < num_compile_units)
+ {
+ // Fire off an assertion if this compile unit already exists for now.
+ // The partial parsing should take care of only setting the compile
+ // unit once, so if this assertion fails, we need to make sure that
+ // we don't have a race condition, or have a second parse of the same
+ // compile unit.
+ assert(m_compile_units[idx].get() == NULL);
+ m_compile_units[idx] = cu;
+ return true;
+ }
}
return false;
}
@@ -118,16 +125,20 @@
uint32_t
SymbolVendor::GetNumCompileUnits()
{
- Mutex::Locker locker(m_mutex);
- if (m_compile_units.empty())
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
{
- if (m_sym_file_ap.get())
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_compile_units.empty())
{
- // Resize our array of compile unit shared pointers -- which will
- // each remain NULL until someone asks for the actual compile unit
- // information. When this happens, the symbol file will be asked
- // to parse this compile unit information.
- m_compile_units.resize(m_sym_file_ap->GetNumCompileUnits());
+ if (m_sym_file_ap.get())
+ {
+ // Resize our array of compile unit shared pointers -- which will
+ // each remain NULL until someone asks for the actual compile unit
+ // information. When this happens, the symbol file will be asked
+ // to parse this compile unit information.
+ m_compile_units.resize(m_sym_file_ap->GetNumCompileUnits());
+ }
}
}
return m_compile_units.size();
@@ -136,63 +147,91 @@
size_t
SymbolVendor::ParseCompileUnitFunctions (const SymbolContext &sc)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ParseCompileUnitFunctions(sc);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ParseCompileUnitFunctions(sc);
+ }
return 0;
}
bool
SymbolVendor::ParseCompileUnitLineTable (const SymbolContext &sc)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ParseCompileUnitLineTable(sc);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ParseCompileUnitLineTable(sc);
+ }
return false;
}
bool
SymbolVendor::ParseCompileUnitSupportFiles (const SymbolContext& sc, FileSpecList& support_files)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ParseCompileUnitSupportFiles(sc, support_files);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ParseCompileUnitSupportFiles(sc, support_files);
+ }
return false;
}
size_t
SymbolVendor::ParseFunctionBlocks (const SymbolContext &sc)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ParseFunctionBlocks(sc);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ParseFunctionBlocks(sc);
+ }
return 0;
}
size_t
SymbolVendor::ParseTypes (const SymbolContext &sc)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ParseTypes(sc);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ParseTypes(sc);
+ }
return 0;
}
size_t
SymbolVendor::ParseVariablesForContext (const SymbolContext& sc)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ParseVariablesForContext(sc);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ParseVariablesForContext(sc);
+ }
return 0;
}
Type*
SymbolVendor::ResolveTypeUID(lldb::user_id_t type_uid)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ResolveTypeUID(type_uid);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ResolveTypeUID(type_uid);
+ }
return NULL;
}
@@ -200,54 +239,78 @@
uint32_t
SymbolVendor::ResolveSymbolContext (const Address& so_addr, uint32_t resolve_scope, SymbolContext& sc)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ResolveSymbolContext(so_addr, resolve_scope, sc);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ResolveSymbolContext(so_addr, resolve_scope, sc);
+ }
return 0;
}
uint32_t
SymbolVendor::ResolveSymbolContext (const FileSpec& file_spec, uint32_t line, bool check_inlines, uint32_t resolve_scope, SymbolContextList& sc_list)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->ResolveSymbolContext(file_spec, line, check_inlines, resolve_scope, sc_list);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->ResolveSymbolContext(file_spec, line, check_inlines, resolve_scope, sc_list);
+ }
return 0;
}
uint32_t
SymbolVendor::FindGlobalVariables (const ConstString &name, const ClangNamespaceDecl *namespace_decl, bool append, uint32_t max_matches, VariableList& variables)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->FindGlobalVariables(name, namespace_decl, append, max_matches, variables);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->FindGlobalVariables(name, namespace_decl, append, max_matches, variables);
+ }
return 0;
}
uint32_t
SymbolVendor::FindGlobalVariables (const RegularExpression& regex, bool append, uint32_t max_matches, VariableList& variables)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->FindGlobalVariables(regex, append, max_matches, variables);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->FindGlobalVariables(regex, append, max_matches, variables);
+ }
return 0;
}
uint32_t
SymbolVendor::FindFunctions(const ConstString &name, const ClangNamespaceDecl *namespace_decl, uint32_t name_type_mask, bool include_inlines, bool append, SymbolContextList& sc_list)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->FindFunctions(name, namespace_decl, name_type_mask, include_inlines, append, sc_list);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->FindFunctions(name, namespace_decl, name_type_mask, include_inlines, append, sc_list);
+ }
return 0;
}
uint32_t
SymbolVendor::FindFunctions(const RegularExpression& regex, bool include_inlines, bool append, SymbolContextList& sc_list)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->FindFunctions(regex, include_inlines, append, sc_list);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->FindFunctions(regex, include_inlines, append, sc_list);
+ }
return 0;
}
@@ -255,9 +318,13 @@
uint32_t
SymbolVendor::FindTypes (const SymbolContext& sc, const ConstString &name, const ClangNamespaceDecl *namespace_decl, bool append, uint32_t max_matches, TypeList& types)
{
- Mutex::Locker locker(m_mutex);
- if (m_sym_file_ap.get())
- return m_sym_file_ap->FindTypes(sc, name, namespace_decl, append, max_matches, types);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ return m_sym_file_ap->FindTypes(sc, name, namespace_decl, append, max_matches, types);
+ }
if (!append)
types.Clear();
return 0;
@@ -266,66 +333,75 @@
ClangNamespaceDecl
SymbolVendor::FindNamespace(const SymbolContext& sc, const ConstString &name, const ClangNamespaceDecl *parent_namespace_decl)
{
- Mutex::Locker locker(m_mutex);
ClangNamespaceDecl namespace_decl;
- if (m_sym_file_ap.get())
- namespace_decl = m_sym_file_ap->FindNamespace (sc, name, parent_namespace_decl);
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
+ {
+ lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+ if (m_sym_file_ap.get())
+ namespace_decl = m_sym_file_ap->FindNamespace (sc, name, parent_namespace_decl);
+ }
return namespace_decl;
}
void
SymbolVendor::Dump(Stream *s)
{
- Mutex::Locker locker(m_mutex);
- bool show_context = false;
-
- s->Printf("%p: ", this);
- s->Indent();
- s->PutCString("SymbolVendor");
- if (m_sym_file_ap.get())
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
{
- ObjectFile *objfile = m_sym_file_ap->GetObjectFile();
- if (objfile)
+ bool show_context = false;
+
+ s->Printf("%p: ", this);
+ s->Indent();
+ s->PutCString("SymbolVendor");
+ if (m_sym_file_ap.get())
{
- const FileSpec &objfile_file_spec = objfile->GetFileSpec();
- if (objfile_file_spec)
+ ObjectFile *objfile = m_sym_file_ap->GetObjectFile();
+ if (objfile)
{
- s->PutCString(" (");
- objfile_file_spec.Dump(s);
- s->PutChar(')');
+ const FileSpec &objfile_file_spec = objfile->GetFileSpec();
+ if (objfile_file_spec)
+ {
+ s->PutCString(" (");
+ objfile_file_spec.Dump(s);
+ s->PutChar(')');
+ }
}
}
+ s->EOL();
+ s->IndentMore();
+ m_type_list.Dump(s, show_context);
+
+ CompileUnitConstIter cu_pos, cu_end;
+ cu_end = m_compile_units.end();
+ for (cu_pos = m_compile_units.begin(); cu_pos != cu_end; ++cu_pos)
+ {
+ // We currently only dump the compile units that have been parsed
+ if (cu_pos->get())
+ (*cu_pos)->Dump(s, show_context);
+ }
+
+ s->IndentLess();
}
- s->EOL();
- s->IndentMore();
- m_type_list.Dump(s, show_context);
-
- CompileUnitConstIter cu_pos, cu_end;
- cu_end = m_compile_units.end();
- for (cu_pos = m_compile_units.begin(); cu_pos != cu_end; ++cu_pos)
- {
- // We currently only dump the compile units that have been parsed
- if (cu_pos->get())
- (*cu_pos)->Dump(s, show_context);
- }
-
- s->IndentLess();
-
}
CompUnitSP
SymbolVendor::GetCompileUnitAtIndex(uint32_t idx)
{
- Mutex::Locker locker(m_mutex);
CompUnitSP cu_sp;
- const uint32_t num_compile_units = GetNumCompileUnits();
- if (idx < num_compile_units)
+ ModuleSP module_sp(GetModule());
+ if (module_sp)
{
- cu_sp = m_compile_units[idx];
- if (cu_sp.get() == NULL)
+ const uint32_t num_compile_units = GetNumCompileUnits();
+ if (idx < num_compile_units)
{
- m_compile_units[idx] = m_sym_file_ap->ParseCompileUnitAtIndex(idx);
cu_sp = m_compile_units[idx];
+ if (cu_sp.get() == NULL)
+ {
+ m_compile_units[idx] = m_sym_file_ap->ParseCompileUnitAtIndex(idx);
+ cu_sp = m_compile_units[idx];
+ }
}
}
return cu_sp;