[Lldb-commits] [lldb] r227935 - Fixed bugs in the multi-threaded access in HostInfoBase. Prior to this fix, static bool variables were used but this is not sufficient. We now use std::call_once in all places where the previous static bool code was used to try to implement thread safety.

Greg Clayton gclayton at apple.com
Mon Feb 2 18:05:44 PST 2015


Author: gclayton
Date: Mon Feb  2 20:05:44 2015
New Revision: 227935

URL: http://llvm.org/viewvc/llvm-project?rev=227935&view=rev
Log:
Fixed bugs in the multi-threaded access in HostInfoBase. Prior to this fix, static bool variables were used but this is not sufficient. We now use std::call_once in all places where the previous static bool code was used to try to implement thread safety.

This was causing code that opened multiple targets to try and get a path to debugserver from the GDB remote communication class, and it would get the LLDB path and some instances would return empty strings and it would cause debugserver to not be found.

<rdar://problem/18756927>


Modified:
    lldb/trunk/source/Core/ConstString.cpp
    lldb/trunk/source/Expression/ClangModulesDeclVendor.cpp
    lldb/trunk/source/Host/common/HostInfoBase.cpp
    lldb/trunk/source/Host/linux/HostInfoLinux.cpp
    lldb/trunk/source/Host/windows/HostInfoWindows.cpp
    lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Core/ConstString.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ConstString.cpp?rev=227935&r1=227934&r2=227935&view=diff
==============================================================================
--- lldb/trunk/source/Core/ConstString.cpp (original)
+++ lldb/trunk/source/Core/ConstString.cpp Mon Feb  2 20:05:44 2015
@@ -11,7 +11,7 @@
 #include "lldb/Host/Mutex.h"
 #include "llvm/ADT/StringMap.h"
 
-#include <mutex>
+#include <mutex> // std::once
 
 using namespace lldb_private;
 

Modified: lldb/trunk/source/Expression/ClangModulesDeclVendor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/ClangModulesDeclVendor.cpp?rev=227935&r1=227934&r2=227935&view=diff
==============================================================================
--- lldb/trunk/source/Expression/ClangModulesDeclVendor.cpp (original)
+++ lldb/trunk/source/Expression/ClangModulesDeclVendor.cpp Mon Feb  2 20:05:44 2015
@@ -7,8 +7,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "lldb/Core/StreamString.h"
+#include <mutex> // std::once
+
 #include "lldb/Expression/ClangModulesDeclVendor.h"
+
+#include "lldb/Core/StreamString.h"
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
@@ -22,7 +25,6 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Serialization/ASTReader.h"
 
-#include <mutex>
 
 using namespace lldb_private;
 

Modified: lldb/trunk/source/Host/common/HostInfoBase.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=227935&r1=227934&r2=227935&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/HostInfoBase.cpp (original)
+++ lldb/trunk/source/Host/common/HostInfoBase.cpp Mon Feb  2 20:05:44 2015
@@ -23,60 +23,56 @@
 #include "llvm/Support/raw_ostream.h"
 
 #include <thread>
