JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, jingham, labath. Herald added a subscriber: kristof.beyls. Herald added a project: All. JDevlieghere requested review of this revision.
Fixes a data race between the main thread and the process handling thread: WARNING: ThreadSanitizer: data race (pid=13380) Write of size 1 at 0x0001067646e8 by thread T8 (mutexes: write M2862): #0 lldb_private::IOHandler::SetIsDone(bool) IOHandler.h:90 (liblldb.15.0.0git.dylib:arm64+0x971d84) #1 IOHandlerProcessSTDIO::Cancel() Process.cpp:4376 (liblldb.15.0.0git.dylib:arm64+0x5de0ec) #2 lldb_private::Debugger::PopIOHandler(std::__1::shared_ptr<lldb_private::IOHandler> const&) Debugger.cpp:1156 (liblldb.15.0.0git.dylib:arm64+0x3cb264) #3 lldb_private::Debugger::RemoveIOHandler(std::__1::shared_ptr<lldb_private::IOHandler> const&) Debugger.cpp:1063 (liblldb.15.0.0git.dylib:arm64+0x3cbce8) #4 lldb_private::Process::PopProcessIOHandler() Process.cpp:4480 (liblldb.15.0.0git.dylib:arm64+0x5c57f8) #5 lldb_private::Debugger::HandleProcessEvent(std::__1::shared_ptr<lldb_private::Event> const&) Debugger.cpp:1549 (liblldb.15.0.0git.dylib:arm64+0x3cea78) #6 lldb_private::Debugger::DefaultEventHandler() Debugger.cpp:1622 (liblldb.15.0.0git.dylib:arm64+0x3cf27c) #7 std::__1::__function::__func<lldb_private::Debugger::StartEventHandlerThread()::$_2, std::__1::allocator<lldb_private::Debugger::StartEventHandlerThread()::$_2>, void* ()>::operator()() function.h:352 (liblldb.15.0.0git.dylib:arm64+0x3d1b94) #8 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) HostNativeThreadBase.cpp:62 (liblldb.15.0.0git.dylib:arm64+0x4c7168) #9 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) HostThreadMacOSX.mm:18 (liblldb.15.0.0git.dylib:arm64+0x29ef544) Previous write of size 1 at 0x0001067646e8 by main thread: #0 lldb_private::IOHandler::SetIsDone(bool) IOHandler.h:90 (liblldb.15.0.0git.dylib:arm64+0x971d84) #1 IOHandlerProcessSTDIO::Run() Process.cpp:4323 (liblldb.15.0.0git.dylib:arm64+0x5ddc2c) #2 lldb_private::Debugger::RunIOHandlers() Debugger.cpp:982 (liblldb.15.0.0git.dylib:arm64+0x3cb448) #3 lldb_private::CommandInterpreter::RunCommandInterpreter(lldb_private::CommandInterpreterRunOptions&) CommandInterpreter.cpp:3298 (liblldb.15.0.0git.dylib:arm64+0x506434) #4 lldb::SBDebugger::RunCommandInterpreter(bool, bool) SBDebugger.cpp:1166 (liblldb.15.0.0git.dylib:arm64+0x535c0) #5 Driver::MainLoop() Driver.cpp:634 (lldb:arm64+0x100006294) #6 main Driver.cpp:853 (lldb:arm64+0x100007344) https://reviews.llvm.org/D120762 Files: lldb/include/lldb/Core/IOHandler.h lldb/source/Core/IOHandler.cpp lldb/source/Target/Process.cpp
Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -4398,7 +4398,7 @@ // interrupt the IOHandlerProcessSTDIO::Run() and we can look at the byte // that was written to the pipe and then call // m_process->SendAsyncInterrupt() from a much safer location in code. - if (m_active) { + if (IsActive()) { char ch = 'i'; // Send 'i' for interrupt size_t bytes_written = 0; Status result = m_pipe.Write(&ch, 1, bytes_written); Index: lldb/source/Core/IOHandler.cpp =================================================================== --- lldb/source/Core/IOHandler.cpp +++ lldb/source/Core/IOHandler.cpp @@ -79,6 +79,31 @@ IOHandler::~IOHandler() = default; +bool IOHandler::IsActive() { + std::lock_guard<std::mutex> guard(m_control_mutex); + return m_active && !m_done; +} + +void IOHandler::SetIsDone(bool b) { + std::lock_guard<std::mutex> guard(m_control_mutex); + m_done = b; +} + +bool IOHandler::GetIsDone() { + std::lock_guard<std::mutex> guard(m_control_mutex); + return m_done; +} + +void IOHandler::Activate() { + std::lock_guard<std::mutex> guard(m_control_mutex); + m_active = true; +} + +void IOHandler::Deactivate() { + std::lock_guard<std::mutex> guard(m_control_mutex); + m_active = false; +} + int IOHandler::GetInputFD() { return (m_input_sp ? m_input_sp->GetDescriptor() : -1); } @@ -558,7 +583,7 @@ StringList lines; if (GetLines(lines, interrupted)) { if (interrupted) { - m_done = m_interrupt_exits; + SetIsDone(m_interrupt_exits); m_delegate.IOHandlerInputInterrupted(*this, line); } else { @@ -566,7 +591,7 @@ m_delegate.IOHandlerInputComplete(*this, line); } } else { - m_done = true; + SetIsDone(true); } } else { if (GetLine(line, interrupted)) { @@ -575,7 +600,7 @@ else m_delegate.IOHandlerInputComplete(*this, line); } else { - m_done = true; + SetIsDone(true); } } } Index: lldb/include/lldb/Core/IOHandler.h =================================================================== --- lldb/include/lldb/Core/IOHandler.h +++ lldb/include/lldb/Core/IOHandler.h @@ -85,17 +85,17 @@ virtual void GotEOF() = 0; - virtual bool IsActive() { return m_active && !m_done; } + virtual bool IsActive(); - virtual void SetIsDone(bool b) { m_done = b; } + virtual void SetIsDone(bool b); - virtual bool GetIsDone() { return m_done; } + virtual bool GetIsDone(); Type GetType() const { return m_type; } - virtual void Activate() { m_active = true; } + virtual void Activate(); - virtual void Deactivate() { m_active = false; } + virtual void Deactivate(); virtual void TerminalSizeChanged() {} @@ -178,10 +178,12 @@ Flags m_flags; Type m_type; void *m_user_data; + +private: + std::mutex m_control_mutex; bool m_done; bool m_active; -private: IOHandler(const IOHandler &) = delete; const IOHandler &operator=(const IOHandler &) = delete; };
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits