[lldb] FIx data race in ThreadedCommunication

[JDevlieghere]

Summary

TSan reports the following race:

Write of size 8 at 0x000107707ee8 by main thread:
  #0 lldb_private::ThreadedCommunication::StartReadThread(...) ThreadedCommunication.cpp:175
  #1 lldb_private::Process::SetSTDIOFileDescriptor(...) Process.cpp:4533
  #2 lldb_private::Platform::DebugProcess(...) Platform.cpp:1121
  #3 lldb_private::PlatformDarwin::DebugProcess(...) PlatformDarwin.cpp:711
  #4 lldb_private::Target::Launch(...) Target.cpp:3235
  #5 CommandObjectProcessLaunch::DoExecute(...) CommandObjectProcess.cpp:256
  #6 lldb_private::CommandObjectParsed::Execute(...) CommandObject.cpp:751
  #7 lldb_private::CommandInterpreter::HandleCommand(...) CommandInterpreter.cpp:2054

Previous read of size 8 at 0x000107707ee8 by thread T5:
  #0 lldb_private::HostThread::IsJoinable(...) const HostThread.cpp:30
  #1 lldb_private::ThreadedCommunication::StopReadThread(...) ThreadedCommunication.cpp:192
  #2 lldb_private::Process::ShouldBroadcastEvent(...) Process.cpp:3420
  #3 lldb_private::Process::HandlePrivateEvent(...) Process.cpp:3728
  #4 lldb_private::Process::RunPrivateStateThread(...) Process.cpp:3914
  #5 std::__1::__function::__func<lldb_private::Process::StartPrivateStateThread(...) function.h:356
  #6 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(...) HostNativeThreadBase.cpp:62
  #7 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(...) HostThreadMacOSX.mm:18

The problem is the lack of synchronization between starting and stopping the read thread. This patch fixes that by protecting those operations with a mutex.

[bulbazord]

Looks straightforward and makes sense. Thanks!

lldb/include/lldb/Core/ThreadedCommunication.h

public:
 
   static ConstString &GetStaticBroadcasterClass();
 
   ConstString &GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-                            /// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic<bool> m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic<bool> m_read_thread_did_exit;
+
   std::string
       m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded
                                         /// access to the cached bytes.
   lldb::ConnectionStatus m_pass_status; ///< Connection status passthrough
                                         /// from read thread.
   Status m_pass_error;                  ///< Error passthrough from read thread.
   std::mutex m_synchronize_mutex;

lldb/source/Core/ThreadedCommunication.cpp

 #include "lldb/Utility/Status.h"
 
 #include "llvm/Support/Compiler.h"
 
 #include <algorithm>
 #include <chrono>
 #include <cstring>
 #include <memory>
+#include <shared_mutex>
 
 #include <cerrno>
 #include <cinttypes>
 #include <cstdio>
 
 using namespace lldb;
 using namespace lldb_private;
 
size_t ThreadedCommunication::Read(void *dst, size_t dst_len,
   }
 
   // We aren't using a read thread, just read the data synchronously in this
   // thread.
   return Communication::Read(dst, dst_len, timeout, status, error_ptr);
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard<std::mutex> lock(m_read_thread_mutex);
+
   if (error_ptr)
     error_ptr->Clear();
 
   if (m_read_thread.IsJoinable())
     return true;
 
   LLDB_LOG(GetLog(LLDBLog::Communication),
            "{0} ThreadedCommunication::StartReadThread ()", this);
bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
 
   if (!m_read_thread.IsJoinable())
     m_read_thread_enabled = false;
 
   return m_read_thread_enabled;
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard<std::mutex> lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
     return true;
 
   LLDB_LOG(GetLog(LLDBLog::Communication),
            "{0} ThreadedCommunication::StopReadThread ()", this);
 
   m_read_thread_enabled = false;
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard<std::mutex> lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
     return true;
 
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 size_t ThreadedCommunication::GetCachedBytes(void *dst, size_t dst_len) {