Hi,
I have attached a patch which addresses 3 related race conditions that
cause the command line (lldb) prompt to get displayed inappropriately and
make it appear it is not working correctly. This issue can be seen on
linux and FreeBSD. I can also artificailly induce the problem on OSX.
The issue happens when the command handler (in the main thread) issues a
command such as run, step or continue. After the command finishes
initiating its action, it returns up the call stack and goes back into the
main command loop waiting for user input. Simultaneously, as the inferior
process starts up, the MonitorChildProcess thread picks up the change and
posts to the PrivateEvent thread. HandePrivateEvent() then calls
PushProcessIOHandler() which will disable the command IO handler and give
the inferior control of the TTY. To observe this on OSX, put a
usleep(100);
immediately prior the PushProcessIOHandler() in HandlePrivateEvent.
My proposed solution is that after a 'run', 'step', or 'continue' command,
insert a synchronization point and wait until HandlePrivateEvent knows the
inferior process is running and has pushed the IO handler. One context
switch (<100us) is usually all the time it takes on my machine. As an
additional safety, I have a timeout (currently 1ms) so it will never hang
the main thread.
Any thoughts, or suggestions would be appreciated.
Regards,
Shawn.
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h (revision 213941)
+++ include/lldb/Target/Process.h (working copy)
@@ -2706,7 +2706,36 @@
bool wait_always = true,
Listener *hijack_listener = NULL);
+ //------------------------------------------------------------------
+ /// Used in conjuction with WaitForProcessRunning to synchronize the main
thread to
+ /// HandlePrivateEvent thread.
+ //------------------------------------------------------------------
+ void
+ ClearProcessRunningCounter()
+ {
+ m_process_running_counter = 0;
+ }
+
+ //------------------------------------------------------------------
+ /// Waits for the process state to be running within a given usec timeout.
+ ///
+ /// The main purpose of this is to implement an interlock waiting for
HandlePrivateEvent
+ /// to push an IOHandler.
+ ///
+ /// @param[in] timeout_usec
+ /// The maximum time length to wait for the process to transition to
the
+ /// eStateRunning state, specified in microseconds.
+ ///
+ /// @return
+ /// A running state if the process did transition into a running state
+ /// within the timeout interval; otherwise, the most recent state the
process was
+ /// in at the time of the timeout.
+ //------------------------------------------------------------------
lldb::StateType
+ WaitForProcessRunning (uint64_t timeout_usec);
+
+
+ lldb::StateType
WaitForStateChangedEvents (const TimeValue *timeout,
lldb::EventSP &event_sp,
Listener *hijack_listener); // Pass NULL to use
builtin listener
@@ -3091,6 +3120,7 @@
std::vector<PreResumeCallbackAndBaton> m_pre_resume_actions;
ProcessRunLock m_public_run_lock;
ProcessRunLock m_private_run_lock;
+ int m_process_running_counter; // used as
synchronization between HandlePrivateEvent pushing IOHandler and main thread
Predicate<bool> m_currently_handling_event; // This predicate
is set in HandlePrivateEvent while all its business is being done.
bool m_currently_handling_do_on_removals;
bool m_resume_requested; // If
m_currently_handling_event or m_currently_handling_do_on_removals are true,
Resume will only request a resume, using this flag to check.
Index: source/Commands/CommandObjectProcess.cpp
===================================================================
--- source/Commands/CommandObjectProcess.cpp (revision 213941)
+++ source/Commands/CommandObjectProcess.cpp (working copy)
@@ -766,8 +766,15 @@
process->GetThreadList().GetThreadAtIndex(idx)->SetResumeState (eStateRunning,
override_suspend);
}
}
-
+ process->ClearProcessRunningCounter();
+
Error error(process->Resume());
+
+ // There is a race condition where this thread will return up the
call stack to the main command
+ // handler and show an (lldb) prompt before HandlePrivateEvent
(from PrivateStateThread) has
+ // a chance to call PushProcessIOHandler().
+ process->WaitForProcessRunning(1000);
+
if (error.Success())
{
result.AppendMessageWithFormat ("Process %" PRIu64 "
resuming\n", process->GetID());
Index: source/Commands/CommandObjectThread.cpp
===================================================================
--- source/Commands/CommandObjectThread.cpp (revision 213941)
+++ source/Commands/CommandObjectThread.cpp (working copy)
@@ -623,9 +623,15 @@
}
process->GetThreadList().SetSelectedThreadByID (thread->GetID());
+ process->ClearProcessRunningCounter();
+
process->Resume ();
-
+ // There is a race condition where this thread will return up the
call stack to the main command handler
+ // and show an (lldb) prompt before HandlePrivateEvent (from
PrivateStateThread) has
+ // a chance to call PushProcessIOHandler().
+ process->WaitForProcessRunning(1000);
+
if (synchronous_execution)
{
StateType state = process->WaitForProcessToStop (NULL);
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp (revision 213941)
+++ source/Target/Process.cpp (working copy)
@@ -697,6 +697,7 @@
m_next_event_action_ap(),
m_public_run_lock (),
m_private_run_lock (),
+ m_process_running_counter (0),
m_currently_handling_event(false),
m_finalize_called(false),
m_clear_thread_plans_on_stop (false),
@@ -889,7 +890,32 @@
return state;
}
+lldb::StateType
+Process::WaitForProcessRunning (uint64_t timeout_usec)
+{
+ Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
+ TimeValue timeout (TimeValue::Now ());
+ timeout.OffsetWithMicroSeconds(timeout_usec);
+ // Make sure the loop only runs at least once.
+ do
+ {
+ // it usually only takes a context switch or two for the flag to get
set
+ usleep(0);
+
+ if(m_process_running_counter > 0)
+ {
+ if (log)
+ log->Printf ("Process::%s pid %" PRIu64 ": SUCCESS",
__FUNCTION__, GetID ());
+ return eStateRunning;
+ }
+ } while (TimeValue::Now () < timeout);
+
+ if (log)
+ log->Printf ("Process::%s pid %" PRIu64 " (timeout=%" PRIu64 "):
FAIL", __FUNCTION__, GetID (), timeout_usec);
+ return GetState();
+}
+
StateType
Process::WaitForProcessToStop (const TimeValue *timeout, lldb::EventSP
*event_sp_ptr, bool wait_always, Listener *hijack_listener)
{
@@ -3878,6 +3904,7 @@
// as this means the curses GUI is in use...
if (!GetTarget().GetDebugger().IsForwardingEvents())
PushProcessIOHandler ();
+ m_process_running_counter++;
}
else if (StateIsStoppedState(new_state, false))
{
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp (revision 213941)
+++ source/Target/Target.cpp (working copy)
@@ -2419,8 +2419,15 @@
if (!synchronous_execution)
m_process_sp->RestoreProcessEvents ();
+ m_process_sp->ClearProcessRunningCounter();
+
error = m_process_sp->PrivateResume();
-
+
+ // there is a race condition where this thread will return up
the call stack to the main command
+ // handler and show an (lldb) prompt before HandlePrivateEvent
(from PrivateStateThread) has
+ // a chance to call PushProcessIOHandler()
+ m_process_sp->WaitForProcessRunning(1000);
+
if (error.Success())
{
if (synchronous_execution)
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
