[Lldb-commits] [lldb] c78e65c - [lldb][plugin] Use counter directly for number of readers (#139252)

[via lldb-commits]

Author: Jacques Pienaar
Date: 2025-05-13T01:52:36-07:00
New Revision: c78e65cc980db9542b32049a5d96b00c64cbc47d

URL: https://github.com/llvm/llvm-project/commit/c78e65cc980db9542b32049a5d96b00c64cbc47d
DIFF: https://github.com/llvm/llvm-project/commit/c78e65cc980db9542b32049a5d96b00c64cbc47d.diff

LOG: [lldb][plugin] Use counter directly for number of readers (#139252)

Here we were initializing & locking a shared_mutex in a thread, while
releasing it in the parent which may/often turned out to be a different
thread (shared_mutex::unlock_shared is undefined behavior if called from
a thread that doesn't hold the lock).

Switch to counter to more simply keep track of number of readers and
simply lock/unlock rather than utilizing reader mutex to verify last
freed (and so requiring this matching thread init/destroy behavior).

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
    lldb/test/API/tools/lldb-dap/console/TestDAP_console.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 7d0afc04ac3b6..ffd6f1dd52aff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -189,21 +189,20 @@ DWARFUnit::ScopedExtractDIEs DWARFUnit::ExtractDIEsScoped() {
 }
 
 DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit &cu) : m_cu(&cu) {
-  m_cu->m_die_array_scoped_mutex.lock_shared();
+  llvm::sys::ScopedLock lock(m_cu->m_die_array_scoped_mutex);
+  ++m_cu->m_die_array_scoped_count;
 }
 
 DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs() {
   if (!m_cu)
     return;
-  m_cu->m_die_array_scoped_mutex.unlock_shared();
-  if (!m_clear_dies || m_cu->m_cancel_scopes)
-    return;
-  // Be sure no other ScopedExtractDIEs is running anymore.
-  llvm::sys::ScopedWriter lock_scoped(m_cu->m_die_array_scoped_mutex);
-  llvm::sys::ScopedWriter lock(m_cu->m_die_array_mutex);
-  if (m_cu->m_cancel_scopes)
-    return;
-  m_cu->ClearDIEsRWLocked();
+  llvm::sys::ScopedLock lock(m_cu->m_die_array_scoped_mutex);
+  --m_cu->m_die_array_scoped_count;
+  if (m_cu->m_die_array_scoped_count == 0 && m_clear_dies &&
+      !m_cu->m_cancel_scopes) {
+    llvm::sys::ScopedWriter lock(m_cu->m_die_array_mutex);
+    m_cu->ClearDIEsRWLocked();
+  }
 }
 
 DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(ScopedExtractDIEs &&rhs)

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 75a003e0a663c..c05bba36ed74b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -17,6 +17,7 @@
 #include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugRnglists.h"
+#include "llvm/Support/Mutex.h"
 #include "llvm/Support/RWMutex.h"
 #include <atomic>
 #include <optional>
@@ -328,7 +329,8 @@ class DWARFUnit : public DWARFExpression::Delegate, public UserID {
   DWARFDebugInfoEntry::collection m_die_array;
   mutable llvm::sys::RWMutex m_die_array_mutex;
   // It is used for tracking of ScopedExtractDIEs instances.
-  mutable llvm::sys::RWMutex m_die_array_scoped_mutex;
+  mutable llvm::sys::Mutex m_die_array_scoped_mutex;
+  mutable int m_die_array_scoped_count = 0;
   // ScopedExtractDIEs instances should not call ClearDIEsRWLocked()
   // as someone called ExtractDIEsIfNeeded().
   std::atomic<bool> m_cancel_scopes;

diff  --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index 8642e317f9b3a..9cdb978368cc1 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -176,7 +176,7 @@ def test_diagnositcs(self):
             f"target create --core  {core}", context="repl"
         )
 
-        output = self.get_important()
+        output = self.get_important(timeout=2.0)
         self.assertIn(
             "warning: unable to retrieve process ID from minidump file",
             output,