Fix race during process interruption

[labath]

Summary

The following situation was occuring in TestAttachResume:

• we did a "continue" from a breakpoint (which involves a private start-stop to step over the breakpoint)
• after receiving the stop-reply from the step-over, we issue a "detach" (which requires a process interrupt)
• at this moment, the public state is "running", private state is "about-to-be-stopped" (the stopped event was broadcast, but it was not received yet)
• StopForDestroyOrDetach (public thread) notes the public state is running, sends an interrupt request to the private thread
• private thread gets the eBroadcastBitInterrupt (before the eStateStopped message), and asks the process plugin to stop (via Halt())
• process plugin says it has nothing to do as the process is already stopped
• private thread shrugs and carries on. receives the stop event, restores the breakpoint and resumes the process.
• after a while, the public thread times out and says it failed to stop the process

This patch does the following:

• splits Halt() into two functions, private and public, their usage depends on the context
  • public Halt(): sends eBroadcastBitInterrupt to the private thread and waits for the Stop event
  • HaltPrivate(): asks the plugin to stop and makes a note that the halt was requested. When the next stop event comes it sets the interrupt flag on it.
• removes HijackPrivateProcessEvents(), as the only user (old Halt()) has gone away
• removes the m_currently_handling_event hack, as the new Halt() does not need it
• adds a use_run_lock parameter to public Halt() and WaitForProcessToStop(). This was needed because RunThreadPlan uses Halt() while holding the run lock and we don't want Halt() to take it away from him.

[amccarth]

Nice detective work.

include/lldb/Target/Process.h

1372 ↗

