Clean up vestigial remnants of locking primitives
This finally removes the use of the Mutex and Condition classes. This is an
intricate patch as the Mutex and Condition classes were tied together.
Furthermore, many places had slightly differing uses of time values. Convert
timeout values to relative everywhere to permit the use of
std::chrono::duration, which is required for the use of
std::condition_variable's timeout. Adjust all Condition and related Mutex
classes over to std::{,recursive_}mutex and std::condition_variable.
This change primarily comes at the cost of breaking the TracingMutex which was
based around the Mutex class. It would be possible to write a wrapper to
provide similar functionality, but that is beyond the scope of this change.
llvm-svn: 277011
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index f164b14..9d6b976f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -152,23 +152,22 @@
//----------------------------------------------------------------------
// GDBRemoteCommunication constructor
//----------------------------------------------------------------------
-GDBRemoteCommunication::GDBRemoteCommunication(const char *comm_name,
- const char *listener_name) :
- Communication(comm_name),
+GDBRemoteCommunication::GDBRemoteCommunication(const char *comm_name, const char *listener_name)
+ : Communication(comm_name),
#ifdef LLDB_CONFIGURATION_DEBUG
- m_packet_timeout (1000),
+ m_packet_timeout(1000),
#else
- m_packet_timeout (1),
+ m_packet_timeout(1),
#endif
- m_echo_number(0),
- m_supports_qEcho (eLazyBoolCalculate),
- m_sequence_mutex (Mutex::eMutexTypeRecursive),
- m_public_is_running (false),
- m_private_is_running (false),
- m_history (512),
- m_send_acks (true),
- m_compression_type (CompressionType::None),
- m_listen_url ()
+ m_echo_number(0),
+ m_supports_qEcho(eLazyBoolCalculate),
+ m_sequence_mutex(),
+ m_public_is_running(false),
+ m_private_is_running(false),
+ m_history(512),
+ m_send_acks(true),
+ m_compression_type(CompressionType::None),
+ m_listen_url()
{
}
@@ -229,7 +228,7 @@
GDBRemoteCommunication::PacketResult
GDBRemoteCommunication::SendPacket (const char *payload, size_t payload_length)
{
- Mutex::Locker locker(m_sequence_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_sequence_mutex);
return SendPacketNoLock (payload, payload_length);
}
@@ -323,20 +322,19 @@
}
bool
-GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker, const char *failure_message)
+GDBRemoteCommunication::GetSequenceMutex(std::unique_lock<std::recursive_mutex> &lock, const char *failure_message)
{
if (IsRunning())
- return locker.TryLock (m_sequence_mutex, failure_message);
+ return (lock = std::unique_lock<std::recursive_mutex>(m_sequence_mutex, std::try_to_lock)).owns_lock();
- locker.Lock (m_sequence_mutex);
+ lock = std::unique_lock<std::recursive_mutex>(m_sequence_mutex);
return true;
}
-
bool
-GDBRemoteCommunication::WaitForNotRunningPrivate (const TimeValue *timeout_ptr)
+GDBRemoteCommunication::WaitForNotRunningPrivate(const std::chrono::microseconds &timeout)
{
- return m_private_is_running.WaitForValueEqualTo (false, timeout_ptr, NULL);
+ return m_private_is_running.WaitForValueEqualTo(false, timeout, NULL);
}
GDBRemoteCommunication::PacketResult
@@ -356,20 +354,22 @@
GDBRemoteCommunication::PacketResult
GDBRemoteCommunication::PopPacketFromQueue (StringExtractorGDBRemote &response, uint32_t timeout_usec)
{
- // Calculate absolute timeout value
- TimeValue timeout = TimeValue::Now();
- timeout.OffsetWithMicroSeconds(timeout_usec);
+ auto until = std::chrono::system_clock::now() + std::chrono::microseconds(timeout_usec);
- do
+ while (true)
{
// scope for the mutex
{
// lock down the packet queue
- Mutex::Locker locker(m_packet_queue_mutex);
+ std::unique_lock<std::mutex> lock(m_packet_queue_mutex);
// Wait on condition variable.
if (m_packet_queue.size() == 0)
- m_condition_queue_not_empty.Wait(m_packet_queue_mutex, &timeout);
+ {
+ std::cv_status result = m_condition_queue_not_empty.wait_until(lock, until);
+ if (result == std::cv_status::timeout)
+ break;
+ }
if (m_packet_queue.size() > 0)
{
@@ -389,7 +389,7 @@
return PacketResult::ErrorDisconnected;
// Loop while not timed out
- } while (TimeValue::Now() < timeout);
+ }
return PacketResult::ErrorReplyTimeout;
}
@@ -1479,12 +1479,11 @@
// scope for the mutex
{
// lock down the packet queue
- Mutex::Locker locker(m_packet_queue_mutex);
+ std::lock_guard<std::mutex> guard(m_packet_queue_mutex);
// push a new packet into the queue
m_packet_queue.push(packet);
// Signal condition variable that we have a packet
- m_condition_queue_not_empty.Signal();
-
+ m_condition_queue_not_empty.notify_one();
}
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 2a01bce..23568cd 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -12,6 +12,8 @@
// C Includes
// C++ Includes
+#include <condition_variable>
+#include <mutex>
#include <string>
#include <queue>
#include <vector>
@@ -22,7 +24,6 @@
#include "lldb/Core/Communication.h"
#include "lldb/Core/Listener.h"
#include "lldb/Host/HostThread.h"
-#include "lldb/Host/Mutex.h"
#include "lldb/Host/Predicate.h"
#include "lldb/Host/TimeValue.h"
#include "lldb/Interpreter/Args.h"
@@ -114,7 +115,7 @@
size_t payload_length);
bool
- GetSequenceMutex(Mutex::Locker& locker, const char *failure_message = nullptr);
+ GetSequenceMutex(std::unique_lock<std::recursive_mutex> &lock, const char *failure_message = nullptr);
PacketType
CheckForPacket (const uint8_t *src,
@@ -285,9 +286,9 @@
uint32_t m_echo_number;
LazyBool m_supports_qEcho;
#ifdef ENABLE_MUTEX_ERROR_CHECKING
- TrackingMutex m_sequence_mutex;
+#error TrackingMutex is no longer supported
#else
- Mutex m_sequence_mutex; // Restrict access to sending/receiving packets to a single thread at a time
+ std::recursive_mutex m_sequence_mutex; // Restrict access to sending/receiving packets to a single thread at a time
#endif
Predicate<bool> m_public_is_running;
Predicate<bool> m_private_is_running;
@@ -320,7 +321,7 @@
bool sync_on_timeout);
bool
- WaitForNotRunningPrivate (const TimeValue *timeout_ptr);
+ WaitForNotRunningPrivate(const std::chrono::microseconds &timeout);
bool
CompressionIsEnabled ()
@@ -364,8 +365,8 @@
private:
std::queue<StringExtractorGDBRemote> m_packet_queue; // The packet queue
- lldb_private::Mutex m_packet_queue_mutex; // Mutex for accessing queue
- Condition m_condition_queue_not_empty; // Condition variable to wait for packets
+ std::mutex m_packet_queue_mutex; // Mutex for accessing queue
+ std::condition_variable m_condition_queue_not_empty; // Condition variable to wait for packets
HostThread m_listen_thread;
std::string m_listen_url;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index ba018be..a9ecd93 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -720,9 +720,10 @@
std::string &response_string
)
{
- Mutex::Locker locker;
- if (!GetSequenceMutex(locker,
- "ProcessGDBRemote::SendPacketsAndConcatenateResponses() failed due to not getting the sequence mutex"))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (!GetSequenceMutex(
+ lock,
+ "ProcessGDBRemote::SendPacketsAndConcatenateResponses() failed due to not getting the sequence mutex"))
{
Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS));
if (log)
@@ -821,7 +822,7 @@
)
{
PacketResult packet_result = PacketResult::ErrorSendFailed;
- Mutex::Locker locker;
+ std::unique_lock<std::recursive_mutex> lock;
Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
// In order to stop async notifications from being processed in the middle of the
@@ -829,7 +830,7 @@
static ListenerSP hijack_listener_sp(Listener::MakeListener("lldb.NotifyHijacker"));
HijackBroadcaster(hijack_listener_sp, eBroadcastBitGdbReadThreadGotNotify);
- if (GetSequenceMutex (locker))
+ if (GetSequenceMutex(lock))
{
packet_result = SendPacketAndWaitForResponseNoLock (payload, payload_length, response);
}
@@ -848,19 +849,22 @@
log->Printf ("async: async packet = %s", m_async_packet.c_str());
bool timed_out = false;
- if (SendInterrupt(locker, 2, timed_out))
+ if (SendInterrupt(lock, 2, timed_out))
{
if (m_interrupt_sent)
{
m_interrupt_sent = false;
- TimeValue timeout_time;
- timeout_time = TimeValue::Now();
- timeout_time.OffsetWithSeconds (m_packet_timeout);
+
+ std::chrono::time_point<std::chrono::system_clock> until;
+ until = std::chrono::system_clock::now() + std::chrono::seconds(m_packet_timeout);
if (log)
log->Printf ("async: sent interrupt");
- if (m_async_packet_predicate.WaitForValueEqualTo (false, &timeout_time, &timed_out))
+ if (m_async_packet_predicate.WaitForValueEqualTo(
+ false, std::chrono::duration_cast<std::chrono::microseconds>(
+ until - std::chrono::system_clock::now()),
+ &timed_out))
{
if (log)
log->Printf ("async: got response");
@@ -876,7 +880,10 @@
}
// Make sure we wait until the continue packet has been sent again...
- if (m_private_is_running.WaitForValueEqualTo (true, &timeout_time, &timed_out))
+ if (m_private_is_running.WaitForValueEqualTo(
+ true, std::chrono::duration_cast<std::chrono::microseconds>(
+ until - std::chrono::system_clock::now()),
+ &timed_out))
{
if (log)
{
@@ -1045,7 +1052,7 @@
log->Printf("GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
// we want to lock down packet sending while we continue
- Mutex::Locker locker(m_sequence_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_sequence_mutex);
// here we broadcast this before we even send the packet!!
// this signals doContinue() to exit
@@ -1094,7 +1101,7 @@
if (log)
log->Printf ("GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
- Mutex::Locker locker(m_sequence_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_sequence_mutex);
StateType state = eStateRunning;
m_public_is_running.SetValue (true, eBroadcastNever);
@@ -1394,8 +1401,8 @@
std::lock_guard<std::recursive_mutex> guard(m_async_mutex);
m_async_signal = signo;
bool timed_out = false;
- Mutex::Locker locker;
- if (SendInterrupt (locker, 1, timed_out))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (SendInterrupt(lock, 1, timed_out))
return true;
m_async_signal = -1;
return false;
@@ -1412,12 +1419,8 @@
// (gdb remote protocol requires this), and do what we need to do, then resume.
bool
-GDBRemoteCommunicationClient::SendInterrupt
-(
- Mutex::Locker& locker,
- uint32_t seconds_to_wait_for_stop,
- bool &timed_out
-)
+GDBRemoteCommunicationClient::SendInterrupt(std::unique_lock<std::recursive_mutex> &lock,
+ uint32_t seconds_to_wait_for_stop, bool &timed_out)
{
timed_out = false;
Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS));
@@ -1425,7 +1428,7 @@
if (IsRunning())
{
// Only send an interrupt if our debugserver is running...
- if (GetSequenceMutex (locker))
+ if (GetSequenceMutex(lock))
{
if (log)
log->Printf ("SendInterrupt () - got sequence mutex without having to interrupt");
@@ -1444,13 +1447,8 @@
m_interrupt_sent = true;
if (seconds_to_wait_for_stop)
{
- TimeValue timeout;
- if (seconds_to_wait_for_stop)
- {
- timeout = TimeValue::Now();
- timeout.OffsetWithSeconds (seconds_to_wait_for_stop);
- }
- if (m_private_is_running.WaitForValueEqualTo (false, &timeout, &timed_out))
+ if (m_private_is_running.WaitForValueEqualTo(false, std::chrono::seconds(seconds_to_wait_for_stop),
+ &timed_out))
{
if (log)
log->PutCString ("SendInterrupt () - sent interrupt, private state stopped");
@@ -3653,10 +3651,10 @@
GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thread_ids,
bool &sequence_mutex_unavailable)
{
- Mutex::Locker locker;
+ std::unique_lock<std::recursive_mutex> lock;
thread_ids.clear();
-
- if (GetSequenceMutex (locker, "ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex"))
+
+ if (GetSequenceMutex(lock, "ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex"))
{
sequence_mutex_unavailable = false;
StringExtractorGDBRemote response;
@@ -4203,8 +4201,8 @@
bool
GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, StringExtractorGDBRemote &response)
{
- Mutex::Locker locker;
- if (GetSequenceMutex (locker, "Didn't get sequence mutex for p packet."))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (GetSequenceMutex(lock, "Didn't get sequence mutex for p packet."))
{
const bool thread_suffix_supported = GetThreadSuffixSupported();
@@ -4228,8 +4226,8 @@
bool
GDBRemoteCommunicationClient::ReadAllRegisters (lldb::tid_t tid, StringExtractorGDBRemote &response)
{
- Mutex::Locker locker;
- if (GetSequenceMutex (locker, "Didn't get sequence mutex for g packet."))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (GetSequenceMutex(lock, "Didn't get sequence mutex for g packet."))
{
const bool thread_suffix_supported = GetThreadSuffixSupported();
@@ -4256,8 +4254,8 @@
return false;
m_supports_QSaveRegisterState = eLazyBoolYes;
- Mutex::Locker locker;
- if (GetSequenceMutex (locker, "Didn't get sequence mutex for QSaveRegisterState."))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (GetSequenceMutex(lock, "Didn't get sequence mutex for QSaveRegisterState."))
{
const bool thread_suffix_supported = GetThreadSuffixSupported();
if (thread_suffix_supported || SetCurrentThread(tid))
@@ -4298,9 +4296,9 @@
// order to be useful
if (m_supports_QSaveRegisterState == eLazyBoolNo)
return false;
-
- Mutex::Locker locker;
- if (GetSequenceMutex (locker, "Didn't get sequence mutex for QRestoreRegisterState."))
+
+ std::unique_lock<std::recursive_mutex> lock;
+ if (GetSequenceMutex(lock, "Didn't get sequence mutex for QRestoreRegisterState."))
{
const bool thread_suffix_supported = GetThreadSuffixSupported();
if (thread_suffix_supported || SetCurrentThread(tid))
@@ -4530,8 +4528,10 @@
if (m_supports_qSymbol && m_qSymbol_requests_done == false)
{
- Mutex::Locker locker;
- if (GetSequenceMutex(locker, "GDBRemoteCommunicationClient::ServeSymbolLookups() failed due to not getting the sequence mutex"))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (GetSequenceMutex(
+ lock,
+ "GDBRemoteCommunicationClient::ServeSymbolLookups() failed due to not getting the sequence mutex"))
{
StreamString packet;
packet.PutCString ("qSymbol::");
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 7088bc0..a603fcf 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -105,9 +105,7 @@
SendAsyncSignal (int signo);
bool
- SendInterrupt (Mutex::Locker &locker,
- uint32_t seconds_to_wait_for_stop,
- bool &timed_out);
+ SendInterrupt(std::unique_lock<std::recursive_mutex> &lock, uint32_t seconds_to_wait_for_stop, bool &timed_out);
lldb::pid_t
GetCurrentProcessID (bool allow_lazy = true);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index e5b347c..628efd9 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -401,8 +401,8 @@
reg_info->byte_size, // dst length
m_reg_data.GetByteOrder())) // dst byte order
{
- Mutex::Locker locker;
- if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for write register."))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (gdb_comm.GetSequenceMutex(lock, "Didn't get sequence mutex for write register."))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
@@ -570,8 +570,8 @@
const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false;
- Mutex::Locker locker;
- if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for read all registers."))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (gdb_comm.GetSequenceMutex(lock, "Didn't get sequence mutex for read all registers."))
{
SyncThreadState(process);
@@ -679,8 +679,8 @@
const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false;
StringExtractorGDBRemote response;
- Mutex::Locker locker;
- if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for write all registers."))
+ std::unique_lock<std::recursive_mutex> lock;
+ if (gdb_comm.GetSequenceMutex(lock, "Didn't get sequence mutex for write all registers."))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 1cea670..a14c825 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1590,9 +1590,6 @@
else
{
EventSP event_sp;
- TimeValue timeout;
- timeout = TimeValue::Now();
- timeout.OffsetWithSeconds (5);
if (!m_async_thread.IsJoinable())
{
error.SetErrorString ("Trying to resume but the async thread is dead.");
@@ -1603,7 +1600,7 @@
m_async_broadcaster.BroadcastEvent (eBroadcastBitAsyncContinue, new EventDataBytes (continue_packet.GetData(), continue_packet.GetSize()));
- if (listener_sp->WaitForEvent (&timeout, event_sp) == false)
+ if (listener_sp->WaitForEvent(std::chrono::seconds(5), event_sp) == false)
{
error.SetErrorString("Resume timed out.");
if (log)
@@ -2717,7 +2714,7 @@
Error error;
bool timed_out = false;
- Mutex::Locker locker;
+ std::unique_lock<std::recursive_mutex> lock;
if (m_public_state.GetValue() == eStateAttaching)
{
@@ -2727,7 +2724,7 @@
}
else
{
- if (!m_gdb_comm.SendInterrupt (locker, 2, timed_out))
+ if (!m_gdb_comm.SendInterrupt(lock, 2, timed_out))
{
if (timed_out)
error.SetErrorString("timed out sending interrupt packet");
@@ -3860,7 +3857,7 @@
{
if (log)
log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") listener.WaitForEvent (NULL, event_sp)...", __FUNCTION__, arg, process->GetID());
- if (process->m_async_listener_sp->WaitForEvent (NULL, event_sp))
+ if (process->m_async_listener_sp->WaitForEvent(std::chrono::microseconds(0), event_sp))
{
const uint32_t event_type = event_sp->GetType();
if (event_sp->BroadcasterIs (&process->m_async_broadcaster))