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
  • [Lldb-commits] [PATCH] ... Jonas Devlieghere via Phabricator via lldb-commits

Reply via email to