[Lldb-commits] [lldb] r217818 - use std::atomic<> to protect variables being accessed by multiple threads
Todd Fiala
todd.fiala at gmail.com
Mon Sep 15 13:07:34 PDT 2014
Author: tfiala
Date: Mon Sep 15 15:07:33 2014
New Revision: 217818
URL: http://llvm.org/viewvc/llvm-project?rev=217818&view=rev
Log:
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.
Modified:
lldb/trunk/include/lldb/Core/Communication.h
lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/lldb-log.cpp
Modified: lldb/trunk/include/lldb/Core/Communication.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Communication.h?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Communication.h (original)
+++ lldb/trunk/include/lldb/Core/Communication.h Mon Sep 15 15:07:33 2014
@@ -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...
Modified: lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h (original)
+++ lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h Mon Sep 15 15:07:33 2014
@@ -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);
Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Sep 15 15:07:33 2014
@@ -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
Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp Mon Sep 15 15:07:33 2014
@@ -258,8 +258,7 @@ ProcessFreeBSD::SendMessage(const Proces
case ProcessMessage::eLimboMessage:
case ProcessMessage::eExitMessage:
- m_exit_status = message.GetExitStatus();
- SetExitStatus(m_exit_status, NULL);
+ SetExitStatus(message.GetExitStatus(), NULL);
break;
case ProcessMessage::eSignalMessage:
Modified: lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp Mon Sep 15 15:07:33 2014
@@ -434,8 +434,7 @@ ProcessPOSIX::SendMessage(const ProcessM
// 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);
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Mon Sep 15 15:07:33 2014
@@ -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;
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Sep 15 15:07:33 2014
@@ -688,6 +688,7 @@ Process::Process(Target &target, Listene
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, cons
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 ();
Modified: lldb/trunk/source/lldb-log.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/lldb-log.cpp?rev=217818&r1=217817&r2=217818&view=diff
==============================================================================
--- lldb/trunk/source/lldb-log.cpp (original)
+++ lldb/trunk/source/lldb-log.cpp Mon Sep 15 15:07:33 2014
@@ -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 ()
More information about the lldb-commits
mailing list