use std::atomic<> to protect variables being accessed by multiple threads

There are several places where multiple threads are accessing the same variables simultaneously without any kind of protection. I propose using std::atomic<> to make it safer. I did a special build of lldb, using the google tool 'thread sanitizer' which identified many cases of multiple threads accessing the same memory. std::atomic is low overhead and does not use any locks for simple types such as int/bool.

See http://reviews.llvm.org/D5302 for more details.

Change by Shawn Best.

llvm-svn: 217818

lldb/include/lldb/Core/Communication.h

@@ -352,7 +352,7 @@ private:
 protected:
     lldb::ConnectionSP m_connection_sp; ///< The connection that is current in use by this communications class.
     HostThread m_read_thread;           ///< The read thread handle in case we need to cancel the thread.
-    bool m_read_thread_enabled;
+    std::atomic<bool> m_read_thread_enabled;
     std::string m_bytes;    ///< A buffer to cache bytes read in the ReadThread function.
     Mutex m_bytes_mutex;    ///< A mutex to protect multi-threaded access to the cached bytes.
     Mutex m_write_mutex;    ///< Don't let multiple threads write at the same time...

lldb/include/lldb/Core/ConnectionFileDescriptor.h

@@ -106,8 +106,8 @@ protected:
 
     Pipe m_pipe;
     Mutex m_mutex;
-    bool m_shutting_down;       // This marks that we are shutting down so if we get woken up from
-                                // BytesAvailable to disconnect, we won't try to read again.
+    std::atomic<bool> m_shutting_down;    // This marks that we are shutting down so if we get woken up from
+                                          // BytesAvailable to disconnect, we won't try to read again.
     bool m_waiting_for_accept;
 private:
     DISALLOW_COPY_AND_ASSIGN (ConnectionFileDescriptor);

lldb/include/lldb/Target/Process.h

@@ -3050,6 +3050,7 @@ protected:
     std::map<uint64_t, uint32_t> m_thread_id_to_index_id_map;
     int                         m_exit_status;          ///< The exit status of the process, or -1 if not set.
     std::string                 m_exit_string;          ///< A textual description of why a process exited.
+    Mutex                       m_exit_status_mutex;    ///< Mutex so m_exit_status m_exit_string can be safely accessed from multiple threads
     Mutex                       m_thread_mutex;
     ThreadList                  m_thread_list_real;     ///< The threads for this process as are known to the protocol we are debugging with
     ThreadList                  m_thread_list;          ///< The threads for this process as the user will see them. This is usually the same as

lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp

@@ -258,8 +258,7 @@ ProcessFreeBSD::SendMessage(const ProcessMessage &message)
 
     case ProcessMessage::eLimboMessage:
     case ProcessMessage::eExitMessage:
-        m_exit_status = message.GetExitStatus();
-        SetExitStatus(m_exit_status, NULL);
+        SetExitStatus(message.GetExitStatus(), NULL);
         break;
 
     case ProcessMessage::eSignalMessage:

lldb/source/Plugins/Process/POSIX/ProcessPOSIX.cpp

@@ -434,8 +434,7 @@ ProcessPOSIX::SendMessage(const ProcessMessage &message)
         // FIXME: I'm not sure we need to do this.
         if (message.GetTID() == GetID())
         {
-            m_exit_status = message.GetExitStatus();
-            SetExitStatus(m_exit_status, NULL);
+            SetExitStatus(message.GetExitStatus(), NULL);
         }
         else if (!IsAThreadRunning())
             SetPrivateState(eStateStopped);

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -328,7 +328,7 @@ protected:
     
     lldb_private::Flags m_flags;            // Process specific flags (see eFlags enums)
     GDBRemoteCommunicationClient m_gdb_comm;
-    lldb::pid_t m_debugserver_pid;
+    std::atomic<lldb::pid_t> m_debugserver_pid;
     StringExtractorGDBRemote m_last_stop_packet;
     lldb_private::Mutex m_last_stop_packet_mutex;
     GDBRemoteDynamicRegisterInfo m_register_info;

lldb/source/Target/Process.cpp

@@ -688,6 +688,7 @@ Process::Process(Target &target, Listener &listener, const UnixSignalsSP &unix_s
     m_thread_id_to_index_id_map (),
     m_exit_status (-1),
     m_exit_string (),
+    m_exit_status_mutex(),
     m_thread_mutex (Mutex::eMutexTypeRecursive),
     m_thread_list_real (this),
     m_thread_list (this),
@@ -1186,6 +1187,8 @@ Process::IsRunning () const
 int
 Process::GetExitStatus ()
 {
+    Mutex::Locker locker (m_exit_status_mutex);
+
     if (m_public_state.GetValue() == eStateExited)
         return m_exit_status;
     return -1;
@@ -1195,6 +1198,8 @@ Process::GetExitStatus ()
 const char *
 Process::GetExitDescription ()
 {
+    Mutex::Locker locker (m_exit_status_mutex);
+
     if (m_public_state.GetValue() == eStateExited && !m_exit_string.empty())
         return m_exit_string.c_str();
     return NULL;
@@ -1219,11 +1224,16 @@ Process::SetExitStatus (int status, const char *cstr)
         return false;
     }
     
-    m_exit_status = status;
-    if (cstr)
-        m_exit_string = cstr;
-    else
-        m_exit_string.clear();
+    // use a mutex to protect the status and string during updating
+    {
+        Mutex::Locker locker (m_exit_status_mutex);
+
+        m_exit_status = status;
+        if (cstr)
+            m_exit_string = cstr;
+        else
+            m_exit_string.clear();
+    }
 
     DidExit ();
 

lldb/source/lldb-log.cpp

@@ -27,7 +27,7 @@ using namespace lldb_private;
 // that will construct the static g_lob_sp the first time this function is 
 // called.
 
-static bool g_log_enabled = false;
+static std::atomic<bool> g_log_enabled {false};
 static Log * g_log = NULL;
 static Log *
 GetLog ()