Skip to content

Commit 962ef99

Browse files
committed
[lldb] Protect RNBRemote from a data race
Summary: Thread sanitizer reports the following data race: ``` Write of size 8 at 0x000103303e70 by thread T1 (mutexes: write M0): #0 RNBRemote::CommDataReceived(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) RNBRemote.cpp:1075 (debugserver:arm64+0x100038db8) (BuildId: f130b34f693c4f3eba96139104af2b7132000000200000000100000000000e00) #1 RNBRemote::ThreadFunctionReadRemoteData(void*) RNBRemote.cpp:1180 (debugserver:arm64+0x1000391dc) (BuildId: f130b34f693c4f3eba96139104af2b7132000000200000000100000000000e00) Previous read of size 8 at 0x000103303e70 by main thread: #0 RNBRemote::GetPacketPayload(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) RNBRemote.cpp:797 (debugserver:arm64+0x100037c5c) (BuildId: f130b34f693c4f3eba96139104af2b7132000000200000000100000000000e00) #1 RNBRemote::GetPacket(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&, RNBRemote::Packet&, bool) RNBRemote.cpp:907 (debugserver:arm64+0x1000378cc) (BuildId: f130b34f693c4f3eba96139104af2b7132000000200000000100000000000e00) ``` RNBRemote already has a mutex, extend its usage to protect the read of m_rx_packets. Reviewers: jdevlieghere, bulbazord, jingham Subscribers:
1 parent 7c9d7b7 commit 962ef99

File tree

1 file changed

+21
-19
lines changed

1 file changed

+21
-19
lines changed

lldb/tools/debugserver/source/RNBRemote.cpp

+21-19
Original file line numberDiff line numberDiff line change
@@ -777,26 +777,28 @@ rnb_err_t RNBRemote::GetPacketPayload(std::string &return_packet) {
777777
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s called",
778778
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__);
779779

780-
PThreadMutex::Locker locker(m_mutex);
781-
if (m_rx_packets.empty()) {
782-
// Only reset the remote command available event if we have no more packets
783-
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
784-
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s error: no packets
785-
// available...", (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
786-
// __FUNCTION__);
787-
return rnb_err;
788-
}
780+
{
781+
PThreadMutex::Locker locker(m_mutex);
782+
if (m_rx_packets.empty()) {
783+
// Only reset the remote command available event if we have no more
784+
// packets
785+
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
786+
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s error: no packets
787+
// available...", (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
788+
// __FUNCTION__);
789+
return rnb_err;
790+
}
789791

790-
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s has %u queued packets",
791-
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
792-
// m_rx_packets.size());
793-
return_packet.swap(m_rx_packets.front());
794-
m_rx_packets.pop_front();
795-
locker.Reset(); // Release our lock on the mutex
796-
797-
if (m_rx_packets.empty()) {
798-
// Reset the remote command available event if we have no more packets
799-
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
792+
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s has %u queued packets",
793+
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
794+
// m_rx_packets.size());
795+
return_packet.swap(m_rx_packets.front());
796+
m_rx_packets.pop_front();
797+
798+
if (m_rx_packets.empty()) {
799+
// Reset the remote command available event if we have no more packets
800+
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
801+
}
800802
}
801803

802804
// DNBLogThreadedIf (LOG_RNB_MEDIUM, "%8u RNBRemote::%s: '%s'",

0 commit comments

Comments
 (0)