[Lldb-commits] [lldb] r202525 - Fixed all overlapping prompt issues.
Greg Clayton
gclayton at apple.com
Fri Feb 28 10:22:24 PST 2014
Author: gclayton
Date: Fri Feb 28 12:22:24 2014
New Revision: 202525
URL: http://llvm.org/viewvc/llvm-project?rev=202525&view=rev
Log:
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>
Modified:
lldb/trunk/include/lldb/Core/Debugger.h
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Target/Process.cpp
Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=202525&r1=202524&r2=202525&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Fri Feb 28 12:22:24 2014
@@ -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 ();
Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=202525&r1=202524&r2=202525&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Fri Feb 28 12:22:24 2014
@@ -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);
Modified: lldb/trunk/source/Core/Debugger.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=202525&r1=202524&r2=202525&view=diff
==============================================================================
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Fri Feb 28 12:22:24 2014
@@ -629,8 +629,7 @@ Debugger::Debugger (lldb::LogOutputCallb
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 IOHandler
// 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 IOHandlerS
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 Even
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)
+ {
+ 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)
{
- StreamFileSP stream_sp (GetOutputFile());
- GetProcessSTDOUT (process_sp.get(), stream_sp.get());
- GetProcessSTDERR (process_sp.get(), NULL);
+
+ // 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 Even
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 Even
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 Even
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 Even
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 Even
{
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
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=202525&r1=202524&r2=202525&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri Feb 28 12:22:24 2014
@@ -1523,7 +1523,6 @@ Process::SetExitStatus (int status, cons
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 &ev
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 &ev
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 &ev
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 (v
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
More information about the lldb-commits
mailing list