clayborg added a comment.

In https://reviews.llvm.org/D48393#1244426, @JDevlieghere wrote:

> Thanks for the information, Greg!
>
> In https://reviews.llvm.org/D48393#1243588, @clayborg wrote:
>
> > A little background might help here: The lldb_private::Module lock is used 
> > to prevent multiple queries into the DWARF from stomping on each other.
> >
> > Multi-threaded DWARF parsing was primarily added to speed up indexing and 
> > the only place it is used. Is that not true? Indexing is just creating name 
> > tables and the accelerator tables we need so that we can partially parse 
> > the DWARF later. No type parsing should be happening when indexing. All 
> > other accesses to the DWARF must be done via a SymbolFile API that takes 
> > the module lock to stop multiple threads from stomping on each other. So my 
> > main point is we need to use the module lock to avoid having multiple 
> > threads doing important work that will cause crashes.
>
>
> Looking at `SymbolFileDWARF`, we only have two methods that take the module 
> lock: `PreloadSymbols` and `CompleteType`.


Most of the locking happens at the SymbolVendor level IIRC. But all virtual 
function SymbolFile entry points need to take the module lock.

> 
> 
>> The only multi-threaded access to DWARF currently should be in the indexing 
>> only and no important parsing stuff should be going on.
> 
> Surely there can be a lot more going on, like two debuggers executing an 
> expression at the same time?

That doesn't matter. One of them will index the DWARF. Both will be able to 
field requests via the virtual functions defined in lldb_private::SymbolFile 
(usually via the lldb_private::SymbolVendor (a class that can combine multiple 
object files/symbol files)) so any query must use the module lock. This is no 
different than two threads making two different requests from a SymbolFileDWARF.

> 
> 
>> If we discover places where multiple threads are making accesses, we just 
>> need to ensure they take the module lock and everything should just work.
> 
> So what you're saying is that we should add the module lock to every endpoint 
> in `SymbolFileDWARF` that can potentially modify our internal data structures 
> (i.e. parsing)?

Yes, any virtual lldb_private::SymbolFile entry point should lock the recursive 
module mutex. Check SymbolVendor as no one directly grabs a SymbolFile and uses 
it, they all go through the module which uses the symbol vendor. So the locking 
will often occur at the module level or the symbol vendor level.


https://reviews.llvm.org/D48393



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to