[lldb-dev] [PATCH] Minor lldb_private::ModuleList fixes

[Piotr Rak]

Hi,

Not 100% sure about initialization (last hunk GetSharedModuleList()), but
as I see it, it was racy, unless it is always protected by something in
Target.

Other two are minor things, with ModuleList::operator= priority inversion
probably never occurring in current code as is. However it might fight back
in future, and since I've noticed it...

Please commit if OK for trunk.

Cheers,
/Piotr
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140323/cbfb906d/attachment.html>
-------------- next part --------------
diff --git a/source/Core/ModuleList.cpp b/source/Core/ModuleList.cpp
index 215611e..d3c803f 100644
--- a/source/Core/ModuleList.cpp
+++ b/source/Core/ModuleList.cpp
@@ -11,6 +11,7 @@
 
 // C Includes
 // C++ Includes
+#include <cstdint>
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/Log.h"
@@ -40,7 +41,8 @@ ModuleList::ModuleList() :
 //----------------------------------------------------------------------
 ModuleList::ModuleList(const ModuleList& rhs) :
     m_modules(),
-    m_modules_mutex (Mutex::eMutexTypeRecursive)
+    m_modules_mutex (Mutex::eMutexTypeRecursive),
+    m_notifier(NULL)
 {
     Mutex::Locker lhs_locker(m_modules_mutex);
     Mutex::Locker rhs_locker(rhs.m_modules_mutex);
@@ -62,9 +64,28 @@ ModuleList::operator= (const ModuleList& rhs)
 {
     if (this != &rhs)
     {
-        Mutex::Locker lhs_locker(m_modules_mutex);
-        Mutex::Locker rhs_locker(rhs.m_modules_mutex);
-        m_modules = rhs.m_modules;
+        // That's probably me nit-picking, but in theoretical situation:
+        //
+        // * that two threads A B and
+        // * two ModuleList's x y do opposite assignemnts ie.:
+        //
+        //  in thread A: | in thread B:
+        //    x = y;     |   y = x;
+        //
+        // This establishes correct(same) lock taking order and thus
+        // avoids priority inversion.
+        if (uintptr_t(this) > uintptr_t(&rhs))
+        {
+            Mutex::Locker lhs_locker(m_modules_mutex);
+            Mutex::Locker rhs_locker(rhs.m_modules_mutex);
+            m_modules = rhs.m_modules;
+        }
+        else
+        {
+            Mutex::Locker rhs_locker(rhs.m_modules_mutex);
+            Mutex::Locker lhs_locker(m_modules_mutex);
+            m_modules = rhs.m_modules;
+        }
     }
     return *this;
 }
@@ -832,6 +853,10 @@ ModuleList::GetIndexForModule (const Module *module) const
 static ModuleList &
 GetSharedModuleList ()
 {
+    // TODO: Replace with pthread_once alike or spinlock...
+    static Mutex g_init_mutex (Mutex::eMutexTypeRecursive);
+
+    Mutex::Locker locker (g_init_mutex);
     // NOTE: Intentionally leak the module list so a program doesn't have to
     // cleanup all modules and object files as it exits. This just wastes time
     // doing a bunch of cleanup that isn't required.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Minor-lldb_private-ModuleList-fixes.patch
Type: text/x-patch
Size: 2812 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140323/cbfb906d/attachment.bin>