[Lldb-commits] [lldb] r262040 - The IOHandlerProcessSTDIO is the _only_ IOHandler that gets pushed and popped from functions that are run due to something that is NOT input from the user. All other IOHandler objects result from input from the user. An issue rose up where if a command caused the process to resume and stop and process state changed, where state changed Event objects were broadcast, it would cause the IOHandlerProcessSTDIO to have its IOHandlerProcessSTDIO::Cancel() function called. This used to always wr...

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Fri Feb 26 11:36:44 2016
New Revision: 262040

URL: http://llvm.org/viewvc/llvm-project?rev=262040&view=rev
Log:
The IOHandlerProcessSTDIO is the _only_ IOHandler that gets pushed and popped from functions that are run due to something that is NOT input from the user. All other IOHandler objects result from input from the user. An issue rose up where if a command caused the process to resume and stop and process state changed, where state changed Event objects were broadcast, it would cause the IOHandlerProcessSTDIO to have its IOHandlerProcessSTDIO::Cancel() function called. This used to always write a byte to the control pipe (IOHandlerProcessSTDIO::m_pipe) even if the IOHandlerProcessSTDIO::Run() was never called. What would happen is:

(lldb) command_that_steps_process_thousands_of_times

As the "command_that_steps_process_thousands_of_times" could be a python command that resumed the process thousands of times and in doing so the IOHandlerProcessSTDIO would get pushed when the process resumed, and popped when it stoppped, causing the call to IOHandlerProcessSTDIO::Cancel(). Since the IOHandler thread is currently in IOHandlerEditline::Run() for the command interpreter handling the "command_that_steps_process_thousands_of_times" command, IOHandlerProcessSTDIO::Run() would never get called, even though the IOHandlerProcessSTDIO is on the top of the stack. This caused the command pipe to keep getting 1 bytes written each time the IOHandlerProcessSTDIO::Cancel() was called and eventually we will deadlock since the write buffer is full.

The fix here is to make sure we are in IOHandlerProcessSTDIO::Run() before we write anything to the command pipe, and just call SetIsDone(true) if we are not.

<rdar://problem/22361364>


Modified:
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=262040&r1=262039&r2=262040&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri Feb 26 11:36:44 2016
@@ -4949,6 +4949,7 @@ public:
 // FD_ZERO, FD_SET are not supported on windows
 #ifndef _WIN32
         const int pipe_read_fd = m_pipe.GetReadFileDescriptor();
+        m_is_running = true;
         while (!GetIsDone())
         {
             fd_set read_fdset;
@@ -4957,6 +4958,7 @@ public:
             FD_SET (pipe_read_fd, &read_fdset);
             const int nfds = std::max<int>(read_fd, pipe_read_fd) + 1;
             int num_set_fds = select (nfds, &read_fdset, NULL, NULL, NULL);
+
             if (num_set_fds < 0)
             {
                 const int select_errno = errno;
@@ -5000,6 +5002,7 @@ public:
                 }
             }
         }
+        m_is_running = false;
 #endif
         terminal_state.Restore();
     }
@@ -5007,9 +5010,24 @@ public:
     void
     Cancel () override
     {
-        char ch = 'q';  // Send 'q' for quit
-        size_t bytes_written = 0;
-        m_pipe.Write(&ch, 1, bytes_written);
+        SetIsDone(true);
+        // Only write to our pipe to cancel if we are in IOHandlerProcessSTDIO::Run().
+        // We can end up with a python command that is being run from the command
+        // interpreter:
+        //
+        // (lldb) step_process_thousands_of_times
+        //
+        // In this case the command interpreter will be in the middle of handling
+        // the command and if the process pushes and pops the IOHandler thousands
+        // of times, we can end up writing to m_pipe without ever consuming the
+        // bytes from the pipe in IOHandlerProcessSTDIO::Run() and end up
+        // deadlocking when the pipe gets fed up and blocks until data is consumed.
+        if (m_is_running)
+        {
+            char ch = 'q';  // Send 'q' for quit
+            size_t bytes_written = 0;
+            m_pipe.Write(&ch, 1, bytes_written);
+        }
     }
 
     bool
@@ -5056,6 +5074,7 @@ protected:
     File m_read_file;   // Read from this file (usually actual STDIN for LLDB
     File m_write_file;  // Write to this file (usually the master pty for getting io to debuggee)
     Pipe m_pipe;
+    std::atomic<bool> m_is_running;
 };
 
 void