Fixed all overlapping prompt issues.

I carefully reviewed exactly how the IOHandlers interact and found places where we weren't properly controlling things. There should be no overlapping prompts and all output should now come out in a controlled fashion.

<rdar://problem/16111293>

llvm-svn: 202525

4 files changed, 143 insertions, 100 deletions

diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index e80ec851679..96a7cc3f93a 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -352,6 +352,13 @@ public:
 
     void
     CancelForwardEvents (const lldb::ListenerSP &listener_sp);
+    
+    bool
+    IsHandlingEvents () const
+    {
+        return IS_VALID_LLDB_HOST_THREAD(m_event_handler_thread);
+    }
+
 protected:
 
     friend class CommandInterpreter;
@@ -420,7 +427,6 @@ protected:
     lldb::thread_t m_event_handler_thread;
     lldb::thread_t m_io_handler_thread;
     lldb::ListenerSP m_forward_listener_sp;
-    bool m_event_handler_thread_alive;
     void
     InstanceInitialize ();
     
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index b74347d37e6..f1281e30c4c 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1418,7 +1418,8 @@ class Process :
     public ExecutionContextScope,
     public PluginInterface
 {
-    friend class ClangFunction; // For WaitForStateChangeEventsPrivate
+    friend class ClangFunction;     // For WaitForStateChangeEventsPrivate
+    friend class Debugger;          // For PopProcessIOHandler
     friend class ProcessEventData;
     friend class StopInfo;
     friend class Target;
@@ -3582,12 +3583,6 @@ public:
     void
     SetSTDIOFileDescriptor (int file_descriptor);
 
-    void
-    WatchForSTDIN (IOHandler &io_handler);
-    
-    void
-    CancelWatchForSTDIN (bool exited);
-    
     //------------------------------------------------------------------
     // Add a permanent region of memory that should never be read or 
     // written to. This can be used to ensure that memory reads or writes
@@ -3874,15 +3869,12 @@ protected:
     static void
     STDIOReadThreadBytesReceived (void *baton, const void *src, size_t src_len);
     
-    void
+    bool
     PushProcessIOHandler ();
     
-    void 
+    bool
     PopProcessIOHandler ();
     
-    void
-    ResetProcessIOHandler ();
-        
     Error
     HaltForDestroyOrDetach(lldb::EventSP &exit_event_sp);
     
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 34e0e329f09..aaacd6e26ca 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -629,8 +629,7 @@ Debugger::Debugger (lldb::LogOutputCallback log_callback, void *baton) :
     m_instance_name (),
     m_loaded_plugins (),
     m_event_handler_thread (LLDB_INVALID_HOST_THREAD),
-    m_io_handler_thread (LLDB_INVALID_HOST_THREAD),
-    m_event_handler_thread_alive(false)
+    m_io_handler_thread (LLDB_INVALID_HOST_THREAD)
 {
     char instance_cstr[256];
     snprintf(instance_cstr, sizeof(instance_cstr), "debugger_%d", (int)GetID());
@@ -960,13 +959,17 @@ Debugger::PushIOHandler (const IOHandlerSP& reader_sp)
     // Got the current top input reader...
     IOHandlerSP top_reader_sp (m_input_reader_stack.Top());
     
-    // Push our new input reader
-    m_input_reader_stack.Push (reader_sp);
+    // Don't push the same IO handler twice...
+    if (reader_sp.get() != top_reader_sp.get())
+    {
+        // Push our new input reader
+        m_input_reader_stack.Push (reader_sp);
 
-    // Interrupt the top input reader to it will exit its Run() function
-    // and let this new input reader take over
-    if (top_reader_sp)
-        top_reader_sp->Deactivate();
+        // Interrupt the top input reader to it will exit its Run() function
+        // and let this new input reader take over
+        if (top_reader_sp)
+            top_reader_sp->Deactivate();
+    }
 }
 
 bool
@@ -985,6 +988,7 @@ Debugger::PopIOHandler (const IOHandlerSP& pop_reader_sp)
         if (!pop_reader_sp || pop_reader_sp.get() == reader_sp.get())
         {
             reader_sp->Deactivate();
+            reader_sp->Cancel();
             m_input_reader_stack.Pop ();
             
             reader_sp = m_input_reader_stack.Top();
@@ -2769,36 +2773,30 @@ Debugger::HandleProcessEvent (const EventSP &event_sp)
     const uint32_t event_type = event_sp->GetType();
     ProcessSP process_sp = Process::ProcessEventData::GetProcessFromEvent(event_sp.get());
     
+    StreamString output_stream;
+    StreamString error_stream;
     const bool gui_enabled = IsForwardingEvents();
-    bool top_io_handler_hid = false;
-    if (gui_enabled == false)
-        top_io_handler_hid = HideTopIOHandler();
 
-    assert (process_sp);
-    
-    if (event_type & Process::eBroadcastBitSTDOUT)
-    {
-        // The process has stdout available, get it and write it out to the
-        // appropriate place.
-        if (top_io_handler_hid)
-            GetProcessSTDOUT (process_sp.get(), NULL);
-    }
-    else if (event_type & Process::eBroadcastBitSTDERR)
-    {
-        // The process has stderr available, get it and write it out to the
-        // appropriate place.
-        if (top_io_handler_hid)
-            GetProcessSTDERR (process_sp.get(), NULL);
-    }
-    else if (event_type & Process::eBroadcastBitStateChanged)
+    if (!gui_enabled)
     {
-        // Drain all stout and stderr so we don't see any output come after
-        // we print our prompts
-        if (top_io_handler_hid)
+        bool pop_process_io_handler = false;
+        assert (process_sp);
+    
+        if (event_type & Process::eBroadcastBitSTDOUT || event_type & Process::eBroadcastBitStateChanged)
         {
-            StreamFileSP stream_sp (GetOutputFile());
-            GetProcessSTDOUT (process_sp.get(), stream_sp.get());
-            GetProcessSTDERR (process_sp.get(), NULL);
+            GetProcessSTDOUT (process_sp.get(), &output_stream);
+        }
+        
+        if (event_type & Process::eBroadcastBitSTDERR || event_type & Process::eBroadcastBitStateChanged)
+        {
+            GetProcessSTDERR (process_sp.get(), &error_stream);
+        }
+    
+        if (event_type & Process::eBroadcastBitStateChanged)
+        {
+
+            // Drain all stout and stderr so we don't see any output come after
+            // we print our prompts
             // Something changed in the process;  get the event and report the process's current status and location to
             // the user.
             StateType event_state = Process::ProcessEventData::GetStateFromEvent (event_sp.get());
@@ -2815,9 +2813,12 @@ Debugger::HandleProcessEvent (const EventSP &event_sp)
                 case eStateStepping:
                 case eStateDetached:
                     {
-                        stream_sp->Printf("Process %" PRIu64 " %s\n",
-                                          process_sp->GetID(),
-                                          StateAsCString (event_state));
+                        output_stream.Printf("Process %" PRIu64 " %s\n",
+                                             process_sp->GetID(),
+                                             StateAsCString (event_state));
+                        
+                        if (event_state == eStateDetached)
+                            pop_process_io_handler = true;
                     }
                     break;
                     
@@ -2826,7 +2827,8 @@ Debugger::HandleProcessEvent (const EventSP &event_sp)
                     break;
                     
                 case eStateExited:
-                    process_sp->GetStatus(*stream_sp);
+                    process_sp->GetStatus(output_stream);
+                    pop_process_io_handler = true;
                     break;
                     
                 case eStateStopped:
@@ -2842,20 +2844,20 @@ Debugger::HandleProcessEvent (const EventSP &event_sp)
                             if (num_reasons == 1)
                             {
                                 const char *reason = Process::ProcessEventData::GetRestartedReasonAtIndex (event_sp.get(), 0);
-                                stream_sp->Printf("Process %" PRIu64 " stopped and restarted: %s\n",
-                                                  process_sp->GetID(),
-                                                  reason ? reason : "<UNKNOWN REASON>");
+                                output_stream.Printf("Process %" PRIu64 " stopped and restarted: %s\n",
+                                                     process_sp->GetID(),
+                                                     reason ? reason : "<UNKNOWN REASON>");
                             }
                             else
                             {
-                                stream_sp->Printf("Process %" PRIu64 " stopped and restarted, reasons:\n",
-                                                   process_sp->GetID());
+                                output_stream.Printf("Process %" PRIu64 " stopped and restarted, reasons:\n",
+                                                     process_sp->GetID());
                                 
 
                                 for (size_t i = 0; i < num_reasons; i++)
                                 {
                                     const char *reason = Process::ProcessEventData::GetRestartedReasonAtIndex (event_sp.get(), i);
-                                    stream_sp->Printf("\t%s\n", reason ? reason : "<UNKNOWN REASON>");
+                                    output_stream.Printf("\t%s\n", reason ? reason : "<UNKNOWN REASON>");
                                 }
                             }
                         }
@@ -2929,8 +2931,8 @@ Debugger::HandleProcessEvent (const EventSP &event_sp)
                             const uint32_t start_frame = 0;
                             const uint32_t num_frames = 1;
                             const uint32_t num_frames_with_source = 1;
-                            process_sp->GetStatus(*stream_sp);
-                            process_sp->GetThreadStatus (*stream_sp,
+                            process_sp->GetStatus(output_stream);
+                            process_sp->GetThreadStatus (output_stream,
                                                          only_threads_with_stop_reason,
                                                          start_frame,
                                                          num_frames,
@@ -2940,20 +2942,46 @@ Debugger::HandleProcessEvent (const EventSP &event_sp)
                         {
                             uint32_t target_idx = GetTargetList().GetIndexOfTarget(process_sp->GetTarget().shared_from_this());
                             if (target_idx != UINT32_MAX)
-                                stream_sp->Printf ("Target %d: (", target_idx);
+                                output_stream.Printf ("Target %d: (", target_idx);
                             else
-                                stream_sp->Printf ("Target <unknown index>: (");
-                            process_sp->GetTarget().Dump (stream_sp.get(), eDescriptionLevelBrief);
-                            stream_sp->Printf (") stopped.\n");
+                                output_stream.Printf ("Target <unknown index>: (");
+                            process_sp->GetTarget().Dump (&output_stream, eDescriptionLevelBrief);
+                            output_stream.Printf (") stopped.\n");
                         }
+                        
+                        // Pop the process IO handler
+                        pop_process_io_handler = true;
                     }
                     break;
             }
         }
-    }
     
-    if (top_io_handler_hid)
-        RefreshTopIOHandler();
+        if (output_stream.GetSize() || error_stream.GetSize())
+        {
+            StreamFileSP error_stream_sp (GetOutputFile());
+            bool top_io_handler_hid = HideTopIOHandler();
+
+            if (output_stream.GetSize())
+            {
+                StreamFileSP output_stream_sp (GetOutputFile());
+                if (output_stream_sp)
+                    output_stream_sp->Write (output_stream.GetData(), output_stream.GetSize());
+            }
+
+            if (error_stream.GetSize())
+            {
+                StreamFileSP error_stream_sp (GetErrorFile());
+                if (error_stream_sp)
+                    error_stream_sp->Write (error_stream.GetData(), error_stream.GetSize());
+            }
+
+            if (top_io_handler_hid)
+                RefreshTopIOHandler();
+        }
+
+        if (pop_process_io_handler)
+            process_sp->PopProcessIOHandler();
+    }
 }
 
 void
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index b7879028514..c7bd0438b08 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1523,7 +1523,6 @@ Process::SetExitStatus (int status, const char *cstr)
     DidExit ();
 
     SetPrivateState (eStateExited);
-    CancelWatchForSTDIN (true);
     return true;
 }
 
@@ -3738,9 +3737,14 @@ Process::Destroy ()
         }
         m_stdio_communication.StopReadThread();
         m_stdio_communication.Disconnect();
+
         if (m_process_input_reader)
+        {
+            m_process_input_reader->SetIsDone(true);
+            m_process_input_reader->Cancel();
             m_process_input_reader.reset();
-        
+        }
+
         // If we exited when we were waiting for a process to stop, then
         // forward the event here so we don't lose the event
         if (exit_event_sp)
@@ -4116,6 +4120,7 @@ Process::HandlePrivateEvent (EventSP &event_sp)
 
     if (should_broadcast)
     {
+        const bool is_hijacked = IsHijackedForEvent(eBroadcastBitStateChanged);
         if (log)
         {
             log->Printf ("Process::%s (pid = %" PRIu64 ") broadcasting new state %s (old state %s) to %s",
@@ -4123,7 +4128,7 @@ Process::HandlePrivateEvent (EventSP &event_sp)
                          GetID(), 
                          StateAsCString(new_state), 
                          StateAsCString (GetState ()),
-                         IsHijackedForEvent(eBroadcastBitStateChanged) ? "hijacked" : "public");
+                         is_hijacked ? "hijacked" : "public");
         }
         Process::ProcessEventData::SetUpdateStateOnRemoval(event_sp.get());
         if (StateIsRunningState (new_state))
