[lldb] Protect RNBRemote from a data race

[augusto2112]

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.

[JDevlieghere]

lldb/tools/debugserver/source/RNBRemote.cpp

780

This is an RAII object, right? Can we just block scope it? Right now it looks like we might not unlock if we return early on line 787.

[jasonmolenda]

lldb/tools/debugserver/source/RNBRemote.cpp

780

I agree with the suggested change. But yeah I think putting the part of this method that touches m_rx_packets in a block, and holding the lock in that block, would be best, make it a little clearer. As for the early return on 787, won't locker release the lock in its dtor?

lldb/tools/debugserver/source/RNBRemote.cpp

 /* Get a packet via gdb remote protocol.
  Strip off the prefix/suffix, verify the checksum to make sure
  a valid packet was received, send an ACK if they match.  */
 
 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;
   }
 
   // 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);
   }
+  locker.Reset(); // Release our lock on the mutex
 
   // DNBLogThreadedIf (LOG_RNB_MEDIUM, "%8u RNBRemote::%s: '%s'",
   // (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
   // return_packet.c_str());
 
   switch (return_packet[0]) {
   case '+':
   case '-':