Skip to content

Commit

Permalink
[lldb] Fix data race in ThreadedCommunication
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JDevlieghere committed Aug 9, 2023
1 parent bdd17b8 commit 1a8d9a7
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
16 changes: 14 additions & 2 deletions lldb/include/lldb/Core/ThreadedCommunication.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions lldb/source/Core/ThreadedCommunication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <chrono>
#include <cstring>
#include <memory>
#include <shared_mutex>

#include <cerrno>
#include <cinttypes>
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;

Expand All @@ -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;

Expand Down

0 comments on commit 1a8d9a7

Please sign in to comment.