+#include <mutex> // std::once
 
 using namespace lldb;
 using namespace lldb_private;
 
 namespace
 {
-void
-CleanupProcessSpecificLLDBTempDir()
-{
-    // Get the process specific LLDB temporary directory and delete it.
-    FileSpec tmpdir_file_spec;
-    if (!HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec))
-        return;
-
-    // Remove the LLDB temporary directory if we have one. Set "recurse" to
-    // true to all files that were created for the LLDB process can be cleaned up.
-    FileSystem::DeleteDirectory(tmpdir_file_spec.GetDirectory().GetCString(), true);
-}
-
-struct HostInfoBaseFields
-{
-    uint32_t m_number_cpus;
-    std::string m_vendor_string;
-    std::string m_os_string;
-    std::string m_host_triple;
-
-    ArchSpec m_host_arch_32;
-    ArchSpec m_host_arch_64;
-
-    FileSpec m_lldb_so_dir;
-    FileSpec m_lldb_support_exe_dir;
-    FileSpec m_lldb_headers_dir;
-    FileSpec m_lldb_python_dir;
-    FileSpec m_lldb_clang_resource_dir;
-    FileSpec m_lldb_system_plugin_dir;
-    FileSpec m_lldb_user_plugin_dir;
-    FileSpec m_lldb_tmp_dir;
-};
-
-HostInfoBaseFields *g_fields = nullptr;
-}
-
-#define COMPUTE_LLDB_PATH(compute_function, member_var)                                                                                    \
-    {                                                                                                                                      \
-        static bool is_initialized = false;                                                                                                \
-        static bool success = false;                                                                                                       \
-        if (!is_initialized)                                                                                                               \
-        {                                                                                                                                  \
-            is_initialized = true;                                                                                                         \
-            success = HostInfo::compute_function(member_var);                                                                              \
-        }                                                                                                                                  \
-        if (success)                                                                                                                       \
-            result = &member_var;                                                                                                          \
-    }
+    void
+    CleanupProcessSpecificLLDBTempDir()
+    {
+        // Get the process specific LLDB temporary directory and delete it.
+        FileSpec tmpdir_file_spec;
+        if (!HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec))
+            return;
+
+        // Remove the LLDB temporary directory if we have one. Set "recurse" to
+        // true to all files that were created for the LLDB process can be cleaned up.
+        FileSystem::DeleteDirectory(tmpdir_file_spec.GetDirectory().GetCString(), true);
+    }
+
+    //----------------------------------------------------------------------
+    // The HostInfoBaseFields is a work around for windows not supporting
+    // static variables correctly in a thread safe way. Really each of the
+    // variables in HostInfoBaseFields should live in the functions in which
+    // they are used and each one should be static, but the work around is
+    // in place to avoid this restriction. Ick.
+    //----------------------------------------------------------------------
+
+    struct HostInfoBaseFields
+    {
+        uint32_t m_number_cpus;
+        std::string m_vendor_string;
+        std::string m_os_string;
+        std::string m_host_triple;
+
+        ArchSpec m_host_arch_32;
+        ArchSpec m_host_arch_64;
+
+        FileSpec m_lldb_so_dir;
+        FileSpec m_lldb_support_exe_dir;
+        FileSpec m_lldb_headers_dir;
+        FileSpec m_lldb_python_dir;
+        FileSpec m_lldb_clang_resource_dir;
+        FileSpec m_lldb_system_plugin_dir;
+        FileSpec m_lldb_user_plugin_dir;
+        FileSpec m_lldb_tmp_dir;
+    };
+    
+    HostInfoBaseFields *g_fields = nullptr;
+}
 
 void
 HostInfoBase::Initialize()
@@ -87,13 +83,10 @@ HostInfoBase::Initialize()
 uint32_t
 HostInfoBase::GetNumberCPUS()
 {
-    static bool is_initialized = false;
-    if (!is_initialized)
-    {
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
         g_fields->m_number_cpus = std::thread::hardware_concurrency();
-        is_initialized = true;
-    }
-
+    });
     return g_fields->m_number_cpus;
 }
 
@@ -106,53 +99,40 @@ HostInfoBase::GetMaxThreadNameLength()
 llvm::StringRef
 HostInfoBase::GetVendorString()
 {
-    static bool is_initialized = false;
-    if (!is_initialized)
-    {
-        const ArchSpec &host_arch = HostInfo::GetArchitecture();
-        const llvm::StringRef &str_ref = host_arch.GetTriple().getVendorName();
-        g_fields->m_vendor_string.assign(str_ref.begin(), str_ref.end());
-        is_initialized = true;
-    }
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
+        g_fields->m_vendor_string = std::move(HostInfo::GetArchitecture().GetTriple().getVendorName().str());
+    });
     return g_fields->m_vendor_string;
 }
 
 llvm::StringRef
 HostInfoBase::GetOSString()
 {
-    static bool is_initialized = false;
-    if (!is_initialized)
-    {
-        const ArchSpec &host_arch = HostInfo::GetArchitecture();
-        const llvm::StringRef &str_ref = host_arch.GetTriple().getOSName();
-        g_fields->m_os_string.assign(str_ref.begin(), str_ref.end());
-        is_initialized = true;
-    }
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
+        g_fields->m_os_string = std::move(HostInfo::GetArchitecture().GetTriple().getOSName());
+    });
     return g_fields->m_os_string;
 }
 
 llvm::StringRef
 HostInfoBase::GetTargetTriple()
 {
-    static bool is_initialized = false;
-    if (!is_initialized)
-    {
-        const ArchSpec &host_arch = HostInfo::GetArchitecture();
-        g_fields->m_host_triple = host_arch.GetTriple().getTriple();
-        is_initialized = true;
-    }
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
+        g_fields->m_host_triple = HostInfo::GetArchitecture().GetTriple().getTriple();
+    });
     return g_fields->m_host_triple;
 }
 
 const ArchSpec &
 HostInfoBase::GetArchitecture(ArchitectureKind arch_kind)
 {
-    static bool is_initialized = false;
-    if (!is_initialized)
-    {
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
         HostInfo::ComputeHostArchitectureSupport(g_fields->m_host_arch_32, g_fields->m_host_arch_64);
-        is_initialized = true;
-    }
+    });
 
     // If an explicit 32 or 64-bit architecture was requested, return that.
     if (arch_kind == eArchKind32)