(On Diff #41147)

Did you mean "Whether to release ..."?

[labath]

Fix typo

[clayborg]

One minor nit, but looks good.

source/Target/Process.cpp

4622 ↗

(On Diff #41215)

Put "{" on next line

[jingham]

This looks correct, I just had a few more nits...

source/Target/Process.cpp

3827–3828 ↗

(On Diff #41215)

Is this comment correct anymore?

5719–5720 ↗

(On Diff #41215)

The convention in lldb for reminding ourselves which parameters we are passing is to do:

const bool clear_thread_plans = false;
const bool use_run_lock = false;
Halt (clear_thread_plans, use_run_lock);

lldb/trunk/include/lldb/Target/Process.h

#endif
     /// @see Thread:Step()
     /// @see Thread:Suspend()
     //------------------------------------------------------------------
     Error
     Resume();
 
     Error
     ResumeSynchronous (Stream *stream);
+
     //------------------------------------------------------------------
     /// Halts a running process.
     ///
     /// This function is not meant to be overridden by Process
     /// subclasses.
     /// If the process is successfully halted, a eStateStopped
     /// process event with GetInterrupted will be broadcast.  If false, we will
     /// halt the process with no events generated by the halt.
     ///
     /// @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.
     ///
     /// This function is not meant to be overridden by Process
     /// subclasses.
     ///
     /// @param[in] keep_stopped
#endif
     WillHalt () { return Error(); }
 
     //------------------------------------------------------------------
     /// Halts a running process.
     ///
     /// 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 
     ///     process was already stopped.
     ///
     /// @return
     ///     Returns \b true if the process successfully halts, \b false
     ///     otherwise.
public:
     // Event Handling
     //------------------------------------------------------------------
     lldb::StateType
     GetNextEvent (lldb::EventSP &event_sp);
 
     // 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
     {
         return m_iohandler_sync.GetValue();
     }
 
     //--------------------------------------------------------------------------------------
public:
         EventActionResult HandleBeingInterrupted() override;
         const char *GetExitString() override;
 
     private:
         uint32_t m_exec_count;
         std::string m_exit_string;
     };
 
-    bool 
-    HijackPrivateProcessEvents (Listener *listener);
-    
-    void 
-    RestorePrivateProcessEvents ();
-    
-    bool
+    bool
     PrivateStateThreadIsValid () const
     {
         return m_private_state_thread.IsJoinable();
     }
     
     void
     ForceNextEventDelivery()
     {
protected:
     AllocatedMemoryCache        m_allocated_memory_cache;
     bool                        m_should_detach;   /// Should we detach if the process object goes away with an explicit call to Kill or Detach?
     LanguageRuntimeCollection   m_language_runtimes;
     InstrumentationRuntimeCollection m_instrumentation_runtimes;
     std::unique_ptr<NextEventAction> m_next_event_action_ap;
     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.
     bool                        m_finalizing; // This is set at the beginning of Process::Finalize() to stop functions from looking up or creating things during a finalize call
     bool                        m_finalize_called; // This is set at the end of Process::Finalize()
     bool                        m_clear_thread_plans_on_stop;
     bool                        m_force_next_event_delivery;
     lldb::StateType             m_last_broadcast_state;   /// This helps with the Public event coalescing in ShouldBroadcastEvent.
protected:
     // private state thread.
     
     lldb::thread_result_t
     RunPrivateStateThread (bool is_secondary_thread);
 
     void
     HandlePrivateEvent (lldb::EventSP &event_sp);
 
+    Error
+    HaltPrivate();
+
     lldb::StateType
     WaitForProcessStopPrivate (const TimeValue *timeout, lldb::EventSP &event_sp);
 
     // This waits for both the state change broadcaster, and the control broadcaster.
     // If control_only, it only waits for the control broadcaster.
 
     bool
     WaitForEventsPrivate (const TimeValue *timeout, lldb::EventSP &event_sp, bool control_only);

lldb/trunk/packages/Python/lldbsuite/test/expression_command/timeout/TestCallWithTimeout.py

def test(self):
         
         # First set the timeout too short, and make sure we fail.
         options = lldb.SBExpressionOptions()
         options.SetTimeoutInMicroSeconds(10)
         options.SetUnwindOnError(True)
 
         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:
 
         options.SetTimeoutInMicroSeconds(1000000)
         value = frame.EvaluateExpression ("wait_a_while (1000)", options)
         self.assertTrue(value.IsValid())
         self.assertTrue (value.GetError().Success() == True)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/attach_resume/TestAttachResume.py

 
 class AttachResumeTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @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()
         self.process_attach_continue_interrupt_detach()
 
     def process_attach_continue_interrupt_detach(self):
         """Test attach/continue/interrupt/detach"""
 
def process_attach_continue_interrupt_detach(self):
 
         # be sure to continue/interrupt/continue (r204504)
         self.runCmd("c")
         lldbutil.expect_state_changes(self, listener, [lldb.eStateRunning])
 
         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')))
 
         self.runCmd("c")
         lldbutil.expect_state_changes(self, listener, [lldb.eStateRunning, lldb.eStateStopped])
         self.expect('br list', 'Breakpoint not hit',
             substrs = ['hit count = 1'])
 

lldb/trunk/source/Target/Process.cpp

Process::Process(lldb::TargetSP target_sp, Listener &listener, const UnixSignalsSP &unix_signals_sp) :
     m_profile_data (),
     m_iohandler_sync (0),
     m_memory_cache (*this),
     m_allocated_memory_cache (*this),
     m_should_detach (false),
     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),
     m_clear_thread_plans_on_stop (false),
     m_force_next_event_delivery (false),
     m_last_broadcast_state (eStateInvalid),
     m_destroy_in_process (false),
     m_can_interpret_function_calls(false),
if (log)
         log->Printf("Process::%s waited for m_iohandler_sync to change from %u, new value is %u", __FUNCTION__, iohandler_id, new_iohandler_id);
 }
 
 StateType
 Process::WaitForProcessToStop (const TimeValue *timeout,
                                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
     // on the event.
     if (event_sp_ptr)
         event_sp_ptr->reset();
     StateType state = GetState();
     // If we are exited or detached, we won't ever get back to any
if (!wait_always &&
         StateIsStoppedState(state, true) &&
         StateIsStoppedState(GetPrivateState(), true))
     {
         if (log)
             log->Printf("Process::%s returning without waiting for events; process private and public states are already 'stopped'.",
                         __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;
     }
 
     while (state != eStateInvalid)
     {
         EventSP event_sp;
         state = WaitForStateChangedEvents (timeout, event_sp, hijack_listener);
         if (event_sp_ptr && event_sp)
             *event_sp_ptr = event_sp;
 
         bool pop_process_io_handler = hijack_listener != NULL;
         Process::HandleProcessStateChangedEvent (event_sp, stream, pop_process_io_handler);
 
         switch (state)
         {
         case eStateCrashed:
         case eStateDetached:
         case eStateExited:
         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:
             if (Process::ProcessEventData::GetRestartedFromEvent(event_sp.get()))
                 continue;
             else
             {
                 // 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;
             }
         default:
             continue;
         }
     }
     return state;

 }
 
 void
 Process::RestoreProcessEvents ()
 {
     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)
 {
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
     if (log)
         log->Printf ("Process::%s (timeout = %p, event_sp)...", __FUNCTION__,
                      static_cast<const void*>(timeout));
if (error.Success())
         }
     }
     else if (log)
         log->Printf ("Process::PrivateResume() got an error \"%s\".", error.AsCString("<unknown error>"));
     return error;
 }
 
 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())
-    {
-        
-        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();
-            }
-        }
-    }
-    // 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);
+    SendAsyncInterrupt();
+        RestoreProcessEvents();
+        return Error();
+
+    // Wait for 10 second for the process to stop.
+    StateType state = WaitForProcessToStop(&timeout_time, &event_sp, true, &halt_listener,
+                                           nullptr, use_run_lock);
+    RestoreProcessEvents();
+    if (state == eStateInvalid || ! event_sp)
+        // We timed out and didn't get a stop event...
+        return Error("Halt timed out. State = %s", StateAsCString(GetState()));
 
