[Lldb-commits] [lldb] 962ef99 - [lldb] Protect RNBRemote from a data race

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2023-09-25T11:17:42-07:00
New Revision: 962ef991da2836775191435fb732fc61ec07d5e6

URL: https://github.com/llvm/llvm-project/commit/962ef991da2836775191435fb732fc61ec07d5e6
DIFF: https://github.com/llvm/llvm-project/commit/962ef991da2836775191435fb732fc61ec07d5e6.diff

LOG: [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:

Added: 
    

Modified: 
    lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index ea039101bb541b0..4e9086b0411f2d7 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -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'",