[Lldb-commits] [PATCH]race condition calling PushProcessIOHandler
Shawn Best
sbest at blueshiftinc.com
Mon Jul 28 14:23:45 PDT 2014
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140728/ff269d5c/attachment.html>
-------------- next part --------------
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)
More information about the lldb-commits
mailing list