[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

[Greg Clayton via Phabricator via lldb-commits]

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