-    return error;
+    BroadcastEvent(event_sp);
+
+    return Error();
 }
 
 Error
 Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp)
 {
     Error error;
     if (m_public_state.GetValue() == eStateRunning)
     {

 }
 
 void
 Process::HandlePrivateEvent (EventSP &event_sp)
 {
     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:
     if (m_next_event_action_ap.get() != NULL)
     {
         NextEventAction::EventActionResult action_result = m_next_event_action_ap->PerformAction(event_sp);
         if (log)
             log->Printf ("Ran next event action, result was %d.", action_result);
if (m_next_event_action_ap.get() != NULL)
             case NextEventAction::eEventActionExit:
                 // Handle Exiting Here.  If we already got an exited event,
                 // we should just propagate it.  Otherwise, swallow this event,
                 // and set our state to exit so the next event will kill us.
                 if (new_state != eStateExited)
                 {
                     // 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;
                 }
                 SetNextEventAction(NULL);
                 break;
         }
     }
     
else
         {
             log->Printf ("Process::%s (pid = %" PRIu64 ") suppressing state %s (old state %s): should_broadcast == false",
                          __FUNCTION__, 
                          GetID(), 
                          StateAsCString(new_state), 
                          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
 Process::PrivateStateThread (void *arg)
 {
     PrivateStateThreadArgs *real_args = static_cast<PrivateStateThreadArgs *> (arg);
     thread_result_t result = real_args->process->RunPrivateStateThread(real_args->is_secondary_thread);
     return result;
 }
 
 thread_result_t
 Process::RunPrivateStateThread (bool is_secondary_thread)
 {
     bool control_only = true;
     m_private_state_control_wait.SetValue (false, eBroadcastNever);
 
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     if (log)
         log->Printf ("Process::%s (arg = %p, pid = %" PRIu64 ") thread starting...",
                      __FUNCTION__, static_cast<void*>(this), GetID());
 
     bool exit_now = false;
+    bool interrupt_requested = false;
     while (!exit_now)
     {
         EventSP event_sp;
         WaitForEventsPrivate (NULL, event_sp, control_only);
         if (event_sp->BroadcasterIs(&m_private_state_control_broadcaster))
         {
             if (log)
                 log->Printf ("Process::%s (arg = %p, pid = %" PRIu64 ") got a control event: %d",
while (!exit_now)
                 BroadcastEvent (eBroadcastBitInterrupt, NULL);
             }
             else
             {
                 if (log)
                     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;
         }
 
         const StateType internal_state = Process::ProcessEventData::GetStateFromEvent(event_sp.get());
 
         if (internal_state != eStateInvalid)
         {
             if (m_clear_thread_plans_on_stop &&
                 StateIsStoppedState(internal_state, true))
             {
                 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);
         }
 
         if (internal_state == eStateInvalid || 
             internal_state == eStateExited  ||
             internal_state == eStateDetached )
         {
             if (log)
#endif
                                         do_resume,
                                         handle_running_event);
                     }
 
                     if (restarted)
                     {
                         // 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();
+                        const bool clear_thread_plans = false;
+                        const bool use_run_lock = false;
+                        Halt(clear_thread_plans, use_run_lock);
                     }
 
                     errors.Printf("Didn't get running event after initial resume, got %s instead.",
                                   StateAsCString(stop_state));
                     return_value = eExpressionSetupError;
                     break;
                 }
 
#endif
 
             if (got_event)
             {
                 if (event_sp.get())
                 {
                     bool keep_going = false;
                     if (event_sp->GetType() == eBroadcastBitInterrupt)
                     {
-                        Halt();
+                        const bool clear_thread_plans = false;
+                        const bool use_run_lock = false;
+                        Halt(clear_thread_plans, use_run_lock);
                         return_value = eExpressionInterrupted;
                         errors.Printf ("Execution halted by user interrupt.");
                         if (log)
                             log->Printf ("Process::RunThreadPlan(): Got  interrupted by eBroadcastBitInterrupted, exiting.");
                         break;
                     }
                     else
                     {
#endif
                 const uint32_t num_retries = 5;
                 while (try_halt_again < num_retries)
                 {
                     Error halt_error;
                     if (do_halt)
                     {
                         if (log)
                             log->Printf ("Process::RunThreadPlan(): Running Halt.");
-                        halt_error = Halt();
+                        const bool clear_thread_plans = false;
+                        const bool use_run_lock = false;
+                        Halt(clear_thread_plans, use_run_lock);
                     }
                     if (halt_error.Success())
                     {
                         if (log)
                             log->PutCString ("Process::RunThreadPlan(): Halt succeeded.");
 
                         real_timeout = TimeValue::Now();
                         real_timeout.OffsetWithMicroSeconds(500000);