labath created this revision.
labath added reviewers: clayborg, ovyalov.
labath added a subscriber: lldb-commits.

There was a race condition in Process class, where we would not wait for 
process stdout to
propagate fully before we would shut down the connection (repro case: slow down 
the stdio thread
by placing a sleep right at the end of the while loop in 
Communication::ReadThread). The Process
class already tried to solve this problem by synchronizing with the read thread 
in
Process::ShouldBroadcastEvent, but unfortunately the connection got closed 
before that in
Process::SetExitStatus. I solve this issue by delaying the connection shutdown 
until we get a
chance to process the event and synchronize. Alternatively, I could have moved 
the
synchronization point to an earlier point in SetExitStatus, but it seems safer 
to delay the
shutdown until other things get a chance to notice the process has exited.

http://reviews.llvm.org/D12558

Files:
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1479,12 +1479,7 @@
     else
         m_exit_string.clear();
 
-    // When we exit, we no longer need to the communication channel
-    m_stdio_communication.Disconnect();
-    m_stdio_communication.StopReadThread();
-    m_stdin_forward = false;
-
-    // And we don't need the input reader anymore as well
+    // When we exit, we don't need the input reader anymore
     if (m_process_input_reader)
     {
         m_process_input_reader->SetIsDone(true);
@@ -4159,6 +4154,10 @@
         case eStateExited:
         case eStateUnloaded:
             m_stdio_communication.SynchronizeWithReadThread();
+            m_stdio_communication.Disconnect();
+            m_stdio_communication.StopReadThread();
+            m_stdin_forward = false;
+
             // fall-through
         case eStateConnected:
         case eStateAttaching:


Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1479,12 +1479,7 @@
     else
         m_exit_string.clear();
 
-    // When we exit, we no longer need to the communication channel
-    m_stdio_communication.Disconnect();
-    m_stdio_communication.StopReadThread();
-    m_stdin_forward = false;
-
-    // And we don't need the input reader anymore as well
+    // When we exit, we don't need the input reader anymore
     if (m_process_input_reader)
     {
         m_process_input_reader->SetIsDone(true);
@@ -4159,6 +4154,10 @@
         case eStateExited:
         case eStateUnloaded:
             m_stdio_communication.SynchronizeWithReadThread();
+            m_stdio_communication.Disconnect();
+            m_stdio_communication.StopReadThread();
+            m_stdin_forward = false;
+
             // fall-through
         case eStateConnected:
         case eStateAttaching:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to