@@ -174,52 +154,123 @@ HostInfoBase::GetLLDBPath(lldb::PathType
         return false;
 #endif
 
-    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
     FileSpec *result = nullptr;
     switch (type)
     {
         case lldb::ePathTypeLLDBShlibDir:
-            COMPUTE_LLDB_PATH(ComputeSharedLibraryDirectory, g_fields->m_lldb_so_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBShlibDir) => '%s'", g_fields->m_lldb_so_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeSharedLibraryDirectory (g_fields->m_lldb_so_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBShlibDir) => '%s'", g_fields->m_lldb_so_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_so_dir;
+            }
             break;
         case lldb::ePathTypeSupportExecutableDir:
-            COMPUTE_LLDB_PATH(ComputeSupportExeDirectory, g_fields->m_lldb_support_exe_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeSupportExecutableDir) => '%s'",
-                            g_fields->m_lldb_support_exe_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeSupportExeDirectory (g_fields->m_lldb_support_exe_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeSupportExecutableDir) => '%s'",
+                                    g_fields->m_lldb_support_exe_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_support_exe_dir;
+            }
             break;
         case lldb::ePathTypeHeaderDir:
-            COMPUTE_LLDB_PATH(ComputeHeaderDirectory, g_fields->m_lldb_headers_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeHeaderDir) => '%s'", g_fields->m_lldb_headers_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeHeaderDirectory (g_fields->m_lldb_headers_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeHeaderDir) => '%s'", g_fields->m_lldb_headers_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_headers_dir;
+            }
             break;
         case lldb::ePathTypePythonDir:
-            COMPUTE_LLDB_PATH(ComputePythonDirectory, g_fields->m_lldb_python_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypePythonDir) => '%s'", g_fields->m_lldb_python_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputePythonDirectory (g_fields->m_lldb_python_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypePythonDir) => '%s'", g_fields->m_lldb_python_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_python_dir;
+            }
             break;
         case lldb::ePathTypeClangDir:
-            COMPUTE_LLDB_PATH(ComputeClangDirectory, g_fields->m_lldb_clang_resource_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeClangResourceDir) => '%s'", g_fields->m_lldb_clang_resource_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeClangDirectory (g_fields->m_lldb_clang_resource_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeClangResourceDir) => '%s'", g_fields->m_lldb_clang_resource_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_clang_resource_dir;
+            }
             break;
         case lldb::ePathTypeLLDBSystemPlugins:
-            COMPUTE_LLDB_PATH(ComputeSystemPluginsDirectory, g_fields->m_lldb_system_plugin_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBSystemPlugins) => '%s'",
-                            g_fields->m_lldb_system_plugin_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeSystemPluginsDirectory (g_fields->m_lldb_system_plugin_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBSystemPlugins) => '%s'",
+                                    g_fields->m_lldb_system_plugin_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_system_plugin_dir;
+            }
             break;
         case lldb::ePathTypeLLDBUserPlugins:
-            COMPUTE_LLDB_PATH(ComputeUserPluginsDirectory, g_fields->m_lldb_user_plugin_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBUserPlugins) => '%s'",
-                            g_fields->m_lldb_user_plugin_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeUserPluginsDirectory (g_fields->m_lldb_user_plugin_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBUserPlugins) => '%s'",
+                                    g_fields->m_lldb_user_plugin_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_user_plugin_dir;
+            }
             break;
         case lldb::ePathTypeLLDBTempSystemDir:
-            COMPUTE_LLDB_PATH(ComputeTempFileDirectory, g_fields->m_lldb_tmp_dir)
-            if (log)
-                log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBTempSystemDir) => '%s'", g_fields->m_lldb_tmp_dir.GetPath().c_str());
+            {
+                static std::once_flag g_once_flag;
+                static bool success = false;
+                std::call_once(g_once_flag,  []() {
+                    success = HostInfo::ComputeTempFileDirectory (g_fields->m_lldb_tmp_dir);
+                    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+                    if (log)
+                        log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBTempSystemDir) => '%s'", g_fields->m_lldb_tmp_dir.GetPath().c_str());
+                });
+                if (success)
+                    result = &g_fields->m_lldb_tmp_dir;
+            }
             break;
     }
 

