[Lldb-commits] [lldb] [lldb] Improve locking in PathMappingLists (NFC) (PR #114576)

[Alex Langford via lldb-commits]

================
@@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
 
 const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
   if (this != &rhs) {
-    std::scoped_lock<std::recursive_mutex, std::recursive_mutex> locks(m_mutex, rhs.m_mutex);
+    std::scoped_lock<std::mutex, std::mutex> pairs_locks(m_pairs_mutex,
+                                                         rhs.m_pairs_mutex);
+    std::scoped_lock<std::mutex, std::mutex> callback_locks(
+        m_callback_mutex, rhs.m_callback_mutex);
----------------
bulbazord wrote:

To future-proof this, we should probably swap the ordering of lock acquisition here. Imagine this scenario:

1. Thread 1 mutates the list. `m_pairs_mutex` is acquired, the work is done, and then it releases the lock.
2. Thread 1 begins the notification process so it grabs `m_callback_mutex` and executes the callback. Simultaneously, the same PathMappingList is used for `operator=` on Thread 2. It will grab the `m_pairs_mutex` and stall on acquiring `m_callback_mutex`.
3. Thread 1's executing a callback that attempts to mutate the underlying PathMappingList, attempting to grab `m_pairs_mutex`. Thread 1 is holding onto `m_callback_mutex` and wants `m_pairs_mutex` and Thread 2 is holding onto `m_pairs_mutex` and wants `m_callback_mutex`. Deadlock.

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