Author: Jonas Devlieghere Date: 2023-08-09T15:15:42-07:00 New Revision: 1a8d9a7657bba79099e6e2a6b0568db53d1e9a23
URL: https://github.com/llvm/llvm-project/commit/1a8d9a7657bba79099e6e2a6b0568db53d1e9a23 DIFF: https://github.com/llvm/llvm-project/commit/1a8d9a7657bba79099e6e2a6b0568db53d1e9a23.diff LOG: [lldb] Fix data race in ThreadedCommunication 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. Differential revision: https://reviews.llvm.org/D157361 Added: Modified: lldb/include/lldb/Core/ThreadedCommunication.h lldb/source/Core/ThreadedCommunication.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/ThreadedCommunication.h b/lldb/include/lldb/Core/ThreadedCommunication.h index 2e3afde3c05826..7ebb77beb77f3d 100644 --- a/lldb/include/lldb/Core/ThreadedCommunication.h +++ b/lldb/include/lldb/Core/ThreadedCommunication.h @@ -223,10 +223,22 @@ class ThreadedCommunication : public Communication, public Broadcaster { } 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 diff --git a/lldb/source/Core/ThreadedCommunication.cpp b/lldb/source/Core/ThreadedCommunication.cpp index 755a158a5359e9..7d8aae5d8ff689 100644 --- a/lldb/source/Core/ThreadedCommunication.cpp +++ b/lldb/source/Core/ThreadedCommunication.cpp @@ -23,6 +23,7 @@ #include <chrono> #include <cstring> #include <memory> +#include <shared_mutex> #include <cerrno> #include <cinttypes> @@ -155,6 +156,8 @@ size_t ThreadedCommunication::Read(void *dst, size_t dst_len, } bool ThreadedCommunication::StartReadThread(Status *error_ptr) { + std::lock_guard<std::mutex> lock(m_read_thread_mutex); + if (error_ptr) error_ptr->Clear(); @@ -189,6 +192,8 @@ bool ThreadedCommunication::StartReadThread(Status *error_ptr) { } bool ThreadedCommunication::StopReadThread(Status *error_ptr) { + std::lock_guard<std::mutex> lock(m_read_thread_mutex); + if (!m_read_thread.IsJoinable()) return true; @@ -199,13 +204,13 @@ bool ThreadedCommunication::StopReadThread(Status *error_ptr) { 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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