@@ -4133,8 +4138,43 @@ Process::HandlePrivateEvent (EventSP &event_sp)
             if (!GetTarget().GetDebugger().IsForwardingEvents())
                 PushProcessIOHandler ();
         }
-        else if (!Process::ProcessEventData::GetRestartedFromEvent(event_sp.get()))
-            PopProcessIOHandler ();
+        else if (StateIsStoppedState(new_state, false))
+        {
+            if (!Process::ProcessEventData::GetRestartedFromEvent(event_sp.get()))
+            {
+                // If the lldb_private::Debugger is handling the events, we don't
+                // want to pop the process IOHandler here, we want to do it when
+                // we receive the stopped event so we can carefully control when
+                // the process IOHandler is popped because when we stop we want to
+                // display some text stating how and why we stopped, then maybe some
+                // process/thread/frame info, and then we want the "(lldb) " prompt
+                // to show up. If we pop the process IOHandler here, then we will
+                // cause the command interpreter to become the top IOHandler after
+                // the process pops off and it will update its prompt right away...
+                // See the Debugger.cpp file where it calls the function as
+                // "process_sp->PopProcessIOHandler()" to see where I am talking about.
+                // Otherwise we end up getting overlapping "(lldb) " prompts and
+                // garbled output.
+                //
+                // If we aren't handling the events in the debugger (which is indicated
+                // by "m_target.GetDebugger().IsHandlingEvents()" returning false) or we
+                // are hijacked, then we always pop the process IO handler manually.
+                // Hijacking happens when the internal process state thread is running
+                // thread plans, or when commands want to run in synchronous mode
+                // and they call "process->WaitForProcessToStop()". An example of something
+                // that will hijack the events is a simple expression:
+                //
+                //  (lldb) expr (int)puts("hello")
+                //
+                // This will cause the internal process state thread to resume and halt
+                // the process (and _it_ will hijack the eBroadcastBitStateChanged
+                // events) and we do need the IO handler to be pushed and popped
+                // correctly.
+                
+                if (is_hijacked || m_target.GetDebugger().IsHandlingEvents() == false)
+                    PopProcessIOHandler ();
+            }
+        }
 
         BroadcastEvent (event_sp);
     }