Modified: lldb/trunk/source/Host/linux/HostInfoLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/HostInfoLinux.cpp?rev=227935&r1=227934&r2=227935&view=diff
==============================================================================
--- lldb/trunk/source/Host/linux/HostInfoLinux.cpp (original)
+++ lldb/trunk/source/Host/linux/HostInfoLinux.cpp Mon Feb  2 20:05:44 2015
@@ -16,6 +16,7 @@
 #include <sys/utsname.h>
 
 #include <algorithm>
+#include <mutex> // std::once
 
 using namespace lldb_private;
 
@@ -56,32 +57,29 @@ HostInfoLinux::GetMaxThreadNameLength()
 bool
 HostInfoLinux::GetOSVersion(uint32_t &major, uint32_t &minor, uint32_t &update)
 {
-    static bool is_initialized = false;
     static bool success = false;
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
 
-    if (!is_initialized)
-    {
-        is_initialized = true;
         struct utsname un;
-
-        if (uname(&un))
-            goto finished;
-
-        int status = sscanf(un.release, "%u.%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor, &g_fields->m_os_update);
-        if (status == 3)
+        if (uname(&un) == 0)
         {
-            success = true;
-            goto finished;
+            int status = sscanf(un.release, "%u.%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor, &g_fields->m_os_update);
+            if (status == 3)
+                success = true;
+            else
+            {
+                // Some kernels omit the update version, so try looking for just "X.Y" and
+                // set update to 0.
+                g_fields->m_os_update = 0;
+                status = sscanf(un.release, "%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor);
+                if (status == 2)
+                    success = true;
+            }
         }
+    });
 
-        // Some kernels omit the update version, so try looking for just "X.Y" and
-        // set update to 0.
-        g_fields->m_os_update = 0;
-        status = sscanf(un.release, "%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor);
-        success = !!(status == 2);
-    }
 
-finished:
     major = g_fields->m_os_major;
     minor = g_fields->m_os_minor;
     update = g_fields->m_os_update;
@@ -120,13 +118,11 @@ HostInfoLinux::GetOSKernelDescription(st
 llvm::StringRef
 HostInfoLinux::GetDistributionId()
 {
-    static bool is_initialized = false;
     // Try to run 'lbs_release -i', and use that response
     // for the distribution id.
-
-    if (!is_initialized)
-    {
-        is_initialized = true;
+    static bool success = false;
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
 
         Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
         if (log)
@@ -202,7 +198,7 @@ HostInfoLinux::GetDistributionId()
             // clean up the file
             pclose(file);
         }
-    }
+    });
 
     return g_fields->m_distribution_id.c_str();
 }

Modified: lldb/trunk/source/Host/windows/HostInfoWindows.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/HostInfoWindows.cpp?rev=227935&r1=227934&r2=227935&view=diff
==============================================================================
--- lldb/trunk/source/Host/windows/HostInfoWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/HostInfoWindows.cpp Mon Feb  2 20:05:44 2015
@@ -9,6 +9,8 @@
 
 #include "lldb/Host/windows/windows.h"
 
+#include <mutex> // std::once
+
 #include "lldb/Host/windows/HostInfoWindows.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
@@ -84,14 +86,11 @@ HostInfoWindows::GetHostname(std::string
 FileSpec
 HostInfoWindows::GetProgramFileSpec()
 {
-    static bool is_initialized = false;
-    if (!is_initialized)
-    {
-        is_initialized = true;
-
-        std::vector<char> buffer(PATH_MAX);
-        ::GetModuleFileName(NULL, &buffer[0], buffer.size());
-        m_program_filespec.SetFile(&buffer[0], false);
+    static std::once_flag g_once_flag;
+    std::call_once(g_once_flag,  []() {
+        char buffer[PATH_MAX];
+        ::GetModuleFileName(NULL, buffer, sizeof(buffer));
+        m_program_filespec.SetFile(buffer, false);
     }
     return m_program_filespec;
 }

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=227935&r1=227934&r2=227935&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Mon Feb  2 20:05:44 2015
@@ -11,7 +11,7 @@
 
 // C Includes
 // C++ Includes
-#include <mutex>
+#include <mutex> // std::once
 #include <string>
 
 // Other libraries and framework includes





More information about the lldb-commits mailing list