Skip to content

Commit

Permalink
[lldb] Protect RNBRemote from a data race
Browse files Browse the repository at this point in the history
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:
  • Loading branch information
augusto2112 committed Sep 25, 2023
1 parent 7c9d7b7 commit 962ef99
Showing 1 changed file with 21 additions and 19 deletions.
40 changes: 21 additions & 19 deletions lldb/tools/debugserver/source/RNBRemote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -777,26 +777,28 @@ rnb_err_t RNBRemote::GetPacketPayload(std::string &return_packet) {
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s called",
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__);

PThreadMutex::Locker locker(m_mutex);
if (m_rx_packets.empty()) {
// Only reset the remote command available event if we have no more packets
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s error: no packets
// available...", (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
// __FUNCTION__);
return rnb_err;
}
{
PThreadMutex::Locker locker(m_mutex);
if (m_rx_packets.empty()) {
// Only reset the remote command available event if we have no more
// packets
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s error: no packets
// available...", (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
// __FUNCTION__);
return rnb_err;
}

// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s has %u queued packets",
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
// m_rx_packets.size());
return_packet.swap(m_rx_packets.front());
m_rx_packets.pop_front();
locker.Reset(); // Release our lock on the mutex

if (m_rx_packets.empty()) {
// Reset the remote command available event if we have no more packets
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
// DNBLogThreadedIf (LOG_RNB_MAX, "%8u RNBRemote::%s has %u queued packets",
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
// m_rx_packets.size());
return_packet.swap(m_rx_packets.front());
m_rx_packets.pop_front();

if (m_rx_packets.empty()) {
// Reset the remote command available event if we have no more packets
m_ctx.Events().ResetEvents(RNBContext::event_read_packet_available);
}
}

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

0 comments on commit 962ef99

Please sign in to comment.