labath updated this revision to Diff 41215.
labath added a comment.
Fix typo
http://reviews.llvm.org/D14989
Files:
include/lldb/Target/Process.h
packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py
packages/Python/lldbsuite/test/functionalities/attach_resume/TestAttachResume.py
source/Target/Process.cpp
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -759,7 +759,6 @@
m_next_event_action_ap(),
m_public_run_lock (),
m_private_run_lock (),
- m_currently_handling_event(false),
m_stop_info_override_callback (NULL),
m_finalizing (false),
m_finalize_called (false),
@@ -992,7 +991,8 @@
EventSP *event_sp_ptr,
bool wait_always,
Listener *hijack_listener,
- Stream *stream)
+ Stream *stream,
+ bool use_run_lock)
{
// We can't just wait for a "stopped" event, because the stopped event may have restarted the target.
// We have to actually check each event, and in the case of a stopped event check the restarted flag
@@ -1019,7 +1019,7 @@
__FUNCTION__);
// We need to toggle the run lock as this won't get done in
// SetPublicState() if the process is hijacked.
- if (hijack_listener)
+ if (hijack_listener && use_run_lock)
m_public_run_lock.SetStopped();
return state;
}
@@ -1042,7 +1042,7 @@
case eStateUnloaded:
// We need to toggle the run lock as this won't get done in
// SetPublicState() if the process is hijacked.
- if (hijack_listener)
+ if (hijack_listener && use_run_lock)
m_public_run_lock.SetStopped();
return state;
case eStateStopped:
@@ -1052,7 +1052,7 @@
{
// We need to toggle the run lock as this won't get done in
// SetPublicState() if the process is hijacked.
- if (hijack_listener)
+ if (hijack_listener && use_run_lock)
m_public_run_lock.SetStopped();
return state;
}
@@ -1308,23 +1308,6 @@
RestoreBroadcaster();
}
-bool
-Process::HijackPrivateProcessEvents (Listener *listener)
-{
- if (listener != NULL)
- {
- return m_private_state_broadcaster.HijackBroadcaster(listener, eBroadcastBitStateChanged | eBroadcastBitInterrupt);
- }
- else
- return false;
-}
-
-void
-Process::RestorePrivateProcessEvents ()
-{
- m_private_state_broadcaster.RestoreBroadcaster();
-}
-
StateType
Process::WaitForStateChangedEvents (const TimeValue *timeout, EventSP &event_sp, Listener *hijack_listener)
{
@@ -3831,101 +3814,52 @@
}
Error
-Process::Halt (bool clear_thread_plans)
+Process::Halt (bool clear_thread_plans, bool use_run_lock)
{
+ if (! StateIsRunningState(m_public_state.GetValue()))
+ return Error("Process is not running.");
+
// Don't clear the m_clear_thread_plans_on_stop, only set it to true if
// in case it was already set and some thread plan logic calls halt on its
// own.
m_clear_thread_plans_on_stop |= clear_thread_plans;
- // First make sure we aren't in the middle of handling an event, or we might restart. This is pretty weak, since
- // we could just straightaway get another event. It just narrows the window...
- m_currently_handling_event.WaitForValueEqualTo(false);
-
-
// Pause our private state thread so we can ensure no one else eats
// the stop event out from under us.
Listener halt_listener ("lldb.process.halt_listener");
- HijackPrivateProcessEvents(&halt_listener);
+ HijackProcessEvents(&halt_listener);
EventSP event_sp;
- Error error (WillHalt());
- bool restored_process_events = false;
- if (error.Success())
+ SendAsyncInterrupt();
+
+ if (m_public_state.GetValue() == eStateAttaching)
{
-
- bool caused_stop = false;
-
- // Ask the process subclass to actually halt our process
- error = DoHalt(caused_stop);
- if (error.Success())
- {
- if (m_public_state.GetValue() == eStateAttaching)
- {
- // Don't hijack and eat the eStateExited as the code that was doing
- // the attach will be waiting for this event...
- RestorePrivateProcessEvents();
- restored_process_events = true;
- SetExitStatus(SIGKILL, "Cancelled async attach.");
- Destroy (false);
- }
- else
- {
- // If "caused_stop" is true, then DoHalt stopped the process. If
- // "caused_stop" is false, the process was already stopped.
- // If the DoHalt caused the process to stop, then we want to catch
- // this event and set the interrupted bool to true before we pass
- // this along so clients know that the process was interrupted by
- // a halt command.
- if (caused_stop)
- {
- // Wait for 1 second for the process to stop.
- TimeValue timeout_time;
- timeout_time = TimeValue::Now();
- timeout_time.OffsetWithSeconds(10);
- bool got_event = halt_listener.WaitForEvent (&timeout_time, event_sp);
- StateType state = ProcessEventData::GetStateFromEvent(event_sp.get());
-
- if (!got_event || state == eStateInvalid)
- {
- // We timeout out and didn't get a stop event...
- error.SetErrorStringWithFormat ("Halt timed out. State = %s", StateAsCString(GetState()));
- }
- else
- {
- if (StateIsStoppedState (state, false))
- {
- // We caused the process to interrupt itself, so mark this
- // as such in the stop event so clients can tell an interrupted
- // process from a natural stop
- ProcessEventData::SetInterruptedInEvent (event_sp.get(), true);
- }
- else
- {
- Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
- if (log)
- log->Printf("Process::Halt() failed to stop, state is: %s", StateAsCString(state));
- error.SetErrorString ("Did not get stopped event after halt.");
- }
- }
- }
- DidHalt();
- }
- }
+ // Don't hijack and eat the eStateExited as the code that was doing
+ // the attach will be waiting for this event...
+ RestoreProcessEvents();
+ SetExitStatus(SIGKILL, "Cancelled async attach.");
+ Destroy (false);
+ return Error();
}
- // Resume our private state thread before we post the event (if any)
- if (!restored_process_events)
- RestorePrivateProcessEvents();
- // Post any event we might have consumed. If all goes well, we will have
- // stopped the process, intercepted the event and set the interrupted
- // bool in the event. Post it to the private event queue and that will end up
- // correctly setting the state.
- if (event_sp)
- m_private_state_broadcaster.BroadcastEvent(event_sp);
+ // Wait for 10 second for the process to stop.
+ TimeValue timeout_time;
+ timeout_time = TimeValue::Now();
+ timeout_time.OffsetWithSeconds(10);
+ StateType state = WaitForProcessToStop(&timeout_time, &event_sp, true, &halt_listener,
+ nullptr, use_run_lock);
+ RestoreProcessEvents();
- return error;
+ if (state == eStateInvalid || ! event_sp)
+ {
+ // We timed out and didn't get a stop event...
+ return Error("Halt timed out. State = %s", StateAsCString(GetState()));
+ }
+
+ BroadcastEvent(event_sp);
+
+ return Error();
}
Error
@@ -4461,8 +4395,6 @@
Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
m_resume_requested = false;
- m_currently_handling_event.SetValue(true, eBroadcastNever);
-
const StateType new_state = Process::ProcessEventData::GetStateFromEvent(event_sp.get());
// First check to see if anybody wants a shot at this event:
@@ -4489,7 +4421,6 @@
{
// FIXME: should cons up an exited event, and discard this one.
SetExitStatus(0, m_next_event_action_ap->GetExitString());
- m_currently_handling_event.SetValue(false, eBroadcastAlways);
SetNextEventAction(NULL);
return;
}
@@ -4579,7 +4510,22 @@
StateAsCString (GetState ()));
}
}
- m_currently_handling_event.SetValue(false, eBroadcastAlways);
+}
+
+Error
+Process::HaltPrivate()
+{
+ EventSP event_sp;
+ Error error (WillHalt());
+ if (error.Fail())
+ return error;
+
+ // Ask the process subclass to actually halt our process
+ bool caused_stop;
+ error = DoHalt(caused_stop);
+
+ DidHalt();
+ return error;
}
thread_result_t
@@ -4602,6 +4548,7 @@
__FUNCTION__, static_cast<void*>(this), GetID());
bool exit_now = false;
+ bool interrupt_requested = false;
while (!exit_now)
{
EventSP event_sp;
@@ -4647,7 +4594,16 @@
log->Printf ("Process::%s (arg = %p, pid = %" PRIu64 ") woke up with an interrupt - Halting.",
__FUNCTION__, static_cast<void*>(this),
GetID());
- Halt();
+ Error error = HaltPrivate();
+ if (error.Fail() && log)
+ log->Printf ("Process::%s (arg = %p, pid = %" PRIu64 ") failed to halt the process: %s",
+ __FUNCTION__, static_cast<void*>(this),
+ GetID(), error.AsCString());
+ // Halt should generate a stopped event. Make a note of the fact that we were
+ // doing the interrupt, so we can set the interrupted flag after we receive the
+ // event. We deliberately set this to true even if HaltPrivate failed, so that we
+ // can interrupt on the next natural stop.
+ interrupt_requested = true;
}
continue;
}
@@ -4662,6 +4618,22 @@
m_clear_thread_plans_on_stop = false;
m_thread_list.DiscardThreadPlans();
}
+
+ if (interrupt_requested) {
+ if (StateIsStoppedState (internal_state, true))
+ {
+ // We requested the interrupt, so mark this as such in the stop event so
+ // clients can tell an interrupted process from a natural stop
+ ProcessEventData::SetInterruptedInEvent (event_sp.get(), true);
+ interrupt_requested = false;
+ }
+ else if (log)
+ {
+ log->Printf("Process::%s interrupt_requested, but a non-stopped state '%s' received.",
+ __FUNCTION__, StateAsCString(internal_state));
+ }
+ }
+
HandlePrivateEvent (event_sp);
}
@@ -5744,7 +5716,7 @@
{
// This is probably an overabundance of caution, I don't think I should ever get a stopped & restarted
// event here. But if I do, the best thing is to Halt and then get out of here.
- Halt();
+ Halt(/*clear_thread_plans = */false, /*use_run_lock = */false);
}
errors.Printf("Didn't get running event after initial resume, got %s instead.",
@@ -5838,7 +5810,7 @@
bool keep_going = false;
if (event_sp->GetType() == eBroadcastBitInterrupt)
{
- Halt();
+ Halt(/*clear_thread_plans = */false, /*use_run_lock = */false);
return_value = eExpressionInterrupted;
errors.Printf ("Execution halted by user interrupt.");
if (log)
@@ -6015,7 +5987,7 @@
{
if (log)
log->Printf ("Process::RunThreadPlan(): Running Halt.");
- halt_error = Halt();
+ halt_error = Halt(/*clear_thread_plans = */false, /*use_run_lock = */false);
}
if (halt_error.Success())
{
Index: packages/Python/lldbsuite/test/functionalities/attach_resume/TestAttachResume.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/attach_resume/TestAttachResume.py
+++ packages/Python/lldbsuite/test/functionalities/attach_resume/TestAttachResume.py
@@ -20,7 +20,6 @@
@skipIfRemote
@expectedFailureFreeBSD('llvm.org/pr19310')
@expectedFailureWindows("llvm.org/pr24778")
- @expectedFlakeyLinux('llvm.org/pr19310')
def test_attach_continue_interrupt_detach(self):
"""Test attach/continue/interrupt/detach"""
self.build()
@@ -52,6 +51,9 @@
self.runCmd("process interrupt")
lldbutil.expect_state_changes(self, listener, [lldb.eStateStopped])
+ # Second interrupt should have no effect.
+ self.expect("process interrupt", patterns=["Process is not running"], error=True)
+
# check that this breakpoint is auto-cleared on detach (r204752)
self.runCmd("br set -f main.cpp -l %u" % (line_number('main.cpp', '// Set breakpoint here')))
Index: packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py
===================================================================
--- packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py
+++ packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py
@@ -57,15 +57,15 @@
frame = thread.GetFrameAtIndex(0)
- value = frame.EvaluateExpression ("wait_a_while (50000)", options)
+ value = frame.EvaluateExpression ("wait_a_while (200000)", options)
self.assertTrue (value.IsValid())
self.assertFalse (value.GetError().Success())
# Now do the same thing with the command line command, and make sure it works too.
interp = self.dbg.GetCommandInterpreter()
result = lldb.SBCommandReturnObject()
- return_value = interp.HandleCommand ("expr -t 100 -u true -- wait_a_while(50000)", result)
+ return_value = interp.HandleCommand ("expr -t 100 -u true -- wait_a_while(200000)", result)
self.assertTrue (return_value == lldb.eReturnStatusFailed)
# Okay, now do it again with long enough time outs:
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -1355,6 +1355,7 @@
Error
ResumeSynchronous (Stream *stream);
+
//------------------------------------------------------------------
/// Halts a running process.
///
@@ -1367,12 +1368,15 @@
/// @param[in] clear_thread_plans
/// If true, when the process stops, clear all thread plans.
///
+ /// @param[in] use_run_lock
+ /// Whether to release the run lock after the stop.
+ ///
/// @return
/// Returns an error object. If the error is empty, the process is halted.
/// otherwise the halt has failed.
//------------------------------------------------------------------
Error
- Halt (bool clear_thread_plans = false);
+ Halt (bool clear_thread_plans = false, bool use_run_lock = true);
//------------------------------------------------------------------
/// Detaches from a running or stopped process.
@@ -1683,9 +1687,8 @@
/// DoHalt must produce one and only one stop StateChanged event if it actually
/// stops the process. If the stop happens through some natural event (for
/// instance a SIGSTOP), then forwarding that event will do. Otherwise, you must
- /// generate the event manually. Note also, the private event thread is stopped when
- /// DoHalt is run to prevent the events generated while halting to trigger
- /// other state changes before the halt is complete.
+ /// generate the event manually. This function is called from the context of the
+ /// private state thread.
///
/// @param[out] caused_stop
/// If true, then this Halt caused the stop, otherwise, the
@@ -2861,12 +2864,16 @@
// Returns the process state when it is stopped. If specified, event_sp_ptr
// is set to the event which triggered the stop. If wait_always = false,
// and the process is already stopped, this function returns immediately.
+ // If the process is hijacked and use_run_lock is true (the default), then this
+ // function releases the run lock after the stop. Setting use_run_lock to false
+ // will avoid this behavior.
lldb::StateType
WaitForProcessToStop(const TimeValue *timeout,
lldb::EventSP *event_sp_ptr = nullptr,
bool wait_always = true,
Listener *hijack_listener = nullptr,
- Stream *stream = nullptr);
+ Stream *stream = nullptr,
+ bool use_run_lock = true);
uint32_t
GetIOHandlerID () const
@@ -3281,12 +3288,6 @@
std::string m_exit_string;
};
- bool
- HijackPrivateProcessEvents (Listener *listener);
-
- void
- RestorePrivateProcessEvents ();
-
bool
PrivateStateThreadIsValid () const
{
@@ -3372,7 +3373,6 @@
std::vector<PreResumeCallbackAndBaton> m_pre_resume_actions;
ProcessRunLock m_public_run_lock;
ProcessRunLock m_private_run_lock;
- Predicate<bool> m_currently_handling_event; // This predicate is set in HandlePrivateEvent while all its business is being done.
ArchSpec::StopInfoOverrideCallbackType m_stop_info_override_callback;
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.
@@ -3436,6 +3436,9 @@
void
HandlePrivateEvent (lldb::EventSP &event_sp);
+ Error
+ HaltPrivate();
+
lldb::StateType
WaitForProcessStopPrivate (const TimeValue *timeout, lldb::EventSP &event_sp);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