@@ -4680,13 +4720,6 @@ Process::STDIOReadThreadBytesReceived (void *baton, const void *src, size_t src_
     process->AppendSTDOUT (static_cast<const char *>(src), src_len);
 }
 
-void
-Process::ResetProcessIOHandler ()
-{   
-    m_process_input_reader.reset();
-}
-
-
 class IOHandlerProcessSTDIO :
     public IOHandler
 {
@@ -4878,22 +4911,6 @@ protected:
 };
 
 void
-Process::WatchForSTDIN (IOHandler &io_handler)
-{
-}
-
-void
-Process::CancelWatchForSTDIN (bool exited)
-{
-    if (m_process_input_reader)
-    {
-        if (exited)
-            m_process_input_reader->SetIsDone(true);
-        m_process_input_reader->Cancel();
-    }
-}
-
-void
 Process::SetSTDIOFileDescriptor (int fd)
 {
     // First set up the Read Thread for reading/handling process I/O
@@ -4916,7 +4933,7 @@ Process::SetSTDIOFileDescriptor (int fd)
     }
 }
 
-void
+bool
 Process::PushProcessIOHandler ()
 {
     IOHandlerSP io_handler_sp (m_process_input_reader);
@@ -4924,18 +4941,18 @@ Process::PushProcessIOHandler ()
     {
         io_handler_sp->SetIsDone(false);
         m_target.GetDebugger().PushIOHandler (io_handler_sp);
+        return true;
     }
+    return false;
 }
 
-void
+bool
 Process::PopProcessIOHandler ()
 {
     IOHandlerSP io_handler_sp (m_process_input_reader);
     if (io_handler_sp)
-    {
-        io_handler_sp->Cancel();
-        m_target.GetDebugger().PopIOHandler (io_handler_sp);
-    }
+        return m_target.GetDebugger().PopIOHandler (io_handler_sp);
+    return false;
 }
 
 // The process needs to know about installed plug-ins