[Lldb-commits] [lldb] [LLDB] Acquire the module mutex at the start of SetLoadAddress (PR #227149)

[Yao Qi via lldb-commits]

qiyao wrote:

> Fixes a potential dead-lock from parallel module loading.
> 
> I received a quick-stack of LLDB hung loading a core with parallel module loading enabled, where two threads were trying to mutate a given module and an object file, but having acquired the module mutex first in one case, and the object file's section mutex first in the second case, which each trying to subsequently acquire the other lock.
> 
> In the update case, [SetLoadAddress acquires the section list mutex and then tries to acquire the module mutex](https://github.com/llvm/llvm-project/blob/60f717946cb5ca911b6be22169b9bd646225c59f/lldb/source/Symbol/ObjectFile.cpp#L614)
> 

> To fix this I have module acquire it's own mutex when calling into a method which may subsequently try to acquire the mutex again in the object file's critical section. This also fits with my mental module of accessing object files, which starts in the module and then calls into the object file. I do find it a bit weird the object file acquires the lock, but I haven't actually looked at any old commits to figure out why.

Here is the lock ordering rule:

https://github.com/llvm/llvm-project/blob/86a445a3f6e3ee145fd9523a6360626b1df99fd1/lldb/include/lldb/Core/Module.h#L843-L845

`Module` is the owner, it holds a `SymbolFile` and an `ObjectFile`. The rule is coarse before fine, owner before owned: code that belongs to the owned object takes the owner's bigger lock first, then its own narrower one, never the reverse.  In your case, `ObjectFileELF::SetLoadAddress` (calls `ObjectFile::GetSectionList`) violates the rule and acquires  section list mutex first and then module mutex.

https://github.com/llvm/llvm-project/blob/180b69dfdf76a1d42cc00489ce3c885f4c434a29/lldb/source/Symbol/ObjectFile.cpp#L613-L619

So, the right fix should be holding module mutex first, and then sections mutex in `ObjectFile::GetSectionList`.

https://github.com/llvm/llvm-project/pull/227149